Skip to content

Fix emitting event "plotly_relayouting" with wrong data #3977

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

Closed
wants to merge 1 commit into from

Conversation

maun
Copy link

@maun maun commented Jun 21, 2019

Currently "plotly_relayouting" events are emitted when creating a zoombox. These events use the updates to the layout which would be applied when zooming. Since no relayouting takes place until the zoombox is applied I removed the event.

Although a new update is computed it is not applied until zooming using the zoombox is finished.
@maun
Copy link
Author

maun commented Jun 21, 2019

To fix the test I could remove them or set the expected relayouting count to zero. An alternative would be to change the event to something else, eg "plotly_zoombox-resized". This would be enable tests like ('handles y-only to xy back to y-only in single zoombox drag motion'). What would you suggest?

@alexcjohnson
Copy link
Collaborator

@maun plotly_relayouting events are not intended to represent actual changes to the layout - that's what plotly_relayout events are for. These events are intended for live updates before the change has actually been applied to the plot. See #3888 and #2082

@maun maun closed this Jun 24, 2019
@maun
Copy link
Author

maun commented Jun 24, 2019

@alexcjohnson I implemented my own range slider and wanted to synchronise the movement with the panning on the graph. This works by using plotly_relayouting , but fails when using the zoombox since this also emits this event.

@jonmmease
Copy link
Contributor

@alexcjohnson I would agree with @maun that this behavior is a bit surprising. The main usecase that I've had in mind for plotly_relayouting is 2D Histogram resampling (http://datashader.org/ from Python, but could also be pure JavaScript). During pan actions I would want to be able to process the entire viewport at each moment during the pan (not only at the end when plotly_relayout is emitted), but it would not make sense for this usecase to process the zoombox region only during box zoom.

Is there anything in the event data to help distinguish between pan and box zoom?

@maun
Copy link
Author

maun commented Jun 24, 2019

@jonmmease Unfortunately not, the event is the same. If you agree to change this somehow by renaming the zoombox event or adding another property I would update my PR.

@alexcjohnson
Copy link
Collaborator

That makes sense, thanks for clarifying. I don't think we want to remove zoombox from this event entirely, I can still imagine uses for it. But labeling it so you could choose to ignore zoombox or whichever ones you don't want seems like a good solution. It's a little weird as we only have the relayout object, but can we just add an interactionType key to that object? @etpinard @antoinerg does that seem reasonable?

@jonmmease
Copy link
Contributor

can we just add an interactionType key to that object?

How about _interactionType so that it's clear that this isn't part of the schema?

@etpinard
Copy link
Contributor

Wouldn't

gd.on('plotly_relayouting', function(d) {
  gd._fullLayout.dragmode // => 'pan' or 'zoom'
})

also give you the right answer?

@maun
Copy link
Author

maun commented Jun 26, 2019

@etpinard Just tried this, for me always "pan" is returned. Probably because I set the default dragmode to panning.

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.

4 participants