refactor(component): refactor Tree component API to StatefulTree #481
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jorgemoya
reviewed
Dec 11, 2020
chanceaclark
force-pushed
the
refactor/tree
branch
from
df85e5f
to
aeb59b3
Compare
chanceaclark
changed the title
jorgemoya
approved these changes
Dec 15, 2020
rtalvarez
reviewed
Dec 15, 2020
BREAKING CHANGE: `Tree` component and now `StatefulTree` with new API changes. See dev docs for new API.
chanceaclark
force-pushed
the
refactor/tree
branch
from
aeb59b3
to
78f7a21
Compare
|
Btw did you check the profiler to get an overall feel on the re-rendering performance of this component? Some real usecases for this component might get a bit heavy (eg category trees) |
|
@rtalvarez I did and it's going to rerender frequently due to the use of the context API. I had some discussions with @deini about this, and the only real solution to circumvent this use a state library (Redux/RxJS/etc.). We may need to find a solution for this for all of our stateful/complex components in the future. I'll keep this PR open a bit longer for comments, and in the meantime I will stress test it with a larger dataset. |
deini
reviewed
Dec 16, 2020
packages/big-design/src/components/StatefulTree/hooks/useSelectable.ts
Outdated
Show resolved
Hide resolved
packages/big-design/src/components/StatefulTree/hooks/useSelectable.ts
Outdated
Show resolved
Hide resolved
packages/big-design/src/components/StatefulTree/hooks/useSelectable.ts
Outdated
Show resolved
Hide resolved
packages/big-design/src/components/StatefulTree/hooks/useVisibleNodes.ts
Outdated
Show resolved
Hide resolved
deini
approved these changes
Dec 16, 2020
chanceaclark
force-pushed
the
refactor/tree
branch
from
7ee5e7e
to
5da9387
Compare
chanceaclark
force-pushed
the
refactor/tree
branch
from
5da9387
to
1480c89
Compare
This was referenced Jan 4, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What
Refactors
Treecomponent to be a more internal component and adds a newStatefulTreecomponent.Why
The old
TreeAPI dealt with a lot of the state management and caused use usage issues:Treecomponent due to some state management requirements.onClickhandlers were being requested for certain DOM interactions.Notes
Treecomponent anymore until we find we need more granular usages (async node fetches, complex node manipulation, etc...). Until that point in time, you should be usingStatefulTree.Treecomponent is now in a good state that we can add drag-n-drop functionality. This functionality will be for another PR.ping @bigcommerce/frontend