Merge remote-tracking branch 'upstream/pull/5443'

app/assets/javascripts/index/new_note.js

@@ -5,7 +5,7 @@ OSM.NewNote = function (map) {
       content = $("#sidebar_content"),
       page = {},
       addNoteButton = $(".control-note .control-button"),
-      newNote,
+      newNoteMarker,
       halo;
 
   var noteIcons = {
@@ -35,39 +35,21 @@ OSM.NewNote = function (map) {
     OSM.router.route("/note/new");
   });
 
-  function createNote(marker, form, url) {
-    var location = marker.getLatLng().wrap();
-
-    marker.options.draggable = false;
-    marker.dragging.disable();
-
-    $(form).find("input[type=submit]").prop("disabled", true);
-
+  function createNote(location, text, callback) {
     $.ajax({
-      url: url,
+      url: "/api/0.6/notes.json",
       type: "POST",
       oauth: true,
       data: {
         lat: location.lat,
         lon: location.lng,
-        text: $(form.text).val()
+        text
       },
-      success: function (feature) {
-        noteCreated(feature, marker);
-      }
+      success: callback
     });
-
-    function noteCreated(feature, marker) {
-      content.find("textarea").val("");
-      updateMarker(feature);
-      newNote = null;
-      noteLayer.removeLayer(marker);
-      addNoteButton.removeClass("active");
-      OSM.router.route("/note/" + feature.properties.id);
-    }
   }
 
-  function updateMarker(feature) {
+  function addCreatedNoteMarker(feature) {
     var marker = L.marker(feature.geometry.coordinates.reverse(), {
       icon: noteIcons[feature.properties.status],
       opacity: 0.9,
@@ -75,7 +57,69 @@ OSM.NewNote = function (map) {
     });
     marker.id = feature.properties.id;
     marker.addTo(noteLayer);
-    return marker;
+  }
+
+  function addHalo(latlng) {
+    if (halo) map.removeLayer(halo);
+
+    halo = L.circleMarker(latlng, {
+      weight: 2.5,
+      radius: 20,
+      fillOpacity: 0.5,
+      color: "#FF6200"
+    });
+
+    map.addLayer(halo);
+  }
+
+  function removeHalo() {
+    if (halo) map.removeLayer(halo);
+    halo = null;
+  }
+
+  function addNewNoteMarker(latlng) {
+    if (newNoteMarker) map.removeLayer(newNoteMarker);
+
+    newNoteMarker = L.marker(latlng, {
+      icon: noteIcons.new,
+      opacity: 0.9,
+      draggable: true
+    });
+
+    newNoteMarker.on("dragstart dragend", function (a) {
+      removeHalo();
+      if (a.type === "dragend") {
+        addHalo(newNoteMarker.getLatLng());
+      }
+    });
+
+    newNoteMarker.addTo(map);
+    addHalo(newNoteMarker.getLatLng());
+
+    newNoteMarker.on("dragend", function () {
+      content.find("textarea").focus();
+    });
+  }
+
+  function removeNewNoteMarker() {
+    removeHalo();
+    if (newNoteMarker) map.removeLayer(newNoteMarker);
+    newNoteMarker = null;
+  }
+
+  function moveNewNotMarkerToClick(e) {
+    if (newNoteMarker) newNoteMarker.setLatLng(e.latlng);
+    if (halo) halo.setLatLng(e.latlng);
+    content.find("textarea").focus();
+  }
+
+  function updateControls() {
+    const zoomedOut = addNoteButton.hasClass("disabled");
+    const withoutText = content.find("textarea").val() === "";
+
+    content.find("#new-note-zoom-warning").prop("hidden", !zoomedOut);
+    content.find("input[type=submit]").prop("disabled", zoomedOut || withoutText);
+    if (newNoteMarker) newNoteMarker.setOpacity(zoomedOut ? 0.5 : 0.9);
   }
 
   page.pushstate = page.popstate = function (path) {
@@ -84,29 +128,7 @@ OSM.NewNote = function (map) {
     });
   };
 
-  function newHalo(loc, a) {
-    var hasHalo = halo && map.hasLayer(halo);
-
-    if (a === "dragstart" && hasHalo) {
-      map.removeLayer(halo);
-    } else {
-      if (hasHalo) map.removeLayer(halo);
-
-      halo = L.circleMarker(loc, {
-        weight: 2.5,
-        radius: 20,
-        fillOpacity: 0.5,
-        color: "#FF6200"
-      });
-
-      map.addLayer(halo);
-    }
-  }
-
   page.load = function (path) {
-    if (addNoteButton.hasClass("disabled")) return;
-    if (addNoteButton.hasClass("active")) return;
-
     addNoteButton.addClass("active");
 
     map.addLayer(noteLayer);
@@ -124,44 +146,39 @@ OSM.NewNote = function (map) {
       padding: [50, 50]
     });
 
-    newNote = L.marker(markerLatlng, {
-      icon: noteIcons.new,
-      opacity: 0.9,
-      draggable: true
-    });
-
-    newNote.on("dragstart dragend", function (a) {
-      newHalo(newNote.getLatLng(), a.type);
-    });
-
-    newNote.addTo(noteLayer);
-    newHalo(newNote.getLatLng());
-
-    newNote.on("remove", function () {
-      addNoteButton.removeClass("active");
-    }).on("dragend", function () {
-      content.find("textarea").focus();
-    });
+    addNewNoteMarker(markerLatlng);
 
     content.find("textarea")
-      .on("input", disableWhenBlank)
+      .on("input", updateControls)
       .focus();
 
-    function disableWhenBlank(e) {
-      $(e.target.form.add).prop("disabled", $(e.target).val() === "");
-    }
-
     content.find("input[type=submit]").on("click", function (e) {
+      const location = newNoteMarker.getLatLng().wrap();
+      const text = content.find("textarea").val();
+
       e.preventDefault();
-      createNote(newNote, e.target.form, "/api/0.6/notes.json");
+      $(this).prop("disabled", true);
+      newNoteMarker.options.draggable = false;
+      newNoteMarker.dragging.disable();
+
+      createNote(location, text, (feature) => {
+        content.find("textarea").val("");
+        addCreatedNoteMarker(feature);
+        OSM.router.route("/note/" + feature.properties.id);
+      });
     });
 
+    map.on("click", moveNewNotMarkerToClick);
+    addNoteButton.on("disabled enabled", updateControls);
+    updateControls();
+
     return map.getState();
   };
 
   page.unload = function () {
-    if (newNote) noteLayer.removeLayer(newNote);
-    if (halo) map.removeLayer(halo);
+    map.off("click", moveNewNotMarkerToClick);
+    addNoteButton.off("disabled enabled", updateControls);
+    removeNewNoteMarker();
     addNoteButton.removeClass("active");
   };
 

app/assets/javascripts/leaflet.note.js

@@ -14,12 +14,19 @@ L.OSM.note = function (options) {
     map.on("zoomend", update);
 
     function update() {
-      var disabled = OSM.STATUS === "database_offline" || map.getZoom() < 12;
+      var wasDisabled = link.hasClass("disabled"),
+          isDisabled = OSM.STATUS === "database_offline" || map.getZoom() < 12;
       link
-        .toggleClass("disabled", disabled)
-        .attr("data-bs-original-title", I18n.t(disabled ?
+        .toggleClass("disabled", isDisabled)
+        .attr("data-bs-original-title", I18n.t(isDisabled ?
           "javascripts.site.createnote_disabled_tooltip" :
           "javascripts.site.createnote_tooltip"));
+
+      if (isDisabled && !wasDisabled) {
+        link.trigger("disabled");
+      } else if (wasDisabled && !isDisabled) {
+        link.trigger("enabled");
+      }
     }
 
     update();

app/views/notes/new.html.erb

@@ -9,6 +9,7 @@
                                          :log_in => link_to(t(".anonymous_warning_log_in"), login_path(:referer => new_note_path)),
                                          :sign_up => link_to(t(".anonymous_warning_sign_up"), user_new_path) %></p>
   <% end %>
+  <p class="alert alert-warning" id="new-note-zoom-warning" hidden><%= t "javascripts.site.createnote_disabled_tooltip" %></p>
   <form action="#">
     <input type="hidden" name="lon" autocomplete="off">
     <input type="hidden" name="lat" autocomplete="off">

test/system/create_note_test.rb

@@ -15,6 +15,60 @@ class CreateNoteTest < ApplicationSystemTestCase
     end
   end
 
+  test "cannot create new note when zoomed out" do
+    visit new_note_path(:anchor => "map=12/0/0")
+
+    within_sidebar do
+      assert_no_content "Zoom in to add a note"
+      assert_button "Add Note", :disabled => true
+
+      fill_in "text", :with => "Some newly added note description"
+
+      assert_no_content "Zoom in to add a note"
+      assert_button "Add Note", :disabled => false
+    end
+
+    find(".control-button.zoomout").click
+
+    within_sidebar do
+      assert_content "Zoom in to add a note"
+      assert_button "Add Note", :disabled => true
+    end
+
+    find(".control-button.zoomin").click
+
+    within_sidebar do
+      assert_no_content "Zoom in to add a note"
+      assert_button "Add Note", :disabled => false
+
+      click_on "Add Note"
+
+      assert_content "Unresolved note ##{Note.last.id}"
+      assert_content "Some newly added note description"
+    end
+  end
+
+  test "can open new note page when zoomed out" do
+    visit new_note_path(:anchor => "map=11/0/0")
+
+    within_sidebar do
+      assert_content "Zoom in to add a note"
+      assert_button "Add Note", :disabled => true
+
+      fill_in "text", :with => "Some newly added note description"
+
+      assert_content "Zoom in to add a note"
+      assert_button "Add Note", :disabled => true
+    end
+
+    find(".control-button.zoomin").click
+
+    within_sidebar do
+      assert_no_content "Zoom in to add a note"
+      assert_button "Add Note", :disabled => false
+    end
+  end
+
   test "cannot create note when api is readonly" do
     with_settings(:status => "api_readonly") do
       visit new_note_path(:anchor => "map=18/0/0")