Skip to content

Tree view component is added#387

Merged
HanjoHagemeierHCL merged 17 commits into
HCL-TECH-SOFTWARE:mainfrom
MuhammedShabeelk:tree-view
Jun 3, 2026
Merged

Tree view component is added#387
HanjoHagemeierHCL merged 17 commits into
HCL-TECH-SOFTWARE:mainfrom
MuhammedShabeelk:tree-view

Conversation

@MuhammedShabeelk
Copy link
Copy Markdown
Contributor

@MuhammedShabeelk MuhammedShabeelk commented May 14, 2026

Screenshot 2026-05-18 at 7 09 13 PM Screenshot 2026-05-18 at 3 56 16 PM

@HanjoHagemeierHCL HanjoHagemeierHCL self-requested a review May 18, 2026 15:27
Copy link
Copy Markdown
Collaborator

@HanjoHagemeierHCL HanjoHagemeierHCL left a comment

Choose a reason for hiding this comment

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

Hey @MuhammedShabeelk, first check looks promising. But I think you have forgotten to implement some unit tests.

https://github.com/HCL-TECH-SOFTWARE/enchanted-react-components/tree/main/src/__tests__/unit

Also do you have a proper design guideline form UIUX for this component and was this checked by someone form UI/UX?

@MuhammedShabeelk
Copy link
Copy Markdown
Contributor Author

Hey @MuhammedShabeelk, first check looks promising. But I think you have forgotten to implement some unit tests.

https://github.com/HCL-TECH-SOFTWARE/enchanted-react-components/tree/main/src/__tests__/unit

Also do you have a proper design guideline form UIUX for this component and was this checked by someone form UI/UX?

will add unit test now and update it.
it is also submitted for the UX review and thel added some comments. i have addressed all the comments and submitted again.

figma link: https://www.figma.com/design/kAQhfivZRNUUOLiwI383HC/HCLED-Redline-Doc-V2.1?node-id=13278-75938&p=f&t=X7idTGxumx1QTeSM-0

@MuhammedShabeelk
Copy link
Copy Markdown
Contributor Author

Unit tests added and UX review is completed.

Copy link
Copy Markdown
Collaborator

@HanjoHagemeierHCL HanjoHagemeierHCL left a comment

Choose a reason for hiding this comment

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

Hey @MuhammedShabeelk, I the RTL mode it looks a bit odd. Can you check this again?

Image

Maybe it would be also senseful to provide a global disable flag for the whole treeview comp, like accordion one.

Can you please also check that all aria attributes are correct.

@HanjoHagemeierHCL
Copy link
Copy Markdown
Collaborator

GitHub Copilot Review Comments


Summary

This PR promotes the TreeView component from src/hidden_components/TreeView/ to src/TreeView/, replacing the previous thin MUI wrappers with a fully featured, custom implementation. The changes span +1424 / -116 lines across 9 files:

  • New: TreeItem.tsx (371 lines), TreeView.tsx (350 lines), TreeView.stories.tsx (394 lines), TreeView.test.tsx (302 lines)
  • Moved/renamed: index.ts (from hidden_components/TreeView)
  • Deleted: Old TreeItem.tsx, TreeView.tsx, TreeView.stories.tsx from hidden_components/
  • Modified: CHANGELOG.md, src/index.ts, src/theme/index.ts

Key features added: custom icon/status/detail/action slots, level-line rendering, keyboard navigation for action buttons, hover-reveal actions, theme overrides, and showLevelLine toggle.


Suggestions for Improvement

  1. CHANGELOG formatting issue — The CHANGELOG entry appears malformed:

    +-Added the `TreeView` component.
    

    Should be:

    - Added the `TreeView` component.
    
  2. RTL support — hardcoded left/paddingLeft — The level-line and content padding use LTR-only CSS properties:

    // TreeItem.tsx
    const contentPaddingLeft = depth > 0 ? 4 + depth * 8 : undefined;
    const lineLeft = 11 + depth * 8;
    // ...
    left: `${lineLeft}px`,

    These should use logical properties (insetInlineStart, paddingInlineStart) to support RTL mode correctly. This aligns with the existing review comment about RTL looking odd.

  3. Magic numbers — Spacing values 4, 8, 11, 3px, 28px, 10px are scattered throughout TreeItem.tsx and TreeView.tsx. Consider extracting them as named constants (e.g., LEVEL_INDENT_PX, ITEM_MIN_HEIGHT, SELECTION_INDICATOR_WIDTH) for maintainability.

  4. Unused dependency in useCallback — In TreeItem.tsx, handleActionKeyDown lists props.nodeId in its dependency array but never uses it in the callback body:

    }, [focusTree, navigateToNextItemAction, props.nodeId]);

    Remove props.nodeId from the dependency array.

  5. MutationObserver per TreeItem — Each TreeItem creates a MutationObserver to detect the Mui-focused class. For large trees (100+ items), this could have performance implications. Consider lifting this to the TreeView level with a single observer or using the existing onNodeFocus callback.

  6. Window-level event listenersTreeView.tsx attaches keydown and mousedown listeners to window for keyboard-vs-mouse detection. These fire on every event globally. Consider scoping to the tree container or using event delegation.

  7. Hardcoded colors in stories — The StatusBadge component uses raw hex colors:

    <CustomIconUserStatusActive style={{ fontSize: 16, color: '#15D36E' }} sx={{ '& path': { stroke: '#07432F' } }} />

    For a design system component library, consider using theme palette tokens instead.

  8. Missing global disabled prop on TreeView — Individual TreeItem nodes can be disabled, but there's no way to disable the entire tree with a single prop. Consider adding a disabled prop to EnhancedTreeViewProps (similar to Accordion).

  9. hoverActions accessibility — Hover actions use visibility: hidden and opacity: 0 by default, making them invisible to screen readers until hover/focus. Consider using aria-hidden toggling or ensuring keyboard users can always reach these actions.


Potential Issues

  1. requestAnimationFrame for synthetic keyboard dispatch — In handleActionKeyDown, arrow/Home/End keys are handled by focusing an element, then using requestAnimationFrame to dispatch a synthetic KeyboardEvent:

    requestAnimationFrame(() => {
      treeUl.dispatchEvent(new KeyboardEvent('keydown', { key, bubbles: true, cancelable: true }));
    });

    This pattern can be brittle — if MUI's internal event handling changes or if React's synthetic event system interferes, navigation could break silently. Consider adding integration tests for keyboard navigation paths.

  2. @mui/lab dependencyTreeView and TreeItem are imported from @mui/lab, which has been deprecated in favor of @mui/x-tree-view in MUI v6+. While this is fine for the current MUI version, it's a future migration concern worth documenting.

  3. Ref type assertion — The setRef callback in TreeItem.tsx uses a type assertion:

    (ref as React.MutableRefObject<HTMLLIElement | null>).current = node;

    If a callback ref is passed, this branch won't execute, which is fine — but the assertion could mask bugs. A safer pattern:

    else if (ref && 'current' in ref) ref.current = node;
  4. Descendant selected styling uses hardcoded rgba — In the theme overrides:

    backgroundColor: 'rgba(5, 80, 220, 0.04)',

    This should use theme.palette.action.selectedOpacity or equivalent token to remain consistent across theme modes (light/dark).

  5. Test coverage gaps — The unit tests cover rendering and basic interactions well, but the following scenarios are missing:

    • Keyboard navigation (Arrow keys, Home/End, Tab through actions)
    • RTL rendering
    • Hover actions visibility on hover/focus
    • detailsAlign="end" layout positioning
    • Multi-select with keyboard (Ctrl+click)
    • showLevelLine default behavior validation (currently only tests explicit true/false)

Best Practices Followed

  • License headers present on all new files with correct 2026 copyright year
  • TypeScript interfaces well-defined (EnhancedTreeItemProps, EnhancedTreeViewProps, TreeViewContextValue)
  • forwardRef correctly used on both TreeView and TreeItem
  • Context API used appropriately for depth tracking and keyboard state
  • Theme integration follows the established getMui*ThemeOverrides pattern
  • Storybook includes both interactive (with controls) and visual test templates
  • Export structure follows project conventions with re-exports from index
  • Clean migration from hidden_components — old files deleted, exports updated in src/index.ts

Overall:
This is a solid implementation of a feature-rich TreeView component. The main areas needing attention before merge are: (1) RTL support using logical CSS properties, (2) removing unused props.nodeId from the dependency array, (3) fixing the CHANGELOG formatting, and (4) replacing hardcoded rgba with theme tokens. The keyboard navigation implementation is ambitious and works, but would benefit from integration tests given its reliance on requestAnimationFrame and synthetic event dispatch. Adding a global disabled prop and improving accessibility for hover actions would bring this in line with other Enchanted components.

@MuhammedShabeelk
Copy link
Copy Markdown
Contributor Author

addressed all the review comments.

@HanjoHagemeierHCL
Copy link
Copy Markdown
Collaborator

GitHub Copilot Review Comments

Summary

This PR promotes the TreeView component from src/hidden_components/TreeView/ to src/TreeView/, replacing the previous thin MUI wrappers with a fully featured, custom implementation. The changes span +1424 / -116 lines across 9 files:

  • New: TreeItem.tsx (371 lines), TreeView.tsx (350 lines), TreeView.stories.tsx (394 lines), TreeView.test.tsx (302 lines)
  • Moved/renamed: index.ts (from hidden_components/TreeView)
  • Deleted: Old TreeItem.tsx, TreeView.tsx, TreeView.stories.tsx from hidden_components/
  • Modified: CHANGELOG.md, src/index.ts, src/theme/index.ts

Key features added: custom icon/status/detail/action slots, level-line rendering, keyboard navigation for action buttons, hover-reveal actions, theme overrides, and showLevelLine toggle.

Suggestions for Improvement

  1. CHANGELOG formatting issue — The CHANGELOG entry appears malformed:

    +-Added the `TreeView` component.
    

    Should be:

    - Added the `TreeView` component.
    
  2. RTL support — hardcoded left/paddingLeft — The level-line and content padding use LTR-only CSS properties:

    // TreeItem.tsx
    const contentPaddingLeft = depth > 0 ? 4 + depth * 8 : undefined;
    const lineLeft = 11 + depth * 8;
    // ...
    left: `${lineLeft}px`,

    These should use logical properties (insetInlineStart, paddingInlineStart) to support RTL mode correctly. This aligns with the existing review comment about RTL looking odd.

  3. Magic numbers — Spacing values 4, 8, 11, 3px, 28px, 10px are scattered throughout TreeItem.tsx and TreeView.tsx. Consider extracting them as named constants (e.g., LEVEL_INDENT_PX, ITEM_MIN_HEIGHT, SELECTION_INDICATOR_WIDTH) for maintainability.

  4. Unused dependency in useCallback — In TreeItem.tsx, handleActionKeyDown lists props.nodeId in its dependency array but never uses it in the callback body:

    }, [focusTree, navigateToNextItemAction, props.nodeId]);

    Remove props.nodeId from the dependency array.

  5. MutationObserver per TreeItem — Each TreeItem creates a MutationObserver to detect the Mui-focused class. For large trees (100+ items), this could have performance implications. Consider lifting this to the TreeView level with a single observer or using the existing onNodeFocus callback.

  6. Window-level event listenersTreeView.tsx attaches keydown and mousedown listeners to window for keyboard-vs-mouse detection. These fire on every event globally. Consider scoping to the tree container or using event delegation.

  7. Hardcoded colors in stories — The StatusBadge component uses raw hex colors:

    <CustomIconUserStatusActive style={{ fontSize: 16, color: '#15D36E' }} sx={{ '& path': { stroke: '#07432F' } }} />

    For a design system component library, consider using theme palette tokens instead.

  8. Missing global disabled prop on TreeView — Individual TreeItem nodes can be disabled, but there's no way to disable the entire tree with a single prop. Consider adding a disabled prop to EnhancedTreeViewProps (similar to Accordion).

  9. hoverActions accessibility — Hover actions use visibility: hidden and opacity: 0 by default, making them invisible to screen readers until hover/focus. Consider using aria-hidden toggling or ensuring keyboard users can always reach these actions.

Potential Issues

  1. requestAnimationFrame for synthetic keyboard dispatch — In handleActionKeyDown, arrow/Home/End keys are handled by focusing an element, then using requestAnimationFrame to dispatch a synthetic KeyboardEvent:

    requestAnimationFrame(() => {
      treeUl.dispatchEvent(new KeyboardEvent('keydown', { key, bubbles: true, cancelable: true }));
    });

    This pattern can be brittle — if MUI's internal event handling changes or if React's synthetic event system interferes, navigation could break silently. Consider adding integration tests for keyboard navigation paths.

  2. @mui/lab dependencyTreeView and TreeItem are imported from @mui/lab, which has been deprecated in favor of @mui/x-tree-view in MUI v6+. While this is fine for the current MUI version, it's a future migration concern worth documenting.

  3. Ref type assertion — The setRef callback in TreeItem.tsx uses a type assertion:

    (ref as React.MutableRefObject<HTMLLIElement | null>).current = node;

    If a callback ref is passed, this branch won't execute, which is fine — but the assertion could mask bugs. A safer pattern:

    else if (ref && 'current' in ref) ref.current = node;
  4. Descendant selected styling uses hardcoded rgba — In the theme overrides:

    backgroundColor: 'rgba(5, 80, 220, 0.04)',

    This should use theme.palette.action.selectedOpacity or equivalent token to remain consistent across theme modes (light/dark).

  5. Test coverage gaps — The unit tests cover rendering and basic interactions well, but the following scenarios are missing:

    • Keyboard navigation (Arrow keys, Home/End, Tab through actions)
    • RTL rendering
    • Hover actions visibility on hover/focus
    • detailsAlign="end" layout positioning
    • Multi-select with keyboard (Ctrl+click)
    • showLevelLine default behavior validation (currently only tests explicit true/false)

Best Practices Followed

  • License headers present on all new files with correct 2026 copyright year
  • TypeScript interfaces well-defined (EnhancedTreeItemProps, EnhancedTreeViewProps, TreeViewContextValue)
  • forwardRef correctly used on both TreeView and TreeItem
  • Context API used appropriately for depth tracking and keyboard state
  • Theme integration follows the established getMui*ThemeOverrides pattern
  • Storybook includes both interactive (with controls) and visual test templates
  • Export structure follows project conventions with re-exports from index
  • Clean migration from hidden_components — old files deleted, exports updated in src/index.ts

Overall: This is a solid implementation of a feature-rich TreeView component. The main areas needing attention before merge are: (1) RTL support using logical CSS properties, (2) removing unused props.nodeId from the dependency array, (3) fixing the CHANGELOG formatting, and (4) replacing hardcoded rgba with theme tokens. The keyboard navigation implementation is ambitious and works, but would benefit from integration tests given its reliance on requestAnimationFrame and synthetic event dispatch. Adding a global disabled prop and improving accessibility for hover actions would bring this in line with other Enchanted components.

Hey @MuhammedShabeelk, do you have checked the points from the Github Copilot review?

I have only check the backgroundColor one, but this is not fixed...

image

@MuhammedShabeelk
Copy link
Copy Markdown
Contributor Author

GitHub Copilot Review Comments

Summary

This PR promotes the TreeView component from src/hidden_components/TreeView/ to src/TreeView/, replacing the previous thin MUI wrappers with a fully featured, custom implementation. The changes span +1424 / -116 lines across 9 files:

  • New: TreeItem.tsx (371 lines), TreeView.tsx (350 lines), TreeView.stories.tsx (394 lines), TreeView.test.tsx (302 lines)
  • Moved/renamed: index.ts (from hidden_components/TreeView)
  • Deleted: Old TreeItem.tsx, TreeView.tsx, TreeView.stories.tsx from hidden_components/
  • Modified: CHANGELOG.md, src/index.ts, src/theme/index.ts

Key features added: custom icon/status/detail/action slots, level-line rendering, keyboard navigation for action buttons, hover-reveal actions, theme overrides, and showLevelLine toggle.

Suggestions for Improvement

  1. CHANGELOG formatting issue — The CHANGELOG entry appears malformed:

    +-Added the `TreeView` component.
    

    Should be:

    - Added the `TreeView` component.
    
  2. RTL support — hardcoded left/paddingLeft — The level-line and content padding use LTR-only CSS properties:

    // TreeItem.tsx
    const contentPaddingLeft = depth > 0 ? 4 + depth * 8 : undefined;
    const lineLeft = 11 + depth * 8;
    // ...
    left: `${lineLeft}px`,

    These should use logical properties (insetInlineStart, paddingInlineStart) to support RTL mode correctly. This aligns with the existing review comment about RTL looking odd.

  3. Magic numbers — Spacing values 4, 8, 11, 3px, 28px, 10px are scattered throughout TreeItem.tsx and TreeView.tsx. Consider extracting them as named constants (e.g., LEVEL_INDENT_PX, ITEM_MIN_HEIGHT, SELECTION_INDICATOR_WIDTH) for maintainability.

  4. Unused dependency in useCallback — In TreeItem.tsx, handleActionKeyDown lists props.nodeId in its dependency array but never uses it in the callback body:

    }, [focusTree, navigateToNextItemAction, props.nodeId]);

    Remove props.nodeId from the dependency array.

  5. MutationObserver per TreeItem — Each TreeItem creates a MutationObserver to detect the Mui-focused class. For large trees (100+ items), this could have performance implications. Consider lifting this to the TreeView level with a single observer or using the existing onNodeFocus callback.

  6. Window-level event listenersTreeView.tsx attaches keydown and mousedown listeners to window for keyboard-vs-mouse detection. These fire on every event globally. Consider scoping to the tree container or using event delegation.

  7. Hardcoded colors in stories — The StatusBadge component uses raw hex colors:

    <CustomIconUserStatusActive style={{ fontSize: 16, color: '#15D36E' }} sx={{ '& path': { stroke: '#07432F' } }} />

    For a design system component library, consider using theme palette tokens instead.

  8. Missing global disabled prop on TreeView — Individual TreeItem nodes can be disabled, but there's no way to disable the entire tree with a single prop. Consider adding a disabled prop to EnhancedTreeViewProps (similar to Accordion).

  9. hoverActions accessibility — Hover actions use visibility: hidden and opacity: 0 by default, making them invisible to screen readers until hover/focus. Consider using aria-hidden toggling or ensuring keyboard users can always reach these actions.

Potential Issues

  1. requestAnimationFrame for synthetic keyboard dispatch — In handleActionKeyDown, arrow/Home/End keys are handled by focusing an element, then using requestAnimationFrame to dispatch a synthetic KeyboardEvent:

    requestAnimationFrame(() => {
      treeUl.dispatchEvent(new KeyboardEvent('keydown', { key, bubbles: true, cancelable: true }));
    });

    This pattern can be brittle — if MUI's internal event handling changes or if React's synthetic event system interferes, navigation could break silently. Consider adding integration tests for keyboard navigation paths.

  2. @mui/lab dependencyTreeView and TreeItem are imported from @mui/lab, which has been deprecated in favor of @mui/x-tree-view in MUI v6+. While this is fine for the current MUI version, it's a future migration concern worth documenting.

  3. Ref type assertion — The setRef callback in TreeItem.tsx uses a type assertion:

    (ref as React.MutableRefObject<HTMLLIElement | null>).current = node;

    If a callback ref is passed, this branch won't execute, which is fine — but the assertion could mask bugs. A safer pattern:

    else if (ref && 'current' in ref) ref.current = node;
  4. Descendant selected styling uses hardcoded rgba — In the theme overrides:

    backgroundColor: 'rgba(5, 80, 220, 0.04)',

    This should use theme.palette.action.selectedOpacity or equivalent token to remain consistent across theme modes (light/dark).

  5. Test coverage gaps — The unit tests cover rendering and basic interactions well, but the following scenarios are missing:

    • Keyboard navigation (Arrow keys, Home/End, Tab through actions)
    • RTL rendering
    • Hover actions visibility on hover/focus
    • detailsAlign="end" layout positioning
    • Multi-select with keyboard (Ctrl+click)
    • showLevelLine default behavior validation (currently only tests explicit true/false)

Best Practices Followed

  • License headers present on all new files with correct 2026 copyright year
  • TypeScript interfaces well-defined (EnhancedTreeItemProps, EnhancedTreeViewProps, TreeViewContextValue)
  • forwardRef correctly used on both TreeView and TreeItem
  • Context API used appropriately for depth tracking and keyboard state
  • Theme integration follows the established getMui*ThemeOverrides pattern
  • Storybook includes both interactive (with controls) and visual test templates
  • Export structure follows project conventions with re-exports from index
  • Clean migration from hidden_components — old files deleted, exports updated in src/index.ts

Overall: This is a solid implementation of a feature-rich TreeView component. The main areas needing attention before merge are: (1) RTL support using logical CSS properties, (2) removing unused props.nodeId from the dependency array, (3) fixing the CHANGELOG formatting, and (4) replacing hardcoded rgba with theme tokens. The keyboard navigation implementation is ambitious and works, but would benefit from integration tests given its reliance on requestAnimationFrame and synthetic event dispatch. Adding a global disabled prop and improving accessibility for hover actions would bring this in line with other Enchanted components.

Hey @MuhammedShabeelk, do you have checked the points from the Github Copilot review?

I have only check the backgroundColor one, but this is not fixed...

image

for this backgroundColor, I asked her, and she said that we don't have this in the library for now

@HanjoHagemeierHCL HanjoHagemeierHCL self-requested a review June 3, 2026 13:13
@HanjoHagemeierHCL HanjoHagemeierHCL merged commit 7dd2350 into HCL-TECH-SOFTWARE:main Jun 3, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants