From f25a2bd88f1574a1bf0c80e53724b97523606198 Mon Sep 17 00:00:00 2001 From: Laszlo Vadasz Date: Thu, 22 Aug 2024 09:13:33 +0100 Subject: [PATCH] fix: pr comments VIII --- src/components/Tree/Tree.stories.docs.mdx | 126 +++++++++++++++++- src/components/Tree/Tree.types.ts | 6 +- src/components/Tree/Tree.utils.ts | 9 +- .../TreeNodeBase/TreeNodeBase.style.scss | 27 ++-- src/components/TreeNodeBase/TreeNodeBase.tsx | 3 +- .../TreeNodeBase/TreeNodeBase.types.ts | 6 +- 6 files changed, 152 insertions(+), 25 deletions(-) diff --git a/src/components/Tree/Tree.stories.docs.mdx b/src/components/Tree/Tree.stories.docs.mdx index daa97fc20b..1faeb4d113 100644 --- a/src/components/Tree/Tree.stories.docs.mdx +++ b/src/components/Tree/Tree.stories.docs.mdx @@ -6,4 +6,128 @@ make it possible to use it with Virtualized trees. Tree nodes must be wrapped in a `` component to ensure proper keyboard navigation and focus management. -Any node inside the `` can access to the tree context using the `useTreeContext` hook. \ No newline at end of file +Any node inside the `` can access to the tree context using the `useTreeContext` hook. + +## Nested VS Flat tree + +There is 2 ways to represent the tree structure in the DOM: + +### 1) Nested elements + +Simple static trees usually represented as nested elements. It's easy to understand and implement and +most of the semantics are implicitly set. But hard to render it automatically especially in virtualized trees. + +```html +
    + +
+ +``` + +### 2) Flat elements + +Flat tree structure is more flexible and can be used with virtualized trees. It requires more explicit semantics. + +```html +

Tree with flat DOM

+
    +
  • + Level 1 +
    +
    • +
  • + Level 2.1 +
    • +
  • + Level 2.2 +
    • +
  • + Level 2.3 +
    • +
  • + Level 2.4 +
    +
    • +
  • + Level 3.1 +
    • +
  • + Level 3.2 +
    • +
  • + Level 2.5 +
    • +
+``` \ No newline at end of file diff --git a/src/components/Tree/Tree.types.ts b/src/components/Tree/Tree.types.ts index 3c893157f5..a944bead51 100644 --- a/src/components/Tree/Tree.types.ts +++ b/src/components/Tree/Tree.types.ts @@ -1,4 +1,4 @@ -import { CSSProperties, HTMLAttributes, ReactNode } from 'react'; +import { CSSProperties, DetailedHTMLProps, HTMLAttributes, ReactNode } from 'react'; /** * The key codes used to navigate the tree. @@ -85,7 +85,7 @@ export type TreeIdNodeMap = Map; /** * The props of the Tree component. */ -export interface Props { +export interface Props extends DetailedHTMLProps, HTMLDivElement> { /** * Custom class for overriding this component's CSS. */ @@ -125,8 +125,6 @@ export interface Props { * 2) Flat: The tree is rendered as a single level list where the DOM does not reflect the semantic structure of the tree, and * we need to provide additional aria attributes to re-build it for the accessibility tree. * Virtualized trees are usually rendered flat. - * - * @see {@link https://codepen.io/maxinteger/pen/zYVNabV Example of a nested and flat tree} * @default true */ isRenderedFlat?: boolean; diff --git a/src/components/Tree/Tree.utils.ts b/src/components/Tree/Tree.utils.ts index 18f15a01c5..f661ad9b81 100644 --- a/src/components/Tree/Tree.utils.ts +++ b/src/components/Tree/Tree.utils.ts @@ -36,6 +36,7 @@ export const getTreeRootId = (tree: TreeIdNodeMap): TreeNodeId | undefined => { * Find the next active tree node based on the current active node * @param tree * @param activeNodeId + * @internal */ const findNextTreeNode = (tree: TreeIdNodeMap, activeNodeId: TreeNodeId): TreeNodeId => { let current = tree.get(activeNodeId); @@ -48,7 +49,8 @@ const findNextTreeNode = (tree: TreeIdNodeMap, activeNodeId: TreeNodeId): TreeNo const loopCheck = new Set(); // Otherwise, find the next sibling - for (;;) { + // eslint-disable-next-line no-constant-condition + while (true) { const parent = tree.get(current.parent); const pos = current.index + 1; @@ -75,6 +77,7 @@ const findNextTreeNode = (tree: TreeIdNodeMap, activeNodeId: TreeNodeId): TreeNo * @param tree * @param excludeRootNode * @param activeNodeId + * @internal */ const findPreviousTreeNode = ( tree: TreeIdNodeMap, @@ -119,6 +122,7 @@ const findPreviousTreeNode = ( * @param tree * @param activeNodeId * @param toggleTreeNode + * @internal */ const openNextNode = ( tree: TreeIdNodeMap, @@ -130,7 +134,6 @@ const openNextNode = ( if (!current.isLeaf) { if (!current.isOpen) { // Open it if it's closed - toggleTreeNode(activeNodeId); return activeNodeId; } else { @@ -149,6 +152,7 @@ const openNextNode = ( * @param activeNodeId * @param excludeRoot * @param toggleTreeNode + * @internal */ const closeNextNode = ( tree: TreeIdNodeMap, @@ -280,6 +284,7 @@ export const getNextActiveNode = ( * @param id * @param prevTree * @param isOpen + * @internal */ export const toggleTreeNodeRecord = ( id: TreeNodeId, diff --git a/src/components/TreeNodeBase/TreeNodeBase.style.scss b/src/components/TreeNodeBase/TreeNodeBase.style.scss index 9b437fa6f3..c8b9f0b5a7 100644 --- a/src/components/TreeNodeBase/TreeNodeBase.style.scss +++ b/src/components/TreeNodeBase/TreeNodeBase.style.scss @@ -9,24 +9,21 @@ transition: background-color 0.2s, color 0.2s, border-color 0.2s; box-sizing: border-box; outline: none; + cursor: pointer; - &[data-interactive='true'] { - cursor: pointer; - - &:hover, - &.hover { - background-color: var(--mds-color-theme-background-primary-hover); - } + &:hover, + &.hover { + background-color: var(--mds-color-theme-background-primary-hover); + } - &:active, - &.active { - color: var(--mds-color-theme-text-primary-normal); - background-color: var(--mds-color-theme-background-primary-active); - } + &:active, + &.active { + color: var(--mds-color-theme-text-primary-normal); + background-color: var(--mds-color-theme-background-primary-active); + } - &.focus { - color: var(--mds-color-theme-text-primary-normal); - } + &.focus { + color: var(--mds-color-theme-text-primary-normal); } & > div[data-position='start'] { diff --git a/src/components/TreeNodeBase/TreeNodeBase.tsx b/src/components/TreeNodeBase/TreeNodeBase.tsx index aa850760e2..7b4986eb7f 100644 --- a/src/components/TreeNodeBase/TreeNodeBase.tsx +++ b/src/components/TreeNodeBase/TreeNodeBase.tsx @@ -108,7 +108,7 @@ const TreeNodeBase = (props: Props, providedRef: TreeNodeBaseRefOrCallbackRef): const { pressProps, isPressed } = usePress({ preventFocusOnPress: true, // we handle it ourselves onPress: internalOnPress, - ...rest, + ref, }); // Prevent tree node update because it can cause state lost in the focused component e.g. Menu @@ -185,6 +185,7 @@ const TreeNodeBase = (props: Props, providedRef: TreeNodeBaseRefOrCallbackRef): lang={lang} {...treeNodePressProps} {...treeContext.getNodeProps(nodeId)} + {...rest} > {content} {treeContext.isRenderedFlat && !isLeaf && ( diff --git a/src/components/TreeNodeBase/TreeNodeBase.types.ts b/src/components/TreeNodeBase/TreeNodeBase.types.ts index 77e21aec80..e777236bd5 100644 --- a/src/components/TreeNodeBase/TreeNodeBase.types.ts +++ b/src/components/TreeNodeBase/TreeNodeBase.types.ts @@ -1,4 +1,4 @@ -import { CSSProperties, ReactNode, RefObject } from 'react'; +import { CSSProperties, DetailedHTMLProps, HTMLAttributes, ReactNode, RefObject } from 'react'; import { PressEvents } from '@react-types/shared'; import { TreeNodeId } from '../Tree'; @@ -17,7 +17,9 @@ export type TreeNodeBaseRefOrCallbackRef = /** * TreeNodeBase Props */ -export interface Props extends PressEvents { +export interface Props + extends PressEvents, + DetailedHTMLProps, HTMLDivElement> { /** * Unique identifier for the tree node */