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

5291: recalculation of dynamic CSS transforms on hover #5302

Merged
merged 12 commits into from
Nov 27, 2020
Merged

5291: recalculation of dynamic CSS transforms on hover #5302

merged 12 commits into from
Nov 27, 2020

Conversation

alexhartstone
Copy link
Contributor

Adds a conditional transform recalculation method to plot api, and calls it in interactions

Copy link
Contributor

@archmoj archmoj left a comment

Choose a reason for hiding this comment

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

Thanks very much for the follow up PR.
Please find my comments below.

@archmoj archmoj added status: in progress bug something broken community community contribution feature something new labels Nov 25, 2020
@alexhartstone
Copy link
Contributor Author

Currently changing tests slightly to check the changes work. Having some issues with the polar tests failing when they shouldn't, will investigate.

@archmoj
Copy link
Contributor

archmoj commented Nov 26, 2020

Here is a demo using the changes made by this branch.
Nice job @alexhartstone 🥇
This looks pretty close if you adjust tests and address syntax errors.
To catch syntax errors before each commit you could simply run the command below:
npm run test-syntax

@archmoj archmoj added this to the v1.58.0 milestone Nov 26, 2020
@alexhartstone
Copy link
Contributor Author

Fixing up syntax errors now - npm run test-syntax doesn't seem to be picking up a lot of my syntax errors, weirdly. I've installed eslint and that seems to get it

@archmoj
Copy link
Contributor

archmoj commented Nov 27, 2020

Have you tested hover on mapbox traces?
You may need to recalculate the transform from src/plots/mapbox/mapbox.js considering changes in #5193.

@archmoj
Copy link
Contributor

archmoj commented Nov 27, 2020

Have you tested hover on mapbox traces?
You may need to recalculate the transform from src/plots/mapbox/mapbox.js considering changes in #5193.

Nevermind. mapbox hovers works as expected.

@archmoj archmoj changed the title 5291: recalculation of transforms on hover 5291: recalculation of dynamic CSS transforms on hover Nov 27, 2020
@archmoj
Copy link
Contributor

archmoj commented Nov 27, 2020

Thanks @alexhartstone
💃

@archmoj archmoj merged commit cdd836c into plotly:master Nov 27, 2020
@archmoj archmoj deleted the 5291-dynamic-transforms branch November 27, 2020 16:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug something broken community community contribution
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants