[openstreetmap/openstreetmap-website] Load notes with remote control (PR #3718)

[Tom Hughes]

@tomhughes commented on this pull request.

I really don't understand some of the breakdown of changes into commits here - the first and last commits are fairly obvious but 591a7464127f5a0e884e34cc4480d257ebc44cb9 and 4f9d7fbe7f36b449c9ed544969a8313763240f7d have descriptions that don't really explain what the commit is doing and for the second one especially I don't really understand what either of the two small changes have to do with each other or with the title of the commit.

> @@ -20,40 +19,41 @@ OSM.Note = function (map) {
       iconAnchor: [12, 40]
     })
   };
+  noteIcons.hidden = noteIcons.closed; // TODO replace with actual "hidden" icon when it's added

Why do we want an icon for hidden notes? They are supposed to be hidden...

> @@ -37,7 +37,8 @@ OSM = {
   SEARCHING:               <%= image_path("searching.gif").to_json %>,
 
   apiUrl: function (object) {
-    var url = "/api/" + OSM.API_VERSION + "/" + object.type + "/" + object.id;
+    var apiType = object.type === "note" ? "notes" : object.type;

This is kind of ugly... I'm half inclined to say why not just use `notes` as the object type, then you wouldn't need to jump through hoops here to fix things up.

Then again that means internally we are inconsistent about whether the object type is singular or plural, which also offends me ;-)

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/3718#pullrequestreview-1122346507
You are receiving this because you are subscribed to this thread.

Message ID: <openstreetmap/openstreetmap-website/pull/3718/review/1122346507 at github.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstreetmap.org/pipermail/rails-dev/attachments/20220927/132c5eb4/attachment.htm>