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

Fx component #1613

Merged
merged 7 commits into from
Apr 20, 2017
Merged

Fx component #1613

merged 7 commits into from
Apr 20, 2017

Conversation

etpinard
Copy link
Contributor

@etpinard etpinard commented Apr 20, 2017

to be merged in #1582

Introducing src/components/fx/ home to all things hover. As hover logic is becoming more and more centralized and less and less cartesian-only, moving this out of src/plots/cartesian/ is a no-brainer.

Down the road (maybe in this PR if easy) we should make Fx a optionally registerable component so that we can remove from our lib/core.js module, making for smaller custom bundles for users not caring about hover labels. Note that src/components/fx/hover.js is over 1000 LOC.

As a bonus, this reorganisation removed 4 5 circular dependencies from src/ (ref #236) 🎉


'use strict';

module.exports = {
Copy link
Contributor Author

@etpinard etpinard Apr 20, 2017

Choose a reason for hiding this comment

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

@alexcjohnson can you think of anything else that might be better off in here?

Copy link
Collaborator

Choose a reason for hiding this comment

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

nope, I think you got it all!

@alexcjohnson
Copy link
Collaborator

Nicely done! 💃

@etpinard
Copy link
Contributor Author

(maybe in this PR if easy) we should make Fx a optionally registerable component

Yep, this turned out to be easy in a96af57 and abcf1b0

@alexcjohnson
Copy link
Collaborator

Yep, this turned out to be easy in a96af57 and abcf1b0

And 🔪 yet another circular dep! 🍻

@etpinard etpinard merged commit 03554a5 into hoverlabel-custom Apr 20, 2017
@etpinard etpinard deleted the fx-component branch April 20, 2017 19:07
@etpinard etpinard mentioned this pull request Apr 21, 2017
3 tasks
etpinard added a commit that referenced this pull request Apr 25, 2017
- somehow `madge` identifies circular in an order-dependent way,
  bump back to pre PR #1613 value.
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.

2 participants