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

gl2d non-visible traces improvements #1300

Merged
merged 4 commits into from Jan 18, 2017
Merged

Conversation

etpinard
Copy link
Contributor

fixes #1298

Previously scattergl traces with visible: false or visible: 'legendonly' weren't filtered out of the convert step where several methods assumed non-empty trace.x and trace.y.

This PR add a third scattergl update path (along side the current fancy and fast paths) for visible !== true traces where gl-vis objects are updated with Float62Array(0) as data.

- to optimize 'visible' toggling
- and fix a few bugs with data-less scattergl traces
- to make sure they don't make Plotly.plot crash
@etpinard etpinard added this to the v1.22.0 milestone Jan 13, 2017
@etpinard etpinard changed the title gl2d non visible trace gl2d non-visible traces Jan 13, 2017
@etpinard etpinard changed the title gl2d non-visible traces gl2d non-visible traces improvements Jan 13, 2017
"x": []
},
{
"type": "scattergl"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and one each that has data but visible: false || 'legendonly'?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done in 2563f17

@alexcjohnson
Copy link
Contributor

How hard would it be to test toggling one of these traces on/off, maybe by clicking the legend? Might be overkill given the tests you do have...

@etpinard
Copy link
Contributor Author

How hard would it be to test toggling one of these traces on/off,

Not hard. At least it we only test the corresponding Plotly.restyle call (as clicking on a legend button would require a setTimeout as there's no way to know when the Promise is resolved. Oh wait, I guess we could listen to 'plotly_restyle'. Oh well, I hope you find the restyle tests satisfactory).

We already have one for gl3d here.

I added one for gl2d in a587f8d

@alexcjohnson
Copy link
Contributor

Those tests are fine. No need to do an actual click.
💃

@etpinard etpinard merged commit e2105d2 into master Jan 18, 2017
@etpinard etpinard deleted the gl2d-not-visible-optimization branch January 18, 2017 14:19
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.

Initializing scattergl with empty traces throws TypeError
2 participants