Don't drag draggables on scroll events - #29842
Conversation
Sorry, something went wrong.
|
Why do we need a custom picker / why does the regular picker fire on scroll events? Should it be fixed generally on the regular picker? |
Sorry, something went wrong.
|
A scroll event comes in looking just like a button press (but with a higher number). It does not seem unreasonable to allow these to trigger pick events in general but to disable them for draggable things specifically. |
Sorry, something went wrong.
|
I don't think I understand this: "Pick" is logically selecting an element by clicking on it. No matter the technical implementation, why should scrolling ever be interpreted as pick? In what use case would that make sense? |
Sorry, something went wrong.
|
pick is possibly an unfortunate choice of language. As an example of a picker that uses scroll, if I wanted to make an artist change size if the mouse is over it and scrolled, then giving a custom function to Artist.set_picker() would be the way to do this. |
Sorry, something went wrong.
timhoffm
left a comment
There was a problem hiding this comment.
Anybody can merge after adding the suggested documentation.
Sorry, something went wrong.
|
|
||
| @staticmethod | ||
| def _picker(artist, mouseevent): | ||
| return (artist.contains(mouseevent) and mouseevent.name != "scroll_event"), {} |
There was a problem hiding this comment.
| return (artist.contains(mouseevent) and mouseevent.name != "scroll_event"), {} | |
| """ | |
| A custom picker to prevent dragging on scroll events. | |
| Dragging is initialized through a pick events, which is typically a | |
| mouse click. However, mouse scrolling also fires pick events and | |
| we need to filter them out so that scrolling does not initalize | |
| dragging. | |
| """ | |
| return (artist.contains(mouseevent) and mouseevent.name != "scroll_event"), {} |
Sorry, something went wrong.
There was a problem hiding this comment.
👍 to adding a comment - I thought this was a bit verbose for a private helper function, so I added a one-liner instead - what do you think?
Sorry, something went wrong.
PR summary
Fixes #29142; replaces #29270. The solution here is to set a custom picker on DraggableBase that doesn't fire if the event is a scroll event.
PR checklist