[openstreetmap/openstreetmap-website] Support android intent uri for JOSM style remote control (#1478)

[Tom Hughes]

tomhughes requested changes on this pull request.



> +    },
+    Android: function() {
+        return /Android/i.test(navigator.userAgent);
+    },
+    BlackBerry: function() {
+        return /BlackBerry/i.test(navigator.userAgent);
+    },
+    iOS: function() {
+        return /iPhone|iPad|iPod/i.test(navigator.userAgent);
+    },
+    any: function() {
+        return (isMobile.Android() || isMobile.BlackBerry() || isMobile.iOS() || isMobile.Windows());
+    }
+  };
+
+  function url() {

This is far too vague as a name for this function at this scope - it should either be more explanatory or the function should be pushed down inside `remoteEditHandler` so that the meaning is implicit.

That said I'm not really sure we it needs to be a function and can't just be done inline in `remoteEditHandler` without having to repeat the android test...

> @@ -255,17 +281,17 @@ $(document).ready(function () {
     var iframe = $('<iframe>')
         .hide()
         .appendTo('body')
-        .attr("src", url + querystring.stringify(query))
+        .attr("src", url() + querystring.stringify(query) + (android ? '#Intent;scheme=josm;end;':''))

If `url` is going to stay as a function then I would just pass `query` in to it and let it figure out the whole URL in one go rather than splitting the logic for the intent URL across two different locations.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/1478#pullrequestreview-25652556
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstreetmap.org/pipermail/rails-dev/attachments/20170307/c662ca2c/attachment.html>