noyainrain / listling

Web app to make and edit lists collaboratively.
https://listling.org/
GNU Affero General Public License v3.0
34 stars 8 forks source link

Improve scroll jumps when clicking on map locations #47

Closed dnnr closed 4 years ago

dnnr commented 5 years ago

When clicking the coordinates of a location entry, the current view is scrolled to ensure that the the point on the map is visible in the viewport.

However, the jump appears to be fixed such that the point on the map invariably ends up at the top of the view with a bit of margin. This can actually turn out be less useful than not scrolling all: If the map is already in full view, clicking the link might lead to scrolling down, unnecessarily obscuring the map.

So, instead of bringing the point on the map in view, it might be better to try to bring the entire map into view (keep highlighting the point, of course). If the map is already fully visible, the view should not be changed at all.

One edge case I'm not sure about: If the map is not fully visible, but the point on the map happens to be, would it be better not to scroll at all, or better bring the map fully into view as well?

noyainrain commented 5 years ago

Good catch, thanks for reporting! :blush:

I agree that bringing the whole map into view, instead of a single location, is the more sensible behavior.

This should be achievable with a small change: <micro-map> already handles the navigate event (we adjust the zoom, so all markers are visible); there we can additionally scrollIntoView() the map.

I am currently working on another feature, if you would like to speed things up, this should be a quick pull request :smile: .


Concerning the view should not change if the map is already fully visible: Scrolling always is the default browser behavior for in-page navigation. For example, clicking on a link to a location/map and to a list item is consistent if we scroll always.

That said, I am not opposed to scrolling only-if-needed, but would then prefer to implement it for in-page navigation in general. scrollMode: "if-needed" might find its way into browsers, which would save us from implementing our own logic.


The relevant code is part of micro, so I've created an issue to track it there: noyainrain/micro#71

noyainrain commented 5 years ago

PS: A related problem, that the header may obscure parts of an element after navigating / scrolling to it, is tracked here: noyainrain/micro#70

noyainrain commented 4 years ago

Fixed via noyainrain/micro#71. Thanks for the contribution @hlisdero! :blush: