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

Add dragmode: false #3170

Merged
merged 6 commits into from
Nov 9, 2018
Merged

Add dragmode: false #3170

merged 6 commits into from
Nov 9, 2018

Conversation

jonfunkhouser
Copy link
Contributor

This PR adds the dragmode: false functionality mentioned in #2844 (comment)

I added an example chart with this setting to my existing sample, so this can be previewed on mobile devices: https://jonfunkhouser.github.io/plotly-mobile/

@etpinard
Copy link
Contributor

etpinard commented Oct 31, 2018

Thanks very much for this PR and let me apologize for the delayed response (our 1.42.0 release cycle was a little hectic).

Could you confirm that hover still works when dragmode: false?

@jonfunkhouser
Copy link
Contributor Author

Sorry, I was away last week. Yes, hover still works in dragMode: false. I updated the sample site to include plotly_hover debug output.

@etpinard
Copy link
Contributor

etpinard commented Nov 5, 2018

Thanks @jonfunkhouser !

peek 2018-11-05 18-13

I wonder if we could do better. By dragging we effectively halt hover and the hover labels (if any) remain in place. Maybe instead we could treat mouse "drag" as a mouse "move" under dragmode: false. But that might make things too confusing though.


Regardless, we'll need to add a test to 🔒 down the hover on dragmode: false behavior. @jonfunkhouser would you be interested?

@jonfunkhouser
Copy link
Contributor Author

Yes, I'll add a test for that case. I'll also investigate your suggestion, it makes a lot of sense.

‘mousemove’ events should emit ‘plotly_hover’ events
@etpinard etpinard added this to the v1.43.0 milestone Nov 6, 2018
@etpinard
Copy link
Contributor

etpinard commented Nov 9, 2018

Brilliant PR, this will be part of our next minor release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants