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

feat(@clayui/core): Add Collection Tree implementation #5045

Merged
merged 3 commits into from
Sep 19, 2022

Conversation

matuzalemsteles
Copy link
Member

@matuzalemsteles matuzalemsteles commented Aug 19, 2022

Closes #5030

This PR is a WIP of the Collection Tree implementation, as mentioned in the issue the hardest part here is getting the nested virtualization to have accessibility, I'm still working on it but I'm submitting a piece of work that adds some more features to the Collection implementation that are needed to use in TreeView.

  • Support to create unique key in tree
  • Add an API to configure a component to wrap the item
  • Adds an API to pass properties to the children function when it's a dynamic collection
  • Adds a property to configure a list of properties that should be omitted when passing item data to children function

These are necessary resources to use in the TreeView component, see the commit d4f8419 that shows how it is being used.

As mentioned in the RFC, most of these changes are just internal architecture changes to keep all components to the same standard both internally and usage, still no components are supported by the collection, follow issue #5032 for more details.

Over the next week I will be sending more commits that I intend to focus only on the implementation of virtualization for the tree, this will be very important for DropDown and TreeView for much more expressive performance gains in TreeView.

ToDo

  • Implement virtualization for collection tree
  • Investigate low test coverage for TreeView (After using the Collection component the coverage dropped)
  • Update TreeView documentation with virtualization feature

@matuzalemsteles
Copy link
Member Author

Well, I found out that the test failures were related to a reference issue of the itemContainer property as it was being created all the time this was causing a component-wide rendering and losing reference and would also cause performance issues, I fixed that and now tests are passing.

I've made some more advances with tree virtualization, I'm trying the strategy of using the nested useVirtualizer apparently maybe it will cause scroll conflicts and not being able to manage what should be visible and have a lot of problems controlling the nested mechanism, I'm maybe going with the strategy of writing our own nested virtualization system accessible to handle tree structures.

@matuzalemsteles
Copy link
Member Author

Well, I tried the nested virtualization strategy using the recent libs but it doesn't work very well because they don't communicate when it's nested, it breaks everything. I think I'm going back to the option to render the tree in list with virtualization for Tree, as it is implemented in other libs, I think we still managed to still maintain accessibility using some properties, like:

  • aria-level
  • aria-setsize & aria-posinset
  • role="treeitem

The problem with this is that it changes all the TreeView markup and this can break some implementations in the portal but I think it is something very valuable that is worth it because it would just update the corresponding classes and we will have a lot of performance gains regardless of the data size.

It is very likely that I will also have to rewrite a good part of the existing mechanisms of the TreeView, for example:

  • Rendering we will need to traverse the tree at render time we can use a Tree Walker strategy to turn parts of the tree into a list.
  • Keyboard interactions will have to be rewritten to identify parent and children otherwise.
  • The TreeView.Group component may not make sense anymore as it is a list with virtualization, we may still consider using it synthetically to simplify interactions and how the developers configure this is more intuitive when dealing with trees.

I'll deep dive into that this week and bring you some more information on the progress of this, I also hope I don't make any API changes or break anything other than the markup that we can't avoid here.

@matuzalemsteles
Copy link
Member Author

I spent quite a bit of time developing/research this but there are a lot of changes I would have to make so I'm guessing that adding this now would take more time than I thought and many internal changes to the TreeView would be affected but also changes to the API, for example TreeView.Group wouldn't make more sense it would also simplify the API because everything is a list internally for the TreeView, the time and effort in this might not be worth it now. I think this still has value but I think we can leave it for a major release so that we can simplify the API and have big performance gains when we start having real performance issues with the current TreeView.

I'll try to dig a little deeper into the things I've done so far and try to explain the things that we would have to change.

The main mechanism here is the tree walker which must have the ability to traverse the tree from one point to another, for example virtualization works over a range of indexes start: 0, end: 10 which are the visible items on the screen , being items root or children of some item and they change as the scroll changes.

tree walker

The example above tries to illustrate some of the diagramming of how this would work, the virtual scroll changes the range of visible indicies and the tree walker navigates through the tree and adds all items that are within that range to the list, the list data contains some properties like item depth level, parent and child, this helps to define accessibility properties for the list in the future.

  • If an item has children and is expanded, the tree walker must enter the item and add the items array and stop when it reaches the range's indicies limit.
  • If the scroll is moving the tree walker should be able to continue navigating through the tree without starting all over again, think of it as a paging cursor, a good strategy I used was to use generators, so we were able to navigate and stop and start from one point we want, this avoids performance problems at render time when there are thousands of items in a tree and navigation through the tree will be cheap without affecting performance or dropping frames.

The implementation of the cursor is more complicated because it can't be the leaf of the tree because it wouldn't know how to go back, maybe we could keep the cursor with the last item at level zero of the tree and the next iteration starts from that point, a strategy would be maintain a structure as a support of items corresponding to the leaf path, that is, the parent items of the leaf that allows you to go back and continue forward, this can help in moments when there is a large amount of items in depth and only they are visible and we don't need get the parent in the level zero and navigate to the corresponding subtree.

treeview-cursor

In the demo example above, moving the virtual scroll changes the range of items that should be visible, we use the cursor of the last iteration, the arrows used define how this flow would work from traversing the items in the tree until reaching the index limit.

  1. If the cursor has no children or is not expanded, fetches the parent in the support structure
  2. If the parent has children, loop through the children from startIndex and add the list
  3. If it has reached the end of the list of the current cursor and there is still space left in the list, fetches the parent of the cursor again
  4. ...

The flow follows the same idea, there are many edge cases in various scenarios here the assumption is that the cursor will always be present when the recomputation is triggered by the virtualizer because it calls with each scroll change, we can also save more information in the cursor like yours index to help you decide when to look for the parent and know which children to add or if the parent's children are not visible. It may be that I'm not covering all the scenarios here.

treeview-cursor-collapse

Another scenario is when the user collates an item with visible children, it follows the same rule recomputing the visible items using the cursor as a starting point, the cursor has its index of the last iteration so we know how many items are added before it or after it, so when looking for the cursor's parent and having children but the cursor is not the first in this list of items we can determine where to start.

A way also to avoid putting a leaf item from the tree as a cursor and putting its parent to always avoid the first search for the parent when it has no children.

The tree walker system is reactive, so even if an expand/collapse, remove an item from the tree, move the items will have no problem what can happen in these situations is the system restarts from zero when the cursor is lost.

The problem with this approach is that it can be very complex with many edge cases that can happen but it can also be simple because it is reactive, I haven't implemented all these details yet, I have a very simple system, I plan to organize and commit to push to this PR and keep that on hold for future work.

There is another strategy like turning the entire tree into a list and using some optimization strategies like slice time, traversing the tree within a deadline, when you don't have time to traverse wait for the next available frame, this is similar to how react-vtree does.

As a final part of this change from tree to list virtualization, some other changes would be needed.

  • <TreeView.Group /> and <TreeView.ItemStack /> would no longer be needed
  • Markup change to list instead of nested elements
  • Change the internal logic of nesting and expanding items

I'm still going to merge the commits I added initially using the base Collection in the TreeView but I'll send it in another PR to keep the history here for the full iteration of the tree virtualization.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add Collection implementation with virtualization for Tree
1 participant