-
Notifications
You must be signed in to change notification settings - Fork 162
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
Improve React-PhyloTree interface #501
Conversation
@jameshadfield: I like this direction a lot. I think this is much clearer than On This is ready to merge from my perspective. @rneher: Let us know if you have any issues; you're much closer to |
👍 |
Instead of React reaching in and calling the appropriate prototype (
updateDistance
,updateMultipleArray
, etc), a single prototype is available -phylotree.change()
- which combines all requested changes (branch thicknesses, visibility, distance etc) into a single D3 call per class (".tip", ".branch" etc). This prevents bugs where multiple D3 update methods were called in quick succession and interfered with each other. It also improves the aesthetics of transitions as all changes are done in the same transition rather than usingsetTimeout
to effectively queue them up.phylotree.change() API examples
phylotree.change({newDistance: "num_date"})
phylotree.change({newLayout: "rect"})
phylotree.change({changeVisibility: true, visibility: <array>})
phylotree.change({changeColorBy: true, stroke: <array>, fill: <array>})
Furthermore, all of these can be combined into a single smooth transition, e.g.
phylotree.change({newDistance: "num_date", newLayout: "rect", changeVisibility: true, visibility: <array>, changeColorBy: true, stroke: <array>, fill: <array>})
Additional changes
computeResponsive
has been sped up as it no longer queries the DOM causing a forced reflow (stop computeResponsive causing a forced reflow #499)cc @trvrb @rneher. Online at auspice-dev.herokuapp.com.