Skip to content

Conversation

Ovgodd
Copy link
Collaborator

@Ovgodd Ovgodd commented Sep 16, 2025

Purpose

Fix tree node lazy-loading behavior when toggling via keyboard.
Previously, children loaded only if the node had already been expanded via mouse.

Proposal

  • Ensure useLoadChildrenOnOpen triggers fetch on first keyboard toggle
  • Prevent multiple fetches with hasLoadedRef

node expansion now triggers lazy loading even without prior mouse interaction

Signed-off-by: Cyril <c.gromoff@gmail.com>
@Ovgodd Ovgodd force-pushed the fix/tree-keyboard-expand-children branch from 2fda6a8 to 1fb0897 Compare September 16, 2025 11:17
@Ovgodd Ovgodd marked this pull request as ready for review September 16, 2025 11:18
Copy link

github-actions bot commented Sep 16, 2025

Size Change: +27 B (0%)

Total Size: 3.66 MB

Filename Size Change
apps/impress/out/_next/static/c3323b88/_buildManifest.js 0 B -889 B (removed) 🏆
apps/impress/out/_next/static/d6e74915/_buildManifest.js 887 B +887 B (new file) 🆕

compressed-size-action

Copy link
Collaborator

@AntoLC AntoLC left a comment

Choose a reason for hiding this comment

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

I think it is better to do the fix in the ui-kit repository as it will benefit for all: https://github.com/suitenumerique/ui-kit/

The problem comes from TreeViewItem component and how handleLoadChildren is loaded.
https://github.com/suitenumerique/ui-kit/blob/b93531e51a2c74527cd0773cb01292cb9a07be3d/src/components/tree-view/TreeViewItem.tsx#L131-L139

The onClick on the arrow should not trigger handleClick but set the node to open:

onClick={(e) => {
   e.stopPropagation();
   node.toggle();
 }}

We should then have a useEffect listening node.isOpen, when isOpen is at true, we trigger handleLoadChildren if not already done. By doing so, it will fix this issue, because the useEffect will be trigger when toggling with the keyboard.

Something like that:

useEffect(() => {
    if (isLeaf || hasLoadedChildren || node.data.value.hasLoadedChildren) {
      return;
    }

    setIsLoading(true);
    context?.treeData.handleLoadChildren(node.data.value.id).then(() =>
    setIsLoading(false));
  }, [node.isOpen]);

@Ovgodd
Copy link
Collaborator Author

Ovgodd commented Sep 18, 2025

I think it is better to do the fix in the ui-kit repository as it will benefit for all: https://github.com/suitenumerique/ui-kit/

The problem comes from TreeViewItem component and how handleLoadChildren is loaded. https://github.com/suitenumerique/ui-kit/blob/b93531e51a2c74527cd0773cb01292cb9a07be3d/src/components/tree-view/TreeViewItem.tsx#L131-L139

The onClick on the arrow should not trigger handleClick but set the node to open:

onClick={(e) => {
   e.stopPropagation();
   node.toggle();
 }}

We should then have a useEffect listening node.isOpen, when isOpen is at true, we trigger handleLoadChildren if not already done. By doing so, it will fix this issue, because the useEffect will be trigger when toggling with the keyboard.

Something like that:

useEffect(() => {
    if (isLeaf || hasLoadedChildren || node.data.value.hasLoadedChildren) {
      return;
    }

    setIsLoading(true);
    context?.treeData.handleLoadChildren(node.data.value.id).then(() =>
    setIsLoading(false));
  }, [node.isOpen]);

Ok I agree yes. Thanks for the detailed explanation !

@Ovgodd
Copy link
Collaborator Author

Ovgodd commented Sep 29, 2025

I’m closing this PR because the fix has been handled directly in the UI Kit.
This way, all applications using the UI Kit will benefit from it: #115
.

@Ovgodd Ovgodd closed this Sep 29, 2025
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.

2 participants