zekefarwell / josm-strava-heatmap

A browser extension that simplifies getting the TMS imagery url for using the Strava Heatmap in JOSM
MIT License
62 stars 3 forks source link

Adds JOSM Remote Control link #3

Closed dericke closed 2 years ago

dericke commented 2 years ago

This PR adds a one-click link to open the selected Strava layer in JOSM, via the remote control functionality. This only loads the imagery for the current session, and will not make the imagery appear in JOSM's Imagery menu.

I messed around a bunch with the way the URL is constructed internally, and am more than happy to make changes to this PR if there's a less-disruptive way to achieve the same result, and to incorporate feedback for better UX.

zekefarwell commented 2 years ago

Hey this is super cool! I didn't even know you could open an imagery layer in JOSM via remote control. It does bring up some questions about what the optimal UX should be. I almost want to just remove the copy to clipboard logic and use only this instead, but it looks like when the imagery is opened this way it doesn't save the layer for later, just for the current session? I guess if it saved it that could be problematic as it might not update an existing layer and users would end up with tons of redundant strava heatmap layers. Maybe saving the layer is pointless anyway since the keys change so often and we should just promote a workflow where you use this extension any time you want to use the heatmap.

I originally wrote this just for the use case of updating an existing layer in the JOSM imagery preferences and that's why the tms: prefix is included.
Screen Shot 2021-10-14 at 5 11 18 PM But that's annoying for JOSM users adding it for the first time and for iD users since you have to remove the prefix in those cases. Maybe the main button should just open up the modal but not copy to the clipboard and then have secondary buttons in the modal for opening directly in JOSM, and copying to clipboard. What do you think?

dericke commented 2 years ago

There's definitely pros and cons to each method. I think it comes down to whether the user would rather spend a shorter amount of time every time they start up JOSM, or a larger amount of time every couple of weeks. I'd be inclined to make both options available to the user.

Maybe the main button should just open up the modal but not copy to the clipboard and then have secondary buttons in the modal for opening directly in JOSM, and copying to clipboard.

I like this approach. We could also add a similar button that uses iD. Would it be a good idea to shrink the current URL block to a single line with a scrollbar? That seems to be a common UI pattern for long strings of text that are only meant to be copied and pasted, but not to be human-readable.

zekefarwell commented 2 years ago

Good call. Probably best to have both the copy option and the remote control link available.

Would it be a good idea to shrink the current URL block to a single line with a scrollbar?

Maybe? I displayed the full url in a block sort of as a confirmation that it had been properly generated and also as a fallback, so if the automatic copy failed you could easily select the text and copy it. This may be totally unnecessary, but I figured I'd be pretty annoyed if the clipboard code stopped working for some reason and there wasn't a way for me to just select and copy the url manually. It does take up a lot of space, but on the other hand there isn't too much else competing for space even with a few new buttons added.

If you're feeling like trying out some additional UI tweaks I can leave this PR open. If not, I'll go ahead and merge it.

dericke commented 2 years ago

Sure, I'll try to make the UI a little prettier, add a link for iD, and I'll leave the text block as is. I'll let you know when I've finished, hopefully in the next few days.

dericke commented 2 years ago

@zekefarwell I made all the changes I planned on making. The icon for the copy button I added is from Google Fonts and is freely-licensed.