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

Centralize drag handlers #2057

Closed
monfera opened this issue Oct 3, 2017 · 2 comments
Closed

Centralize drag handlers #2057

monfera opened this issue Oct 3, 2017 · 2 comments

Comments

@monfera
Copy link
Contributor

monfera commented Oct 3, 2017

Followup of #2052 (comment)

@alexcjohnson
Copy link
Collaborator

#2241 adjusted dragElement to take care of click events as well. This has a number of benefits: it simplifies all the click vs (drag)done logic, and ensures that you do exactly one of those - a mousedown/up must be either a click or a drag. It also allows us to capture right clicks. Some TODOs came up in that PR:

  • Any trace type that uses selection.on('click', clickHandler) would benefit from being converted to dragElement, for the above mentioned reasons. This includes at least geo, mapbox, parcoords, pie, sankey, table. geo has a couple of additional issue: right-click starts a pan/select, which then you have to click again to get out of; it also needs to have hover effects removed when you start selecting or panning.
  • Annotations have a plotly_annotationclick event that should also be extended to support right-click.
  • Possibly create a config parameter to manage right-click behavior. Right now the hacky behavior that some users depend on is that if you attach a contextmenu handler to gd, and call preventDefault() in it, this prevents the native context menu from appearing, and then a right click (for those traces using dragElement anyway) get a plotly_click event and the dev can look at button(s), ctrlKey, etc to determine what kind of a click it was. But we don't want this to always, as mostly devs won't be thinking about what kind of click they're getting, and folks may want to keep the native context menu accessible. A config option seems like a clean way to expose this.

@gvwilson
Copy link
Contributor

Hi - this issue has been sitting for a while, so as part of our effort to tidy up our public repositories I'm going to close it. If it's still a concern, we'd be grateful if you could open a new issue (with a short reproducible example if appropriate) so that we can add it to our stack. Cheers - @gvwilson

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

No branches or pull requests

3 participants