-
Notifications
You must be signed in to change notification settings - Fork 41
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
feat: GroupTreePlot --- Refactor to typescript + better D3 and React intergation + some small features #2367
base: master
Are you sure you want to change the base?
feat: GroupTreePlot --- Refactor to typescript + better D3 and React intergation + some small features #2367
Conversation
Convert this comment to an issue when this is merged |
Resolves #1829
refactor
: Fully rewrites the old D3-onlyGroupTreePlot
component to more seemlessly use both D3 and React to build the graphdependency
: Installed thereact-transition-group
package to facilitate leave-enter transitions for graph elementsrefactor
:groupTreeAssembler
has been replaced; it has been split it into by react components (to handle svg rendering), and the newDataAssembler
class, to handle the data itself. Rewritten in proper Typescriptfix
: The rendered tree is now more centered in the SVGfix
: The tree will now fill the entire width of the SVGfeature/fix
: Child nodes can now be minimized and expanded, as intendedfeature
: AddedinitialVisibleDepth
; renders the tree with all nodes from the given depth collapsedfeature
: The plot is now fully resizable./?path=/story/grouptreeplot-demo--resizable
)Known issue (slight animation regression)
Because each component initializes their own enter/leave animations in their enter/leave-callbakcs, the D3 animations get slightly de-synchronized, so new/old tree edges get slightly "detached" from the tree when it grows shrinks (only during the animation). The d3 only version did not have this issue (as it set up all animations in one single update), so it's a regression.
However, this is only noticable in slower animations, and with a 200ms transition duration, it's more or less impossible to notice, so I'm leaving it unresolved for now