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

Redraw restrictions panel when dragging sidebar #5502

Merged

Conversation

jguthrie100
Copy link
Contributor

Fixes #5474

I've put together a bit of a hacky fix for the restrictions editor not scaling properly with the sidebar being dragged.

Ideally, I'd call the redraw() method direction from the sidebar's drag handler, but can't figure out a way to get access to it efficiently, so instead I just add a dragging class when the drag starts and remove it when it stops.

The restrictions.js file then just sets a setInterval that checks whether the dragged class is present or not, and then redraws the panel every 50ms if it is.

The redraw interval can be changed easily enough (is 50ms too frequent?), but when dragging, the street graphics are chopped off so close to the border of the svg, that when dragging, the chopped-off street edge is still visible for a few ms until the panel redraws.

The only way I can think of getting around that is to render the junction so that the streets are initially drawn much further out past the svg border, meaning when the svg resizes, there is much more 'buffer' space before the street-end is reached. (if that makes sense?)

@jguthrie100
Copy link
Contributor Author

Got a laggy computer, but shows the general idea..

image

@quincylvania
Copy link
Collaborator

quincylvania commented Nov 19, 2018

Hi @jguthrie100! Thanks for working on this. It would be nice if it only responded during resizing. Otherwise the event is constantly firing. You may want to look at the onResize function that's called every time the window or the sidebar is resized, or the sidebar drag handler. It may be nice to add some sort of "sidebar did drag" event that the restriction editor can listen for.

@jguthrie100
Copy link
Contributor Author

I've changed the implementation so that the restrictions panel is redrawn everytime ui.onResize is called, however, it means that its called 1000+ times whilst dragging the bar even just a little bit across.

Do you know of any good coding practices to limit the call numbers (other than setInterval or having some rudimentary counter loop?) - unless its fine as is?

@jguthrie100 jguthrie100 force-pushed the redraw_restrictions_on_sidebar_drag branch from 2bed29d to c002ab8 Compare November 21, 2018 09:05
@jguthrie100
Copy link
Contributor Author

Improved the algorithm significantly. Now only redraws if the sidebar has moved above a specified threshold (in this case 2px - easily changed)

image

@jguthrie100 jguthrie100 force-pushed the redraw_restrictions_on_sidebar_drag branch from 703516d to 7bf4c46 Compare November 21, 2018 09:25
@jguthrie100
Copy link
Contributor Author

It may be nice to add some sort of "sidebar did drag" event that the restriction editor can listen for.

@quincylvania I'm a bit unsure what you mean by that?

And is there anything I can change to get this merged? The redraw threshold could probably be lowered to 1px if there are concerns about it being a bit jaggy, and I'm assuming most browsers would be able to handle it (considering the entire map is redrawn so frequently whilst dragging around the map!)

@quincylvania
Copy link
Collaborator

@quincylvania I'm a bit unsure what you mean by that?

I'm not sure I know what I mean 😂

It looks like you found a way to react to the changes without a timer and to avoid too many redraws, so it seems fine to me! I do think I'd prefer a 1px threshold unless we see real performance issues; we can always adjust it later.

As far as getting this merged, @bhousel can speak to that.

@bhousel
Copy link
Member

bhousel commented Nov 30, 2018

Thanks @jguthrie100 & @quincylvania - this looks pretty good to merge...
I might eventually switch over the custom event dispatch to a more idiomatic D3.js way of doing it, but for now it's cool..

@bhousel bhousel merged commit 8b0e729 into openstreetmap:master Nov 30, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Turn restriction editor should redraw when the sidebar resizes
3 participants