Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New map UI #328

Merged
merged 71 commits into from Jul 19, 2013

Conversation

@jfirebaugh
Copy link
Member

commented Jul 9, 2013

This isn't quite ready to go (waiting on an upstream Leaflet bug), but I wanted to share and possibly get deployed for testing.

image

Notable features:

  • New layer chooser UI
  • New geolocation control
  • New share control (with room for "Include marker" checkbox and reorganization/consolidation with some of the "Export" features)
  • Consistent style with map UI on other pages (just the zoom buttons)

Based on work by @samanpwbb.

@Zverik

This comment has been minimized.

Copy link
Contributor

commented Jul 9, 2013

I don't like low-zoom thumbnails for layers: they don't demonstate what layers are about. I'd prefer z13 fragment of any city.

"Map Data" and "Map Notes" look similar, but they are completely different: the former is used by expert mappers to debug the map, and the latter is a first stop for any user wanting to improve the map without learning to draw on their own. I propose to modify "Map Notes" button: since the layer panel would probably take the whole right side of a screen, it could be moved to the bottom of it and somehow visually linked to "Add a note" button. At least it should be renamed: e.g. simply "Notes".

@openstreetmap-website

This comment has been minimized.

Copy link

commented Jul 9, 2013

Or how about a live thumbnail of the current area at the current zoom level for each of the different possible layers?

On 9 Jul 2013, at 13:12, Ilya Zverev notifications@github.com wrote:

I don't like low-zoom thumbnails for layers: they don't demonstate what layers are about. I'd prefer z13 fragment of any city.

"Map Data" and "Map Notes" look similar, but they are completely different: the former is used by expert mappers to debug the map, and the latter is a first stop for any user wanting to improve the map without learning to draw on their own. I propose to modify "Map Notes" button: since the layer panel would probably take the whole right side of a screen, it could be moved to the bottom of it and somehow visually linked to "Add a note" button. At least it should be renamed: e.g. simply "Notes".


Reply to this email directly or view it on GitHub.


rails-dev mailing list
rails-dev@openstreetmap.org
http://lists.openstreetmap.org/listinfo/rails-dev

@tomhughes

This comment has been minimized.

Copy link
Member

commented Jul 9, 2013

@Zverik it probably isn't obvious from the pictures @jfirebaugh included, but those are not static images - they are live maps of the area you are looking at.

I've put a test site up at http://mapui.apis.dev.openstreetmap.org/ now, where you can see how it works.

@systemed

This comment has been minimized.

Copy link
Contributor

commented Jul 9, 2013

Works really nicely in the flesh - I like it.

@Zverik - re: "Map Data" and "Map Notes" - not really that different in this case, I think. Adding a note is certainly user-facing, and that's what the little always-visible speech bubble icon is for (though I admit the icon's meaning isn't too clear to me). But reading existing notes is more of a mapper-facing activity: the only reason to read notes is to fix them.

@Zverik

This comment has been minimized.

Copy link
Contributor

commented Jul 9, 2013

Ah, ok.

There are a lot of bugs with panels in Opera 12.15. For example, "»" button and scrollbars don't work, most clicks go to the map, and not to the panel, only two last layers can be selected, and they are somehow selected both. "Map Notes" checkbox has three states, two of which are "enabled", but only one of them is "actually show notes".

Also, there is probably a cross-browser bug: open "layers" panel, then change to "share", move the map, return to "layers": all four maps there will move smoothly. I guess this feature should be disabled.

@Zverik

This comment has been minimized.

Copy link
Contributor

commented Jul 9, 2013

As for design, it is hard to notice transparency change on hover, especially on bright layers, like a transport map. Icons are confusing: there should be at least tooltips. Having controls at the right side is confusing: I understand this would make more sense when other improvements in Saman's presentation are implemented, but for now I think this is a change too big, and will confuse a lot of user, especially with other map providers keeping their zoom controls at the left or top sides.

I like that panels don't shift the map, unlike regular tabs (e.g. "Export"). But they shift all the buttons, I'm not sure this is good.

Regular panels (the same "Export") are separated from the map with a gray line, but new panels are not. Since there are lines in new panels as well, I suggest adding those vertical separating lines.

@tomhughes

This comment has been minimized.

Copy link
Member

commented Jul 9, 2013

Err.. The buttons do have tooltips!

@tmcw

This comment has been minimized.

Copy link
Contributor

commented Jul 9, 2013

I think this is a change too big, and will confuse a lot of user, especially with other map providers keeping their zoom controls at the left or top sides.

Google Maps is bottom-right, Bing is top-right.

@Zverik

This comment has been minimized.

Copy link
Contributor

commented Jul 9, 2013

Err.. The buttons do have tooltips!

I didn't mean title html attribute :) It's quicker to click and see that to wait for that tooltip to appear.

In Saman's presentation there were better tooltips, which look like leaflet's labels (for example, hover over icons on this site).

Google Maps is bottom-right

googleul

@Zverik

This comment has been minimized.

Copy link
Contributor

commented Jul 9, 2013

Also, I have noticed there is no zoom slider now, only "+" and "­-" buttons. While this probably looks better, it makes navigating slower, when you don't have a scroll wheel at hand (I don't use mouse at work). Especially when the map changes its zoom not instantly, but with a fancy smooth transition,

@tmcw

This comment has been minimized.

Copy link
Contributor

commented Jul 9, 2013

Sorry, was referring to the new Google Maps.

1gabi_181y p

@Zverik

This comment has been minimized.

Copy link
Contributor

commented Jul 9, 2013

Buttons at right looked fantastic in the design presented at SotM US. Not because they are there, and not because Google did it. All other elements in those mock-ups were located not at the left side, like they are now, but at the top of the page, along with toolbar buttons. So there are two sides on which a user have to concentrate their attention, not three, as proposed in this pull request.

@lonvia

This comment has been minimized.

Copy link
Contributor

commented Jul 9, 2013

Is is really necessary to use read-only edit boxes for sharing links? Copying from these boxes is seriously broken on Linux because the data ends up in the wrong clipboard. Also, checking out the link requires to put the cursor in the box and move it around which is more complicated than necessary.

On a more general node for usability of the share control: the main use case of the current permalink, at least for me as a mapper, is not sharing a link but updating the map: click on permalink, reload page and get updated map tiles. It would be nice if that would still be possible in one or two clicks.

@systemed

This comment has been minimized.

Copy link
Contributor

commented Jul 9, 2013

@lonvia The latter is in theory possible by clicking 'View' rather than Permalink, though there's some Leaflet 0.6-related breakage which causes the layer parameter to go a bit wacko.

@tomhughes

This comment has been minimized.

Copy link
Member

commented Jul 9, 2013

@lonvia well in theory just reloading should work as the position cookie remembers the viewport, though that will fail if the current page URL includes a location. The solution to that is probably to have a continuously updated location in the URL fragment.

@jfirebaugh

This comment has been minimized.

Copy link
Member Author

commented Jul 9, 2013

read-only edit boxes

Can you elaborate on the issue? This is just a normal <input type=text/>, not disabled or read-only in any way.

I've made it auto-select the text when clicked, so that makes it easier. There's currently no cross-browser way to copy text to the clipboard without using flash.

the main use case of the current permalink, at least for me as a mapper, is not sharing a link but updating the map: click on permalink, reload page and get updated map tiles

This is pretty far off from the intended use for the permalink -- I'd rather address it directly (along the lines of what @tomhughes has just suggested) rather than have it constrain the design of the sharing controls.

@woodpeck

This comment has been minimized.

Copy link
Contributor

commented Jul 9, 2013

Old behaviour under Linux: 1. right-click on permalink, 2. select "copy link location", 3. move cursor to target window (e.g. mail client), 4. click middle mouse button, done.

New behaviour: 1. Click on "share". Full text is auto-selected but not copied to clipboard. 2. right-click on text box. 3. select "copy" from drop-down. 4. move mouse to target window, 5. right click, 6. select "paste" from drop-down, done.

The old behaviour utilises the standard way of copying and pasting plain text under X called the "primary selection". This was used even when applications didn't have menu items for "copy" and "paste", and can even today be used with applications that only expect keyboard input. Simply mark something with the mouse anywhere and it is in your selection, no "copy" required. The new behaviour uses the "clipboard selection" which is not the preferred way of dealing with copy+paste because it requires more clicks. The painfulness of the new behaviour can be reduced by using key combinations instead of the context menu, but this is not standardised under X - for example, to paste a link into a terminal window I have to use Ctrl+C in the browser, but Shift+Ctrl+V in the terminal.

I'm pretty sure I explained that in detail to someone from MapBox a while ago (there was some redesign discussion and I explained how the way everyone was doing this with auto-select text fields was totally broken in Linux) but maybe that was before your time ;)

@ajashton

This comment has been minimized.

Copy link

commented Jul 10, 2013

For me the new behaviour works fine with the primary selection on Ubuntu in Chrome and Firefox. Left click on the share button, the text is automatically selected, and I can paste it where I like with the middle mouse button. No right-click-menu business necessary.

@Zverik

This comment has been minimized.

Copy link
Contributor

commented Jul 10, 2013

Maybe It'd be useful to make "Share" button a permalink itself, but catch clicks with javascript, so one can open a permalink in a new browser tab immediately (with shift or middle mouse button), but if they need a shortlink, they would open the panel.

Also, I'm for updating the URL in the browser's location field, but not sure if this is good: three major map providers I use (Google, Yandex, Bing) don't do that.

@lonvia

This comment has been minimized.

Copy link
Contributor

commented Jul 10, 2013

@ajashton It works sometimes but not always. The following always breaks for me: open xterm, select a bit of text there (and leave it selected), switch to Firefox/Chromium/whatnot and try copying the input box link.

@tomhughes There are other corner cases where the cookie reload does not work as expected, e.g. after you have used search.

@jfirebaugh I'm all for making this reload trick more obvious, most beginners probably don't even know that they can make their data appear on the map immediately this way. It does not need to be intermingled with share but it is something to think about before the permalink disappears. @systemed's View suggestion will do the job just fine though, so I'd consider that part of my comment solved.

@jfirebaugh

This comment has been minimized.

Copy link
Member Author

commented Jul 10, 2013

Ok, I switched back to <a> elements for the permalinks.

@seav

This comment has been minimized.

Copy link
Contributor

commented Jul 14, 2013

The solution to that is probably to have a continuously updated location in the URL fragment.

Please no. This pollutes the browser history with tons of unnecessary URLs. But I think HTML5 has a way to manage the browser history so the pollution does not happen. However, I'm not sure if this HTML5 API is supported by most browsers.

@seav

This comment has been minimized.

Copy link
Contributor

commented Jul 14, 2013

Regarding the Layers panel, I like how it makes things more understandable for people unfamiliar with OSM, but I don't like how I have to click and then scroll if I have to display map notes/data (not all of us use very high resolution displays). I use these overlays to debug the map often and adding more mouse movement is not the way to go. Maybe there can be a preference setting to make this panel minimalist (i.e., current layer control) for experienced users.

@jfirebaugh

This comment has been minimized.

Copy link
Member Author

commented Jul 17, 2013

I've rebased this onto master, which has been updated to Leaflet 0.6.3, fixing the layer issues, and refined the CSS for small screens. Those are the last actionable items I had on my list. Anything else gating a deploy?

@lonvia

This comment has been minimized.

Copy link
Contributor

commented Jul 18, 2013

The layer selector still does not work well on my tiny netbook (approx. 700px usable space vertically). Apart from having to scroll to get to the data layer selector, you also get double scroll bars which are ok usability-wise but look strange.

layersel_scrollbar

The explanations use up quite a bit of screen space, maybe they could be made a tiny bit more compact. (Personally, I'd hide the first one in a mouse over. One would expect that layer selectors are known by the average internet user.)

And to add some real bike shedding: the punctuation in the first explanation is wrong. ;)

@tmcw

This comment has been minimized.

Copy link
Contributor

commented Jul 18, 2013

@lonvia

OSM provides the underlying data for all these maps

Is correct, are you referring to something else?

@tomhughes

This comment has been minimized.

Copy link
Member

commented Jul 18, 2013

So a few minor cosmetic points:

  • The checkbox to activate the data layer should be disabled until z15 as it won't do anything until then.
  • The icons could probably do with outline: none to stop the dotted border appearing when clicked.

I am bit disturbed to see that my horrifically ugly title bar from the left hand side bar has now appeared on the new right hand side bar, but I guess for consistency we should probably stick with it for now and get rid of them together.

I also have to say that I really don't like the share panel the way it is. I'm rather afraid that by switching to links we made the error of doing what is most convenient for the handful of very technical users that are part of this thread at the expense of what will be comprehensible to the vast majority of average users. It is certainly hard to see how what we are left with here will be expanded to incorporate the other features that we want to add to the sharing panel.

@woodpeck

This comment has been minimized.

Copy link
Contributor

commented Jul 18, 2013

Is there a way how we could do something that is "comprehensible to the vast majority of average users" (the old permalink obviously wasn't, in your opinion, @tomhughes?) while at the same time not alienating the "handful of very technical users"? Would it be possible to take the original design and add to that some undocumented hidden hyperlink that can easily be used by those in the know, and doesn't confuse those tho don't know?

@seav

This comment has been minimized.

Copy link
Contributor

commented Jul 18, 2013

This was a problem with the old Map Key and it's been carried over to this new UI: If the current map layer doesn't have a map key, the map key remains blank. Maybe we can instead disable the Map Key button or show an explanatory text?

@tomhughes

This comment has been minimized.

Copy link
Member

commented Jul 18, 2013

I'll give you the data layer one (maybe that got lost in the migration to leaflet, or maybe I'm just misremembering) but it is true that the current interface disables the "add note" function until you get to z12 while this new UI allows you to add a ntoe at any zoom.

@lxbarth

This comment has been minimized.

Copy link
Contributor

commented Jul 18, 2013

Re: link vs text field for permalinks and shortlinks, @woodpeck said:

Is there a way how we could do something that is "comprehensible to the vast majority of average users"

I suggest to bring back text fields to select the links from (very common) but do a small chain-icon right next to the text field labels ("Long link" and "Short link") that is clickable.

@openstreetmap-mirror openstreetmap-mirror merged commit 7094370 into openstreetmap:master Jul 19, 2013
@tomhughes

This comment has been minimized.

Copy link
Member

commented Jul 19, 2013

I've merged and deployed this. I do have one question from my code review last night, namely what the origins of app/assets/javascripts/querystring.js are, as it has the appearance of being third party code, but it's in app rather than vendor?

@maning

This comment has been minimized.

Copy link

commented Jul 19, 2013

A few comments using the live-site:

  • Viewing a small area (z18), opening the Map Layers and then checking the Map Data, the left panel appears but not data is shown.
  • Using the Share icon, I clicked the Short link then I clicked the Add note to the map, nothing happens. Clicking the Long link and then the Add note to the map works as expected.

Overall, nice UI.

@tomhughes

This comment has been minimized.

Copy link
Member

commented Jul 19, 2013

@maning since this is now merged it's probably better to open new issues for any bugs rather than commenting here.

@maning

This comment has been minimized.

Copy link

commented Jul 19, 2013

OK, opened two separate issues (#347, #348)

@jfirebaugh

This comment has been minimized.

Copy link
Member Author

commented Jul 19, 2013

querystring.js is a browserified copy of component/querystring. It's on my list of things to clean up -- it should live in vendor, and I'd rather not introduce browserify here. Considering a vendorered jquery-deparam as a replacement.

@woodpeck

This comment has been minimized.

Copy link
Contributor

commented Sep 1, 2013

I would like to offer a "lesson learned" (more precisely: "lesson that should be learned") for this pull request. Many existing users were confused by the sudden loss of the "export" tab, the former functionality of which is now partly behind "share" and partly behind an export link on the left sidebar.

The lesson is: Our web site shouldn't be subjected to large changes without at least thinking of the current users and finding some way to communicate the change to them where it is not obvious. The removal of the "export" tab was such a non-obvious change. One possibility would have been to keep the export tab around as a dummy (for a while!) that would open a pop-up message explaining that the content had moved. Or a MOTD-style thing that explains the major changes of the past months. Whatever - just some sort of "transition strategy" for existing users. I understand that this is easier said than done especially in a multi-language web site. But there have been 8 questions to help.openstreetmap.org alone about the "lost" export tab, and for any user who dares to post a question there are probably ten others who just give up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
You can’t perform that action at this time.