From 0202a26e676d26698498f1d309f49d479156abed Mon Sep 17 00:00:00 2001 From: Katsiaryna Pustakhod Date: Thu, 4 May 2023 15:28:55 +0200 Subject: [PATCH 1/6] Refactoring: Move node logic in separate file. --- src/TreeView/constants.ts | 24 +++ src/TreeView/index.tsx | 425 +------------------------------------- src/TreeView/node.tsx | 345 +++++++++++++++++++++++++++++++ src/TreeView/types.ts | 95 +++++++-- src/TreeView/utils.ts | 3 + src/index.ts | 16 +- 6 files changed, 475 insertions(+), 433 deletions(-) create mode 100644 src/TreeView/constants.ts create mode 100644 src/TreeView/node.tsx diff --git a/src/TreeView/constants.ts b/src/TreeView/constants.ts new file mode 100644 index 00000000..4ad6aa31 --- /dev/null +++ b/src/TreeView/constants.ts @@ -0,0 +1,24 @@ +export const baseClassNames = { + root: "tree", + node: "tree-node", + branch: "tree-node__branch", + branchWrapper: "tree-branch-wrapper", + leafListItem: "tree-leaf-list-item", + leaf: "tree-node__leaf", + nodeGroup: "tree-node-group", +} as const; + +export const clickActions = { + select: "SELECT", + focus: "FOCUS", + exclusiveSelect: "EXCLUSIVE_SELECT", +} as const; + +export const CLICK_ACTIONS = Object.freeze(Object.values(clickActions)); + +export const nodeActions = { + check: "check", + select: "select", +} as const; + +export const NODE_ACTIONS = Object.freeze(Object.values(nodeActions)); diff --git a/src/TreeView/index.tsx b/src/TreeView/index.tsx index 9d4a99f6..e10d509d 100644 --- a/src/TreeView/index.tsx +++ b/src/TreeView/index.tsx @@ -8,20 +8,18 @@ import { TreeViewAction, } from "./reducer"; import { - EventCallback, + ClickActions, INode, - INodeRef, INodeRefs, + INodeRendererProps, + NodeAction, NodeId, TreeViewData, } from "./types"; import { - composeHandlers, difference, focusRef, getAccessibleRange, - getAriaChecked, - getAriaSelected, getDescendants, getLastAccessible, getNextAccessible, @@ -39,17 +37,15 @@ import { getTreeParent, getTreeNode, validateTreeViewData, + noop, } from "./utils"; - -const baseClassNames = { - root: "tree", - node: "tree-node", - branch: "tree-node__branch", - branchWrapper: "tree-branch-wrapper", - leafListItem: "tree-leaf-list-item", - leaf: "tree-node__leaf", - nodeGroup: "tree-node-group", -} as const; +import { Node } from "./node"; +import { + baseClassNames, + clickActions, + CLICK_ACTIONS, + NODE_ACTIONS, +} from "./constants"; interface IUseTreeProps { data: TreeViewData; @@ -380,77 +376,6 @@ const useTree = ({ return [state, dispatch] as const; }; -const clickActions = { - select: "SELECT", - focus: "FOCUS", - exclusiveSelect: "EXCLUSIVE_SELECT", -} as const; - -export const CLICK_ACTIONS = Object.freeze(Object.values(clickActions)); - -type ValueOf = T[keyof T]; -export type ClickActions = ValueOf; - -type ActionableNode = - | { - "aria-selected": boolean | undefined; - } - | { - "aria-checked": boolean | undefined | "mixed"; - }; - -export type LeafProps = ActionableNode & { - role: string; - tabIndex: number; - onClick: EventCallback; - ref: (x: T | null) => void; - className: string; - "aria-setsize": number; - "aria-posinset": number; - "aria-level": number; - "aria-selected": boolean; - disabled: boolean; - "aria-disabled": boolean; -}; - -export interface IBranchProps { - onClick: EventCallback; - className: string; -} - -export interface INodeRendererProps { - /** The object that represents the rendered node */ - element: INode; - /** A function which gives back the props to pass to the node */ - getNodeProps: (args?: { - onClick?: EventCallback; - }) => IBranchProps | LeafProps; - /** Whether the rendered node is a branch node */ - isBranch: boolean; - /** Whether the rendered node is selected */ - isSelected: boolean; - /** If the node is a branch node, whether it is half-selected, else undefined */ - isHalfSelected: boolean; - /** If the node is a branch node, whether it is expanded, else undefined */ - isExpanded: boolean; - /** Whether the rendered node is disabled */ - isDisabled: boolean; - /** A positive integer that corresponds to the aria-level attribute */ - level: number; - /** A positive integer that corresponds to the aria-setsize attribute */ - setsize: number; - /** A positive integer that corresponds to the aria-posinset attribute */ - posinset: number; - /** Function to assign to the onClick event handler of the element(s) that will toggle the selected state */ - handleSelect: EventCallback; - /** Function to assign to the onClick event handler of the element(s) that will toggle the expanded state */ - handleExpand: EventCallback; - /** Function to dispatch actions */ - dispatch: React.Dispatch; - /** state of the treeview */ - treeState: ITreeViewState; -} - export interface ITreeViewOnSelectProps { element: INode; isBranch: boolean; @@ -479,15 +404,6 @@ export interface ITreeViewOnLoadDataProps { treeState: ITreeViewState; } -const nodeActions = { - check: "check", - select: "select", -} as const; - -export const NODE_ACTIONS = Object.freeze(Object.values(nodeActions)); - -export type NodeAction = ValueOf; - export interface ITreeViewProps { /** Tree data*/ data: TreeViewData; @@ -535,8 +451,6 @@ export interface ITreeViewProps { }) => void; } -// eslint-disable-next-line @typescript-eslint/no-empty-function -const noop = () => {}; const TreeView = React.forwardRef( function TreeView( { @@ -653,323 +567,6 @@ const TreeView = React.forwardRef( } ); -interface INodeProps { - element: INode; - dispatch: React.Dispatch; - data: TreeViewData; - nodeAction: NodeAction; - selectedIds: Set; - tabbableId: NodeId; - isFocused: boolean; - expandedIds: Set; - disabledIds: Set; - halfSelectedIds: Set; - lastUserSelect: NodeId; - nodeRefs: INodeRefs; - leafRefs: INodeRefs; - baseClassNames: typeof baseClassNames; - nodeRenderer: (props: INodeRendererProps) => React.ReactNode; - setsize: number; - posinset: number; - level: number; - propagateCollapse: boolean; - propagateSelect: boolean; - multiSelect: boolean; - togglableSelect: boolean; - clickAction?: ClickActions; - state: ITreeViewState; - propagateSelectUpwards: boolean; -} - -const Node = (props: INodeProps) => { - const { - element, - dispatch, - data, - selectedIds, - tabbableId, - isFocused, - expandedIds, - disabledIds, - halfSelectedIds, - lastUserSelect, - nodeRefs, - leafRefs, - baseClassNames, - nodeRenderer, - nodeAction, - setsize, - posinset, - level, - propagateCollapse, - propagateSelect, - multiSelect, - togglableSelect, - clickAction, - state, - } = props; - - const handleExpand: EventCallback = (event) => { - if (event.ctrlKey || event.altKey || event.shiftKey) return; - if (expandedIds.has(element.id) && propagateCollapse) { - const ids: NodeId[] = [ - element.id, - ...getDescendants(data, element.id, new Set()), - ]; - dispatch({ - type: treeTypes.collapseMany, - ids, - lastInteractedWith: element.id, - }); - } else { - dispatch({ - type: treeTypes.toggle, - id: element.id, - lastInteractedWith: element.id, - }); - } - }; - - const handleFocus = (): void => - dispatch({ - type: treeTypes.focus, - id: element.id, - lastInteractedWith: element.id, - }); - - const handleSelect: EventCallback = (event) => { - if (event.shiftKey) { - let ids = getAccessibleRange({ - data, - expandedIds, - from: lastUserSelect, - to: element.id, - }).filter((id) => !disabledIds.has(id)); - ids = propagateSelect ? propagatedIds(data, ids, disabledIds) : ids; - dispatch({ - type: treeTypes.exclusiveChangeSelectMany, - select: true, - multiSelect, - ids, - lastInteractedWith: element.id, - }); - } else if (event.ctrlKey || clickAction === clickActions.select) { - const isSelectedAndHasSelectedDescendants = isBranchSelectedAndHasSelectedDescendants( - data, - element.id, - selectedIds - ); - - //Select - dispatch({ - type: togglableSelect - ? isSelectedAndHasSelectedDescendants - ? treeTypes.halfSelect - : treeTypes.toggleSelect - : treeTypes.select, - id: element.id, - multiSelect, - lastInteractedWith: element.id, - }); - propagateSelect && - !disabledIds.has(element.id) && - dispatch({ - type: treeTypes.changeSelectMany, - ids: propagatedIds(data, [element.id], disabledIds), - select: togglableSelect ? !selectedIds.has(element.id) : true, - multiSelect, - lastInteractedWith: element.id, - }); - } else if (clickAction === clickActions.exclusiveSelect) { - dispatch({ - type: togglableSelect ? treeTypes.toggleSelect : treeTypes.select, - id: element.id, - multiSelect: false, - lastInteractedWith: element.id, - }); - } else if (clickAction === clickActions.focus) { - dispatch({ - type: treeTypes.focus, - id: element.id, - lastInteractedWith: element.id, - }); - } - }; - - const getClasses = (className: string) => { - return cx(className, { - [`${className}--expanded`]: expandedIds.has(element.id), - [`${className}--selected`]: selectedIds.has(element.id), - [`${className}--focused`]: tabbableId === element.id && isFocused, - }); - }; - const ariaActionProp = - nodeAction === "select" - ? { - "aria-selected": getAriaSelected({ - isSelected: selectedIds.has(element.id), - isDisabled: disabledIds.has(element.id), - multiSelect, - }), - } - : { - "aria-checked": getAriaChecked({ - isSelected: selectedIds.has(element.id), - isDisabled: disabledIds.has(element.id), - isHalfSelected: halfSelectedIds.has(element.id), - multiSelect, - }), - }; - const getLeafProps = (args: { onClick?: EventCallback } = {}) => { - const { onClick } = args; - return { - role: "treeitem", - tabIndex: tabbableId === element.id ? 0 : -1, - onClick: - onClick == null - ? composeHandlers(handleSelect, handleFocus) - : composeHandlers(onClick, handleFocus), - ref: (x: INodeRef) => { - if (nodeRefs?.current != null && leafRefs?.current != null) { - nodeRefs.current[element.id] = x; - leafRefs.current[element.id] = x; - } - }, - className: cx(getClasses(baseClassNames.node), baseClassNames.leaf), - "aria-setsize": setsize, - "aria-posinset": posinset, - "aria-level": level, - disabled: disabledIds.has(element.id), - "aria-disabled": disabledIds.has(element.id), - ...ariaActionProp, - }; - }; - - const getBranchLeafProps = (args: { onClick?: EventCallback } = {}) => { - const { onClick } = args; - return { - onClick: - onClick == null - ? composeHandlers(handleSelect, handleExpand, handleFocus) - : composeHandlers(onClick, handleFocus), - className: cx(getClasses(baseClassNames.node), baseClassNames.branch), - ref: (x: INodeRef) => { - if (leafRefs?.current != null) { - leafRefs.current[element.id] = x; - } - }, - }; - }; - - return isBranchNode(data, element.id) || element.isBranch ? ( -
  • { - if (nodeRefs?.current != null && x != null) { - nodeRefs.current[element.id] = x; - } - }} - className={baseClassNames.branchWrapper} - {...ariaActionProp} - > - <> - {nodeRenderer({ - element, - isBranch: true, - isSelected: selectedIds.has(element.id), - isHalfSelected: halfSelectedIds.has(element.id), - isExpanded: expandedIds.has(element.id), - isDisabled: disabledIds.has(element.id), - dispatch, - getNodeProps: getBranchLeafProps, - setsize, - posinset, - level, - handleSelect, - handleExpand, - treeState: state, - })} - - -
    • - ) : ( -
  • - {nodeRenderer({ - element, - isBranch: false, - isSelected: selectedIds.has(element.id), - isHalfSelected: false, - isExpanded: false, - isDisabled: disabledIds.has(element.id), - dispatch, - getNodeProps: getLeafProps, - setsize, - posinset, - level, - handleSelect, - handleExpand: noop, - treeState: state, - })} -
    • - ); -}; - -interface INodeGroupProps extends Omit { - getClasses: (className: string) => string; - /** don't send this. The NodeGroup render function, determines it for you */ - setsize?: undefined; - /** don't send this. The NodeGroup render function, determines it for you */ - posinset?: undefined; -} - -/** - * It's convenient to pass props down to the child, but we don't want to pass everything since it would create incorrect values for setsize and posinset - */ -const removeIrrelevantGroupProps = ( - nodeProps: INodeProps -): Omit => { - // eslint-disable-next-line @typescript-eslint/no-unused-vars - const { setsize, posinset, ...rest } = nodeProps; - return rest; -}; - -const NodeGroup = ({ - data, - element, - expandedIds, - getClasses, - baseClassNames, - level, - ...rest -}: INodeGroupProps) => ( -
      - {expandedIds.has(element.id) && - element.children.length > 0 && - element.children.map((x, index) => ( - - ))} -
    -); - const handleKeyDown = ({ data, expandedIds, diff --git a/src/TreeView/node.tsx b/src/TreeView/node.tsx new file mode 100644 index 00000000..99364c08 --- /dev/null +++ b/src/TreeView/node.tsx @@ -0,0 +1,345 @@ +import React from "react"; +import cx from "classnames"; +import { ITreeViewState, treeTypes, TreeViewAction } from "./reducer"; +import { + ClickActions, + EventCallback, + INode, + INodeRef, + INodeRefs, + INodeRendererProps, + NodeAction, + NodeId, + TreeViewData, +} from "./types"; +import { + composeHandlers, + getAccessibleRange, + getAriaChecked, + getAriaSelected, + getDescendants, + getTreeNode, + isBranchNode, + isBranchSelectedAndHasSelectedDescendants, + noop, + propagatedIds, +} from "./utils"; +import { baseClassNames, clickActions } from "./constants"; + +export interface INodeProps { + element: INode; + dispatch: React.Dispatch; + data: TreeViewData; + nodeAction: NodeAction; + selectedIds: Set; + tabbableId: NodeId; + isFocused: boolean; + expandedIds: Set; + disabledIds: Set; + halfSelectedIds: Set; + lastUserSelect: NodeId; + nodeRefs: INodeRefs; + leafRefs: INodeRefs; + baseClassNames: typeof baseClassNames; + nodeRenderer: (props: INodeRendererProps) => React.ReactNode; + setsize: number; + posinset: number; + level: number; + propagateCollapse: boolean; + propagateSelect: boolean; + multiSelect: boolean; + togglableSelect: boolean; + clickAction?: ClickActions; + state: ITreeViewState; + propagateSelectUpwards: boolean; +} + +export interface INodeGroupProps + extends Omit { + getClasses: (className: string) => string; + /** don't send this. The NodeGroup render function, determines it for you */ + setsize?: undefined; + /** don't send this. The NodeGroup render function, determines it for you */ + posinset?: undefined; +} + +/** + * It's convenient to pass props down to the child, but we don't want to pass everything since it would create incorrect values for setsize and posinset + */ +const removeIrrelevantGroupProps = ( + nodeProps: INodeProps +): Omit => { + // eslint-disable-next-line @typescript-eslint/no-unused-vars + const { setsize, posinset, ...rest } = nodeProps; + return rest; +}; + +export const Node = (props: INodeProps) => { + const { + element, + dispatch, + data, + selectedIds, + tabbableId, + isFocused, + expandedIds, + disabledIds, + halfSelectedIds, + lastUserSelect, + nodeRefs, + leafRefs, + baseClassNames, + nodeRenderer, + nodeAction, + setsize, + posinset, + level, + propagateCollapse, + propagateSelect, + multiSelect, + togglableSelect, + clickAction, + state, + } = props; + + const handleExpand: EventCallback = (event) => { + if (event.ctrlKey || event.altKey || event.shiftKey) return; + if (expandedIds.has(element.id) && propagateCollapse) { + const ids: NodeId[] = [ + element.id, + ...getDescendants(data, element.id, new Set()), + ]; + dispatch({ + type: treeTypes.collapseMany, + ids, + lastInteractedWith: element.id, + }); + } else { + dispatch({ + type: treeTypes.toggle, + id: element.id, + lastInteractedWith: element.id, + }); + } + }; + + const handleFocus = (): void => + dispatch({ + type: treeTypes.focus, + id: element.id, + lastInteractedWith: element.id, + }); + + const handleSelect: EventCallback = (event) => { + if (event.shiftKey) { + let ids = getAccessibleRange({ + data, + expandedIds, + from: lastUserSelect, + to: element.id, + }).filter((id) => !disabledIds.has(id)); + ids = propagateSelect ? propagatedIds(data, ids, disabledIds) : ids; + dispatch({ + type: treeTypes.exclusiveChangeSelectMany, + select: true, + multiSelect, + ids, + lastInteractedWith: element.id, + }); + } else if (event.ctrlKey || clickAction === clickActions.select) { + const isSelectedAndHasSelectedDescendants = isBranchSelectedAndHasSelectedDescendants( + data, + element.id, + selectedIds + ); + + //Select + dispatch({ + type: togglableSelect + ? isSelectedAndHasSelectedDescendants + ? treeTypes.halfSelect + : treeTypes.toggleSelect + : treeTypes.select, + id: element.id, + multiSelect, + lastInteractedWith: element.id, + }); + propagateSelect && + !disabledIds.has(element.id) && + dispatch({ + type: treeTypes.changeSelectMany, + ids: propagatedIds(data, [element.id], disabledIds), + select: togglableSelect ? !selectedIds.has(element.id) : true, + multiSelect, + lastInteractedWith: element.id, + }); + } else if (clickAction === clickActions.exclusiveSelect) { + dispatch({ + type: togglableSelect ? treeTypes.toggleSelect : treeTypes.select, + id: element.id, + multiSelect: false, + lastInteractedWith: element.id, + }); + } else if (clickAction === clickActions.focus) { + dispatch({ + type: treeTypes.focus, + id: element.id, + lastInteractedWith: element.id, + }); + } + }; + + const getClasses = (className: string) => { + return cx(className, { + [`${className}--expanded`]: expandedIds.has(element.id), + [`${className}--selected`]: selectedIds.has(element.id), + [`${className}--focused`]: tabbableId === element.id && isFocused, + }); + }; + const ariaActionProp = + nodeAction === "select" + ? { + "aria-selected": getAriaSelected({ + isSelected: selectedIds.has(element.id), + isDisabled: disabledIds.has(element.id), + multiSelect, + }), + } + : { + "aria-checked": getAriaChecked({ + isSelected: selectedIds.has(element.id), + isDisabled: disabledIds.has(element.id), + isHalfSelected: halfSelectedIds.has(element.id), + multiSelect, + }), + }; + const getLeafProps = (args: { onClick?: EventCallback } = {}) => { + const { onClick } = args; + return { + role: "treeitem", + tabIndex: tabbableId === element.id ? 0 : -1, + onClick: + onClick == null + ? composeHandlers(handleSelect, handleFocus) + : composeHandlers(onClick, handleFocus), + ref: (x: INodeRef) => { + if (nodeRefs?.current != null && leafRefs?.current != null) { + nodeRefs.current[element.id] = x; + leafRefs.current[element.id] = x; + } + }, + className: cx(getClasses(baseClassNames.node), baseClassNames.leaf), + "aria-setsize": setsize, + "aria-posinset": posinset, + "aria-level": level, + disabled: disabledIds.has(element.id), + "aria-disabled": disabledIds.has(element.id), + ...ariaActionProp, + }; + }; + + const getBranchLeafProps = (args: { onClick?: EventCallback } = {}) => { + const { onClick } = args; + return { + onClick: + onClick == null + ? composeHandlers(handleSelect, handleExpand, handleFocus) + : composeHandlers(onClick, handleFocus), + className: cx(getClasses(baseClassNames.node), baseClassNames.branch), + ref: (x: INodeRef) => { + if (leafRefs?.current != null) { + leafRefs.current[element.id] = x; + } + }, + }; + }; + + return isBranchNode(data, element.id) || element.isBranch ? ( +
  • { + if (nodeRefs?.current != null && x != null) { + nodeRefs.current[element.id] = x; + } + }} + className={baseClassNames.branchWrapper} + {...ariaActionProp} + > + <> + {nodeRenderer({ + element, + isBranch: true, + isSelected: selectedIds.has(element.id), + isHalfSelected: halfSelectedIds.has(element.id), + isExpanded: expandedIds.has(element.id), + isDisabled: disabledIds.has(element.id), + dispatch, + getNodeProps: getBranchLeafProps, + setsize, + posinset, + level, + handleSelect, + handleExpand, + treeState: state, + })} + + +
    • + ) : ( +
  • + {nodeRenderer({ + element, + isBranch: false, + isSelected: selectedIds.has(element.id), + isHalfSelected: false, + isExpanded: false, + isDisabled: disabledIds.has(element.id), + dispatch, + getNodeProps: getLeafProps, + setsize, + posinset, + level, + handleSelect, + handleExpand: noop, + treeState: state, + })} +
    • + ); +}; + +export const NodeGroup = ({ + data, + element, + expandedIds, + getClasses, + baseClassNames, + level, + ...rest +}: INodeGroupProps) => ( +
      + {expandedIds.has(element.id) && + element.children.length > 0 && + element.children.map((x, index) => ( + + ))} +
    +); diff --git a/src/TreeView/types.ts b/src/TreeView/types.ts index 2a61bb5b..76be03d4 100644 --- a/src/TreeView/types.ts +++ b/src/TreeView/types.ts @@ -1,17 +1,13 @@ +import { clickActions, nodeActions } from "./constants"; +import { ITreeViewState, TreeViewAction } from "./reducer"; + +export type ValueOf = T[keyof T]; +export type ClickActions = ValueOf; + +export type NodeAction = ValueOf; + export type NodeId = number | string; -export interface INode { - /** A non-negative integer that uniquely identifies the node */ - id: NodeId; - /** Used to match on key press */ - name: string; - /** An array with the ids of the children nodes */ - children: NodeId[]; - /** The parent of the node; null for the root node */ - parent: NodeId | null; - /** Used to indicated whether a node is branch to be able load async data onExpand*/ - isBranch?: boolean; -} export type INodeRef = HTMLLIElement | HTMLDivElement; export type INodeRefs = null | React.RefObject<{ [key: NodeId]: INodeRef; @@ -21,4 +17,77 @@ export type TreeViewData = INode[]; export type EventCallback = ( event: React.MouseEvent | React.KeyboardEvent -) => void; + ) => void; + + export interface INode { + /** A non-negative integer that uniquely identifies the node */ + id: NodeId; + /** Used to match on key press */ + name: string; + /** An array with the ids of the children nodes */ + children: NodeId[]; + /** The parent of the node; null for the root node */ + parent: NodeId | null; + /** Used to indicated whether a node is branch to be able load async data onExpand*/ + isBranch?: boolean; + } + + export interface INodeRendererProps { + /** The object that represents the rendered node */ + element: INode; + /** A function which gives back the props to pass to the node */ + getNodeProps: (args?: { + onClick?: EventCallback; + }) => IBranchProps | LeafProps; + /** Whether the rendered node is a branch node */ + isBranch: boolean; + /** Whether the rendered node is selected */ + isSelected: boolean; + /** If the node is a branch node, whether it is half-selected, else undefined */ + isHalfSelected: boolean; + /** If the node is a branch node, whether it is expanded, else undefined */ + isExpanded: boolean; + /** Whether the rendered node is disabled */ + isDisabled: boolean; + /** A positive integer that corresponds to the aria-level attribute */ + level: number; + /** A positive integer that corresponds to the aria-setsize attribute */ + setsize: number; + /** A positive integer that corresponds to the aria-posinset attribute */ + posinset: number; + /** Function to assign to the onClick event handler of the element(s) that will toggle the selected state */ + handleSelect: EventCallback; + /** Function to assign to the onClick event handler of the element(s) that will toggle the expanded state */ + handleExpand: EventCallback; + /** Function to dispatch actions */ + dispatch: React.Dispatch; + /** state of the treeview */ + treeState: ITreeViewState; + } + + type ActionableNode = + | { + "aria-selected": boolean | undefined; + } + | { + "aria-checked": boolean | undefined | "mixed"; + }; + + export type LeafProps = ActionableNode & { + role: string; + tabIndex: number; + onClick: EventCallback; + ref: (x: T | null) => void; + className: string; + "aria-setsize": number; + "aria-posinset": number; + "aria-level": number; + "aria-selected": boolean; + disabled: boolean; + "aria-disabled": boolean; + }; + + export interface IBranchProps { + onClick: EventCallback; + className: string; + } \ No newline at end of file diff --git a/src/TreeView/utils.ts b/src/TreeView/utils.ts index 4a584c96..79a100ba 100644 --- a/src/TreeView/utils.ts +++ b/src/TreeView/utils.ts @@ -1,6 +1,9 @@ import { useEffect, useRef } from "react"; import { EventCallback, INode, INodeRef, NodeId, TreeViewData } from "./types"; +// eslint-disable-next-line @typescript-eslint/no-empty-function +export const noop = () => {}; + export const composeHandlers = ( ...handlers: EventCallback[] ): EventCallback => (event): void => { diff --git a/src/index.ts b/src/index.ts index 2f110b7f..79557903 100644 --- a/src/index.ts +++ b/src/index.ts @@ -1,16 +1,20 @@ import TreeView, { - ClickActions, - CLICK_ACTIONS, - IBranchProps, - LeafProps, - INodeRendererProps, ITreeViewOnExpandProps, ITreeViewOnSelectProps, ITreeViewProps, ITreeViewOnLoadDataProps, } from "./TreeView"; +import { CLICK_ACTIONS } from "./TreeView/constants"; import { ITreeViewState, TreeViewAction } from "./TreeView/reducer"; -import { INode, TreeViewData, EventCallback } from "./TreeView/types"; +import { + INode, + TreeViewData, + EventCallback, + ClickActions, + INodeRendererProps, + IBranchProps, + LeafProps, +} from "./TreeView/types"; import { flattenTree } from "./TreeView/utils"; export { From f58c56b29cb7eace884b352f39c150cf4ce3b32a Mon Sep 17 00:00:00 2001 From: Katsiaryna Pustakhod Date: Fri, 5 May 2023 10:58:16 +0200 Subject: [PATCH 2/6] Add onNodeSelected callback to treeView props. --- src/TreeView/index.tsx | 42 +++++++++++++++++++++++++++++++++++++++++ src/TreeView/node.tsx | 4 ++++ src/TreeView/reducer.ts | 12 ++++++++++++ src/index.ts | 2 ++ 4 files changed, 60 insertions(+) diff --git a/src/TreeView/index.tsx b/src/TreeView/index.tsx index e10d509d..08e04c71 100644 --- a/src/TreeView/index.tsx +++ b/src/TreeView/index.tsx @@ -57,6 +57,7 @@ interface IUseTreeProps { nodeRefs: INodeRefs; leafRefs: INodeRefs; onSelect?: (props: ITreeViewOnSelectProps) => void; + onNodeSelect?: (props: ITreeViewOnNodeSelectProps) => void; onExpand?: (props: ITreeViewOnExpandProps) => void; multiSelect?: boolean; propagateSelectUpwards?: boolean; @@ -76,6 +77,7 @@ const useTree = ({ nodeRefs, leafRefs, onSelect, + onNodeSelect, onExpand, onLoadData, togglableSelect, @@ -92,6 +94,7 @@ const useTree = ({ halfSelectedIds: new Set(), lastUserSelect: treeParentNode.children[0], lastInteractedWith: null, + lastManuallyToggled: null, disabledIds: new Set(defaultDisabledIds), }); @@ -103,6 +106,7 @@ const useTree = ({ halfSelectedIds, lastAction, lastInteractedWith, + lastManuallyToggled, } = state; const prevSelectedIds = usePrevious(selectedIds) || new Set(); const toggledIds = symmetricDifference(selectedIds, prevSelectedIds); @@ -135,6 +139,25 @@ const useTree = ({ state, ]); + useEffect(() => { + if (onNodeSelect != null && onNodeSelect !== noop) { + const diffDeselectedIds = difference(prevSelectedIds, selectedIds); + const diffSelectedIds = difference(selectedIds, prevSelectedIds); + + if ( + lastManuallyToggled != null && + (diffSelectedIds.size || diffDeselectedIds.size) + ) { + onNodeSelect({ + element: getTreeNode(data, lastManuallyToggled), + isSelected: selectedIds.has(lastManuallyToggled), + isBranch: isBranchNode(data, lastManuallyToggled), + treeState: state, + }); + } + } + }, [lastManuallyToggled, selectedIds]); + const prevExpandedIds = usePrevious(expandedIds) || new Set(); useEffect(() => { const toggledExpandIds = symmetricDifference(expandedIds, prevExpandedIds); @@ -340,6 +363,7 @@ const useTree = ({ keepFocus: true, NotUserAction: true, lastInteractedWith, + lastManuallyToggled, }); } } @@ -386,6 +410,13 @@ export interface ITreeViewOnSelectProps { treeState: ITreeViewState; } +export interface ITreeViewOnNodeSelectProps { + element: INode; + isSelected: boolean; + isBranch: boolean; + treeState?: ITreeViewState; +} + export interface ITreeViewOnExpandProps { element: INode; isExpanded: boolean; @@ -409,6 +440,8 @@ export interface ITreeViewProps { data: TreeViewData; /** Function called when a node changes its selected state */ onSelect?: (props: ITreeViewOnSelectProps) => void; + /** Function called when a single node is manually selected/unselected. */ + onNodeSelect?: (props: ITreeViewOnNodeSelectProps) => void; /** Function called when a node changes its expanded state */ onExpand?: (props: ITreeViewOnExpandProps) => void; /** Function called to load data asynchronously on expand */ @@ -458,6 +491,7 @@ const TreeView = React.forwardRef( selectedIds, nodeRenderer, onSelect = noop, + onNodeSelect = noop, onExpand = noop, onLoadData, className = "", @@ -491,6 +525,7 @@ const TreeView = React.forwardRef( nodeRefs, leafRefs, onSelect, + onNodeSelect, onExpand, onLoadData, togglableSelect, @@ -656,6 +691,7 @@ const handleKeyDown = ({ select: true, multiSelect, lastInteractedWith: previous, + lastManuallyToggled: previous, }); dispatch({ type: treeTypes.focus, @@ -677,6 +713,7 @@ const handleKeyDown = ({ multiSelect, select: true, lastInteractedWith: next, + lastManuallyToggled: next, }); dispatch({ type: treeTypes.focus, @@ -829,6 +866,7 @@ const handleKeyDown = ({ id: id, multiSelect, lastInteractedWith: id, + lastManuallyToggled: id, }); propagateSelect && !disabledIds.has(element.id) && @@ -838,6 +876,7 @@ const handleKeyDown = ({ select: togglableSelect ? !selectedIds.has(id) : true, multiSelect, lastInteractedWith: id, + lastManuallyToggled: id, }); expandOnKeyboardSelect && dispatch({ type: treeTypes.toggle, id, lastInteractedWith: id }); @@ -875,6 +914,9 @@ TreeView.propTypes = { /** Function called when a node changes its selected state */ onSelect: PropTypes.func, + /** Function called when a single node is manually selected/unselected. */ + onNodeSelect: PropTypes.func, + /** Function called when a node changes its expanded state */ onExpand: PropTypes.func, diff --git a/src/TreeView/node.tsx b/src/TreeView/node.tsx index 99364c08..ee076ea5 100644 --- a/src/TreeView/node.tsx +++ b/src/TreeView/node.tsx @@ -145,6 +145,7 @@ export const Node = (props: INodeProps) => { multiSelect, ids, lastInteractedWith: element.id, + lastManuallyToggled: element.id, }); } else if (event.ctrlKey || clickAction === clickActions.select) { const isSelectedAndHasSelectedDescendants = isBranchSelectedAndHasSelectedDescendants( @@ -163,6 +164,7 @@ export const Node = (props: INodeProps) => { id: element.id, multiSelect, lastInteractedWith: element.id, + lastManuallyToggled: element.id, }); propagateSelect && !disabledIds.has(element.id) && @@ -172,6 +174,7 @@ export const Node = (props: INodeProps) => { select: togglableSelect ? !selectedIds.has(element.id) : true, multiSelect, lastInteractedWith: element.id, + lastManuallyToggled: element.id, }); } else if (clickAction === clickActions.exclusiveSelect) { dispatch({ @@ -179,6 +182,7 @@ export const Node = (props: INodeProps) => { id: element.id, multiSelect: false, lastInteractedWith: element.id, + lastManuallyToggled: element.id, }); } else if (clickAction === clickActions.focus) { dispatch({ diff --git a/src/TreeView/reducer.ts b/src/TreeView/reducer.ts index 633431f1..ab968124 100644 --- a/src/TreeView/reducer.ts +++ b/src/TreeView/reducer.ts @@ -37,6 +37,7 @@ export type TreeViewAction = keepFocus?: boolean; NotUserAction?: boolean; lastInteractedWith?: NodeId | null; + lastManuallyToggled?: NodeId | null; } | { type: "DESELECT"; @@ -46,6 +47,7 @@ export type TreeViewAction = keepFocus?: boolean; NotUserAction?: boolean; lastInteractedWith?: NodeId | null; + lastManuallyToggled?: NodeId | null; } | { type: "TOGGLE"; id: NodeId; lastInteractedWith?: NodeId | null } | { @@ -54,6 +56,7 @@ export type TreeViewAction = multiSelect?: boolean; NotUserAction?: boolean; lastInteractedWith?: NodeId | null; + lastManuallyToggled?: NodeId | null; } | { type: "SELECT_MANY"; @@ -61,6 +64,7 @@ export type TreeViewAction = select?: boolean; multiSelect?: boolean; lastInteractedWith?: NodeId | null; + lastManuallyToggled?: NodeId | null; } | { type: "EXCLUSIVE_SELECT_MANY" } | { @@ -69,6 +73,7 @@ export type TreeViewAction = select?: boolean; multiSelect?: boolean; lastInteractedWith?: NodeId | null; + lastManuallyToggled?: NodeId | null; } | { type: "FOCUS"; id: NodeId; lastInteractedWith?: NodeId | null } | { type: "BLUR" } @@ -92,6 +97,8 @@ export interface ITreeViewState { lastUserSelect: NodeId; /** Last node interacted with */ lastInteractedWith?: NodeId | null; + /** Last node manually selected/deselected */ + lastManuallyToggled?: NodeId | null; lastAction?: TreeViewAction["type"]; } @@ -197,6 +204,7 @@ export const treeReducer = ( lastUserSelect: action.NotUserAction ? state.lastUserSelect : action.id, lastAction: action.type, lastInteractedWith: action.lastInteractedWith, + lastManuallyToggled: action.lastManuallyToggled, }; } case treeTypes.deselect: { @@ -220,6 +228,7 @@ export const treeReducer = ( lastUserSelect: action.NotUserAction ? state.lastUserSelect : action.id, lastAction: action.type, lastInteractedWith: action.lastInteractedWith, + lastManuallyToggled: action.lastManuallyToggled, }; } case treeTypes.toggleSelect: { @@ -246,6 +255,7 @@ export const treeReducer = ( lastUserSelect: action.NotUserAction ? state.lastUserSelect : action.id, lastAction: action.type, lastInteractedWith: action.lastInteractedWith, + lastManuallyToggled: action.lastManuallyToggled, }; } case treeTypes.changeSelectMany: { @@ -264,6 +274,7 @@ export const treeReducer = ( halfSelectedIds, lastAction: action.type, lastInteractedWith: action.lastInteractedWith, + lastManuallyToggled: action.lastManuallyToggled, }; } return state; @@ -284,6 +295,7 @@ export const treeReducer = ( halfSelectedIds, lastAction: action.type, lastInteractedWith: action.lastInteractedWith, + lastManuallyToggled: action.lastManuallyToggled, }; } return state; diff --git a/src/index.ts b/src/index.ts index 79557903..c918dfe1 100644 --- a/src/index.ts +++ b/src/index.ts @@ -1,6 +1,7 @@ import TreeView, { ITreeViewOnExpandProps, ITreeViewOnSelectProps, + ITreeViewOnNodeSelectProps, ITreeViewProps, ITreeViewOnLoadDataProps, } from "./TreeView"; @@ -23,6 +24,7 @@ export { INode, TreeViewData, ITreeViewOnSelectProps, + ITreeViewOnNodeSelectProps, CLICK_ACTIONS, ITreeViewOnExpandProps, ITreeViewOnLoadDataProps, From 43bcc92c570bdd81f58020a222066497fb1cf434 Mon Sep 17 00:00:00 2001 From: Katsiaryna Pustakhod Date: Fri, 5 May 2023 11:10:46 +0200 Subject: [PATCH 3/6] Update readme files --- README.md | 29 ++++++++++++++++++----------- src/TreeView/README.md | 8 +++++++- website/docs/api.md | 29 ++++++++++++++++++----------- 3 files changed, 43 insertions(+), 23 deletions(-) diff --git a/README.md b/README.md index 8882e7bc..c1a65796 100644 --- a/README.md +++ b/README.md @@ -21,6 +21,7 @@ A react component that implements the treeview pattern as described by the [WAI- | `data` | `array[node]` | `required` | Tree data | | `nodeRenderer` | `func` | `required` | Render prop for the node (see below for more details) | | `onSelect` | `func` | `noop` | Function called when a node changes its selected state | +| `onNodeSelect` | `func` | `noop` | Function called when a node was manually selected/deselected | | `onExpand` | `func` | `noop` | Function called when a node changes its expanded state | | `className` | `string` | `""` | className to add to the outermost dom element, al `ul` with `role = "tree"` | | `multiSelect` | `bool` | `false` | Allows multiple nodes to be selected | @@ -72,7 +73,7 @@ const data = [ ]; ``` -The array can also be generated from a nested object using the `flattenTree` helper (see the examples below). +The array can also be generated from a nested object using the `flattenTree` helper (see the examples below). Data supports non-sequential ids provided by user. @@ -106,6 +107,11 @@ Data supports non-sequential ids provided by user. - _Arguments_: `onSelect({element, isBranch, isExpanded, isSelected, isHalfSelected, isDisabled, treeState })` Note: the function uses the state _after_ the selection. +## onNodeSelect + +- _Arguments_: `onNodeSelect({element, isBranch, isSelected, treeState })` + Note: the function uses the state _after_ the selection. + ## onExpand - _Arguments_: `onExpand({element, isExpanded, isSelected, isHalfSelected, isDisabled, treeState})` @@ -162,13 +168,14 @@ Follows the same conventions described in https://www.w3.org/TR/wai-aria-practic The internal state of the component. -| Property | Type | Default | Description | -| ------------------ | ---------------- | ----------------------------- | ---------------------------------------- | -| selectedIds | `Set` | `new Set(defaultSelectedIds)` | Set of the ids of the selected nodes | -| tabbableId | `number` | `data[0].children[0]` | Id of the node with tabindex = 0 | -| isFocused | `bool` | `false` | Whether the tree has focus | -| expandedIds | `Set` | `new Set(defaultExpandedIds)` | Set of the ids of the expanded nodes | -| halfSelectedIds | `Set` | `new Set()` | Set of the ids of the selected nodes | -| lastUserSelect | `number` | `data[0].children[0]` | Last selection made directly by the user | -| lastInteractedWith | `number or null` | `null` | Last node interacted with | -| disabledIds | `Set` | `new Set(defaultDisabledIds)` | Set of the ids of the selected nodes | +| Property | Type | Default | Description | +| ------------------- | ---------------- | ----------------------------- | ----------------------------------------------- | +| selectedIds | `Set` | `new Set(defaultSelectedIds)` | Set of the ids of the selected nodes | +| tabbableId | `number` | `data[0].children[0]` | Id of the node with tabindex = 0 | +| isFocused | `bool` | `false` | Whether the tree has focus | +| expandedIds | `Set` | `new Set(defaultExpandedIds)` | Set of the ids of the expanded nodes | +| halfSelectedIds | `Set` | `new Set()` | Set of the ids of the selected nodes | +| lastUserSelect | `number` | `data[0].children[0]` | Last selection made directly by the user | +| lastInteractedWith | `number or null` | `null` | Last node interacted with | +| lastManuallyToggled | `number or null` | `null` | Last node that was manually selected/deselected | +| disabledIds | `Set` | `new Set(defaultDisabledIds)` | Set of the ids of the selected nodes | diff --git a/src/TreeView/README.md b/src/TreeView/README.md index 24976e73..d835e7eb 100644 --- a/src/TreeView/README.md +++ b/src/TreeView/README.md @@ -15,6 +15,7 @@ A react component that implements the treeview pattern as described by the [WAI- | `data` | `array[node]` | `required` | Tree data | | `nodeRenderer` | `func` | `required` | Render prop for the node (see below for more details) | | `onSelect` | `func` | `noop` | Function called when a node changes its selected state | +| `onNodeSelect` | `func` | `noop` | Function called when a node was manually selected/deselected | | `onExpand` | `func` | `noop` | Function called when a node changes its expanded state | | `className` | `string` | `""` | className to add to the outermost dom element, al `ul` with `role = "tree"` | | `multiSelect` | `bool` | `false` | Allows multiple nodes to be selected | @@ -65,7 +66,7 @@ const data = [ ]; ``` -The array can also be generated from a nested object using the flattenTree helper (see the examples below). +The array can also be generated from a nested object using the flattenTree helper (see the examples below). Data supports non-sequential ids provided by user. @@ -99,6 +100,11 @@ Data supports non-sequential ids provided by user. - _Arguments_: `onSelect({element, isBranch, isExpanded, isSelected, isHalfSelected, isDisabled, treeState })` Note: the function uses the state _after_ the selection. +## onNodeSelect + +- _Arguments_: `onNodeSelect({element, isBranch, isSelected, treeState })` + Note: the function uses the state _after_ the selection. + ### onExpand - _Arguments_: `onExpand({element, isExpanded, isSelected, isHalfSelected, isDisabled, treeState})` diff --git a/website/docs/api.md b/website/docs/api.md index f77ebef5..f7a32967 100644 --- a/website/docs/api.md +++ b/website/docs/api.md @@ -9,6 +9,7 @@ title: API Reference | `data` | `array[node]` | `required` | Tree data | | `nodeRenderer` | `func` | `required` | Render prop for the node (see below for more details) | | `onSelect` | `func` | `noop` | Function called when a node changes its selected state | +| `onNodeSelect` | `func` | `noop` | Function called when a node was manually selected/deselected | | `onExpand` | `func` | `noop` | Function called when a node changes its expanded state | | `className` | `string` | `""` | className to add to the outermost dom element, al `ul` with `role = "tree"` | | `multiSelect` | `bool` | `false` | Allows multiple nodes to be selected | @@ -60,7 +61,7 @@ const data = [ ]; ``` -The array can also be generated from a nested object using the `flattenTree` helper (see the examples below). +The array can also be generated from a nested object using the `flattenTree` helper (see the examples below). Data supports non-sequential ids provided by user. @@ -100,6 +101,11 @@ the handleSelect and handleExpand for just a part of the component, as demonstra - _Arguments_: `onSelect({element, isBranch, isExpanded, isSelected, isHalfSelected, isDisabled, treeState })` Note: the function uses the state _after_ the selection. +## onNodeSelect + +- _Arguments_: `onNodeSelect({element, isBranch, isSelected, treeState })` + Note: the function uses the state _after_ the selection. + ## onExpand - _Arguments_: `onExpand({element, isExpanded, isSelected, isHalfSelected, isDisabled, treeState})` @@ -156,13 +162,14 @@ Follows the same conventions described in https://www.w3.org/TR/wai-aria-practic The internal state of the component. -| Property | Type | Default | Description | -| ------------------ | ---------------- | ----------------------------- | ---------------------------------------- | -| selectedIds | `Set` | `new Set(defaultSelectedIds)` | Set of the ids of the selected nodes | -| tabbableId | `number` | `data[0].children[0]` | Id of the node with tabindex = 0 | -| isFocused | `bool` | `false` | Whether the tree has focus | -| expandedIds | `Set` | `new Set(defaultExpandedIds)` | Set of the ids of the expanded nodes | -| halfSelectedIds | `Set` | `new Set()` | Set of the ids of the selected nodes | -| lastUserSelect | `number` | `data[0].children[0]` | Last selection made directly by the user | -| lastInteractedWith | `number or null` | `null` | Last node interacted with | -| disabledIds | `Set` | `new Set(defaultDisabledIds)` | Set of the ids of the selected nodes | +| Property | Type | Default | Description | +| ------------------- | ---------------- | ----------------------------- | ----------------------------------------------- | +| selectedIds | `Set` | `new Set(defaultSelectedIds)` | Set of the ids of the selected nodes | +| tabbableId | `number` | `data[0].children[0]` | Id of the node with tabindex = 0 | +| isFocused | `bool` | `false` | Whether the tree has focus | +| expandedIds | `Set` | `new Set(defaultExpandedIds)` | Set of the ids of the expanded nodes | +| halfSelectedIds | `Set` | `new Set()` | Set of the ids of the selected nodes | +| lastUserSelect | `number` | `data[0].children[0]` | Last selection made directly by the user | +| lastInteractedWith | `number or null` | `null` | Last node interacted with | +| lastManuallyToggled | `number or null` | `null` | Last node that was manually selected/deselected | +| disabledIds | `Set` | `new Set(defaultDisabledIds)` | Set of the ids of the selected nodes | From 7e64e4b80228e43c638e4632f57e3ff4292d73a9 Mon Sep 17 00:00:00 2001 From: Katsiaryna Pustakhod Date: Mon, 8 May 2023 10:47:35 +0200 Subject: [PATCH 4/6] Add tests --- src/__tests__/TreeViewOnNodeSelect.test.tsx | 311 ++++++++++++++++++++ 1 file changed, 311 insertions(+) create mode 100644 src/__tests__/TreeViewOnNodeSelect.test.tsx diff --git a/src/__tests__/TreeViewOnNodeSelect.test.tsx b/src/__tests__/TreeViewOnNodeSelect.test.tsx new file mode 100644 index 00000000..6980791c --- /dev/null +++ b/src/__tests__/TreeViewOnNodeSelect.test.tsx @@ -0,0 +1,311 @@ +import "@testing-library/jest-dom/extend-expect"; +import React from "react"; +import TreeView from "../TreeView"; +import { fireEvent, render } from "@testing-library/react"; +import { flattenTree } from "../TreeView/utils"; +import { NodeId } from "../TreeView/types"; + +const folder = { + name: "", + children: [ + { + name: "Fruits", + children: [ + { name: "Avocados" }, + { name: "Bananas" }, + { name: "Berries" }, + { name: "Oranges" }, + { name: "Pears" }, + ], + }, + { + name: "Drinks", + children: [ + { name: "Apple Juice" }, + { name: "Chocolate" }, + { name: "Coffee" }, + { + name: "Tea", + children: [ + { name: "Black Tea" }, + { name: "Green Tea" }, + { name: "Red Tea" }, + { name: "Matcha" }, + ], + }, + ], + }, + { + name: "Vegetables", + children: [ + { name: "Beets" }, + { name: "Carrots" }, + { name: "Celery" }, + { name: "Lettuce" }, + { name: "Onions" }, + ], + }, + ], +}; + +const data = flattenTree(folder); + +interface TreeViewOnNodeSelectProps { + selectedIds?: NodeId[]; +} + +function TreeViewOnNodeSelect(props: TreeViewOnNodeSelectProps) { + const { selectedIds } = props; + return ( +
    + console.log(props)} + multiSelect + defaultExpandedIds={[1, 7, 11, 16]} + propagateSelect + propagateSelectUpwards + togglableSelect + nodeAction="check" + nodeRenderer={({ + element, + getNodeProps, + handleSelect, + handleExpand, + }) => { + return ( +
    +
    { + handleSelect(e); + e.stopPropagation(); + }} + /> + + {element.name}-{element.id} + +
    + ); + }} + /> +
    + ); +} + +jest.spyOn(global.console, "log"); + +test("onNodeSelect should be called when node manually seelcted/deselected", () => { + const { queryAllByRole } = render(); + const nodes = queryAllByRole("treeitem"); + + const expected = { + element: { children: [], id: 5, name: "Oranges", parent: 1 }, + isBranch: false, + isSelected: true, + treeState: { + disabledIds: new Set(), + expandedIds: new Set([1, 7, 11, 16]), + halfSelectedIds: new Set([1]), // this not present currently + isFocused: true, + lastAction: "HALF_SELECT", + lastInteractedWith: 5, + lastManuallyToggled: 5, + lastUserSelect: 5, + selectedIds: new Set([5]), + tabbableId: 5, + }, + }; + + if (document.activeElement == null) + throw new Error( + `Expected to find an active element on the document (after focusing the second element with role["treeitem"]), but did not.` + ); + + fireEvent.click(nodes[4].getElementsByClassName("checkbox-icon")[0]); + + expect(console.log).toBeCalledWith(expected); + expect(nodes[4]).toHaveAttribute("aria-checked", "true"); +}); + +test("onNodeSelect should be called only for interacted node", () => { + const { queryAllByRole } = render(); + const nodes = queryAllByRole("treeitem"); + + const expected = { + element: { id: 1, name: "Fruits", children: [2, 3, 4, 5, 6], parent: 0 }, + isSelected: true, + isBranch: true, + treeState: { + disabledIds: new Set(), + expandedIds: new Set([1, 7, 11, 16]), + halfSelectedIds: new Set(), + isFocused: true, + lastAction: "SELECT_MANY", + lastInteractedWith: 1, + lastManuallyToggled: 1, + lastUserSelect: 1, + selectedIds: new Set([1, 2, 3, 4, 5, 6]), + tabbableId: 1, + }, + }; + + if (document.activeElement == null) + throw new Error( + `Expected to find an active element on the document (after focusing the second element with role["treeitem"]), but did not.` + ); + + fireEvent.click(nodes[0].getElementsByClassName("checkbox-icon")[0]); + + expect(console.log).toBeCalledWith(expected); + expect(nodes[0]).toHaveAttribute("aria-checked", "true"); + expect(nodes[1]).toHaveAttribute("aria-checked", "true"); + expect(nodes[2]).toHaveAttribute("aria-checked", "true"); + expect(nodes[3]).toHaveAttribute("aria-checked", "true"); + expect(nodes[4]).toHaveAttribute("aria-checked", "true"); + expect(nodes[5]).toHaveAttribute("aria-checked", "true"); +}); + +test("onNodeSelect should not be called when controlled selection", () => { + render(); + expect(console.log).not.toBeCalled(); +}); + +describe("onNodeSelect should be called when node selected/deselected via keyboard", () => { + test("Enter", () => { + const expected = { + element: { children: [], id: 2, name: "Avocados", parent: 1 }, + isBranch: false, + isSelected: true, + treeState: { + disabledIds: new Set(), + expandedIds: new Set([1, 7, 11, 16]), + halfSelectedIds: new Set([1]), // this not present currently + isFocused: true, + lastAction: "HALF_SELECT", + lastInteractedWith: 2, + lastManuallyToggled: 2, + lastUserSelect: 2, + selectedIds: new Set([2]), + tabbableId: 2, + }, + }; + + const { queryAllByRole } = render(); + const nodes = queryAllByRole("treeitem"); + + if (document.activeElement == null) + throw new Error( + `Expected to find an active element on the document (after focusing the second element with role["treeitem"]), but did not.` + ); + nodes[0].focus(); //Fruits + fireEvent.keyDown(nodes[0], { key: "ArrowDown" }); //Avocados + fireEvent.keyDown(document.activeElement, { key: "Enter" }); + + expect(console.log).toBeCalledWith(expected); + expect(nodes[1]).toHaveAttribute("aria-checked", "true"); + }); + test("Space", () => { + const expected = { + element: { children: [], id: 2, name: "Avocados", parent: 1 }, + isBranch: false, + isSelected: true, + treeState: { + disabledIds: new Set(), + expandedIds: new Set([1, 7, 11, 16]), + halfSelectedIds: new Set([1]), // this not present currently + isFocused: true, + lastAction: "HALF_SELECT", + lastInteractedWith: 2, + lastManuallyToggled: 2, + lastUserSelect: 2, + selectedIds: new Set([2]), + tabbableId: 2, + }, + }; + + const { queryAllByRole } = render(); + const nodes = queryAllByRole("treeitem"); + + if (document.activeElement == null) + throw new Error( + `Expected to find an active element on the document (after focusing the second element with role["treeitem"]), but did not.` + ); + nodes[0].focus(); //Fruits + fireEvent.keyDown(nodes[0], { key: "ArrowDown" }); //Avocados + fireEvent.keyDown(document.activeElement, { key: " " }); + + expect(console.log).toBeCalledWith(expected); + expect(nodes[1]).toHaveAttribute("aria-checked", "true"); + }); + test("Shift+ArrowUp", () => { + const expected = { + element: { id: 1, name: "Fruits", children: [2, 3, 4, 5, 6], parent: 0 }, + isSelected: true, + isBranch: true, + treeState: { + disabledIds: new Set(), + expandedIds: new Set([1, 7, 11, 16]), + halfSelectedIds: new Set(), + isFocused: true, + lastAction: "FOCUS", + lastInteractedWith: 1, + lastManuallyToggled: 1, + lastUserSelect: 1, + selectedIds: new Set([1, 2, 3, 4, 5, 6]), + tabbableId: 1, + }, + }; + + const { queryAllByRole } = render(); + const nodes = queryAllByRole("treeitem"); + + if (document.activeElement == null) + throw new Error( + `Expected to find an active element on the document (after focusing the second element with role["treeitem"]), but did not.` + ); + nodes[0].focus(); //Fruits + fireEvent.keyDown(nodes[0], { key: "ArrowDown" }); //Avocados + fireEvent.keyDown(document.activeElement, { + key: "ArrowUp", + shiftKey: true, + }); + + expect(console.log).toBeCalledWith(expected); + expect(nodes[0]).toHaveAttribute("aria-checked", "true"); + }); + test("Shift+ArrowDown", () => { + const expected = { + element: { children: [], id: 2, name: "Avocados", parent: 1 }, + isBranch: false, + isSelected: true, + treeState: { + disabledIds: new Set(), + expandedIds: new Set([1, 7, 11, 16]), + halfSelectedIds: new Set([1]), // this not present currently + isFocused: true, + lastAction: "HALF_SELECT", + lastInteractedWith: 2, + lastManuallyToggled: 2, + lastUserSelect: 1, + selectedIds: new Set([2]), + tabbableId: 2, + }, + }; + + const { queryAllByRole } = render(); + const nodes = queryAllByRole("treeitem"); + + if (document.activeElement == null) + throw new Error( + `Expected to find an active element on the document (after focusing the second element with role["treeitem"]), but did not.` + ); + nodes[0].focus(); //Fruits + fireEvent.keyDown(nodes[0], { key: "ArrowDown", shiftKey: true }); //Avocados + + expect(console.log).toBeCalledWith(expected); + expect(nodes[1]).toHaveAttribute("aria-checked", "true"); + }); +}); From d9149952fd9cad846fd7cd45bc04f73ffeb721c5 Mon Sep 17 00:00:00 2001 From: Katsiaryna Pustakhod Date: Mon, 8 May 2023 10:47:59 +0200 Subject: [PATCH 5/6] Fix onNodeSelect to be called with halfSelected updates. --- src/TreeView/index.tsx | 50 +++++++++++++++++++++++++++++++++--------- 1 file changed, 40 insertions(+), 10 deletions(-) diff --git a/src/TreeView/index.tsx b/src/TreeView/index.tsx index 08e04c71..70c82744 100644 --- a/src/TreeView/index.tsx +++ b/src/TreeView/index.tsx @@ -38,6 +38,7 @@ import { getTreeNode, validateTreeViewData, noop, + getAncestors, } from "./utils"; import { Node } from "./node"; import { @@ -139,24 +140,53 @@ const useTree = ({ state, ]); + const prevHalfSelected = usePrevious(halfSelectedIds) || new Set(); useEffect(() => { if (onNodeSelect != null && onNodeSelect !== noop) { - const diffDeselectedIds = difference(prevSelectedIds, selectedIds); - const diffSelectedIds = difference(selectedIds, prevSelectedIds); - - if ( - lastManuallyToggled != null && - (diffSelectedIds.size || diffDeselectedIds.size) - ) { - onNodeSelect({ + const isSelectedIdsChanged = + difference(prevSelectedIds, selectedIds).size || + difference(selectedIds, prevSelectedIds).size; + const isHalfSelectedIdsChanged = + difference(prevHalfSelected, halfSelectedIds).size || + difference(halfSelectedIds, prevHalfSelected).size; + + if (lastManuallyToggled != null) { + const onNodeSelectProps = { element: getTreeNode(data, lastManuallyToggled), isSelected: selectedIds.has(lastManuallyToggled), isBranch: isBranchNode(data, lastManuallyToggled), treeState: state, - }); + }; + if ( + propagateSelectUpwards && + getAncestors(data, lastManuallyToggled, disabledIds).length + ) { + const { some } = propagateSelectChange( + data, + new Set([lastManuallyToggled]), + selectedIds, + disabledIds, + halfSelectedIds, + multiSelect + ); + if (isHalfSelectedIdsChanged) { + //when node click triggers propagation and both halfSelectedIds and selectedIds changed + onNodeSelect(onNodeSelectProps); + } else if ( + some.size && + !Array.from(some).find((i) => !halfSelectedIds.has(i)) && + isSelectedIdsChanged + ) { + //when node click doesn't trigger propagation, but selectedIds changed + onNodeSelect(onNodeSelectProps); + } + } else if (isSelectedIdsChanged) { + //when node click doesn't trigger propagation + onNodeSelect(onNodeSelectProps); + } } } - }, [lastManuallyToggled, selectedIds]); + }, [lastManuallyToggled, selectedIds, halfSelectedIds]); const prevExpandedIds = usePrevious(expandedIds) || new Set(); useEffect(() => { From 8baad329a366fb2d015acfc662c2e11ff51f4ec8 Mon Sep 17 00:00:00 2001 From: Katsiaryna Pustakhod Date: Tue, 9 May 2023 08:33:54 +0200 Subject: [PATCH 6/6] Fixing onNodeSelect call logic. --- README.md | 5 +- src/TreeView/README.md | 2 +- src/TreeView/index.tsx | 51 ++++--------------- src/TreeView/reducer.ts | 10 +++- src/__tests__/TreeViewOnNodeSelect.test.tsx | 42 +++++++++++---- website/docs/api.md | 2 +- .../MultiSelectCheckboxControlled/index.js | 2 + 7 files changed, 56 insertions(+), 58 deletions(-) diff --git a/README.md b/README.md index c1a65796..81015424 100644 --- a/README.md +++ b/README.md @@ -110,8 +110,7 @@ Data supports non-sequential ids provided by user. ## onNodeSelect - _Arguments_: `onNodeSelect({element, isBranch, isSelected, treeState })` - Note: the function uses the state _after_ the selection. - + Note: the function uses the state right _after_ the selection before propagation. ## onExpand - _Arguments_: `onExpand({element, isExpanded, isSelected, isHalfSelected, isDisabled, treeState})` @@ -120,7 +119,7 @@ Data supports non-sequential ids provided by user. ## onLoadData - _Arguments_: `onLoadData({element, isExpanded, isSelected, isHalfSelected, isDisabled, treeState})` - Note: the function uses the state _after_ inital data is loaded and on expansion . + Note: the function uses the state _after_ inital data is loaded and on expansion.

    ## Keyboard Navigation diff --git a/src/TreeView/README.md b/src/TreeView/README.md index d835e7eb..ef4d0416 100644 --- a/src/TreeView/README.md +++ b/src/TreeView/README.md @@ -103,7 +103,7 @@ Data supports non-sequential ids provided by user. ## onNodeSelect - _Arguments_: `onNodeSelect({element, isBranch, isSelected, treeState })` - Note: the function uses the state _after_ the selection. + Note: the function uses the state right _after_ the selection before propagation. ### onExpand diff --git a/src/TreeView/index.tsx b/src/TreeView/index.tsx index 70c82744..9df997f3 100644 --- a/src/TreeView/index.tsx +++ b/src/TreeView/index.tsx @@ -38,7 +38,6 @@ import { getTreeNode, validateTreeViewData, noop, - getAncestors, } from "./utils"; import { Node } from "./node"; import { @@ -140,53 +139,21 @@ const useTree = ({ state, ]); - const prevHalfSelected = usePrevious(halfSelectedIds) || new Set(); useEffect(() => { if (onNodeSelect != null && onNodeSelect !== noop) { - const isSelectedIdsChanged = - difference(prevSelectedIds, selectedIds).size || - difference(selectedIds, prevSelectedIds).size; - const isHalfSelectedIdsChanged = - difference(prevHalfSelected, halfSelectedIds).size || - difference(halfSelectedIds, prevHalfSelected).size; - if (lastManuallyToggled != null) { - const onNodeSelectProps = { - element: getTreeNode(data, lastManuallyToggled), - isSelected: selectedIds.has(lastManuallyToggled), - isBranch: isBranchNode(data, lastManuallyToggled), - treeState: state, - }; - if ( - propagateSelectUpwards && - getAncestors(data, lastManuallyToggled, disabledIds).length - ) { - const { some } = propagateSelectChange( - data, - new Set([lastManuallyToggled]), - selectedIds, - disabledIds, - halfSelectedIds, - multiSelect - ); - if (isHalfSelectedIdsChanged) { - //when node click triggers propagation and both halfSelectedIds and selectedIds changed - onNodeSelect(onNodeSelectProps); - } else if ( - some.size && - !Array.from(some).find((i) => !halfSelectedIds.has(i)) && - isSelectedIdsChanged - ) { - //when node click doesn't trigger propagation, but selectedIds changed - onNodeSelect(onNodeSelectProps); - } - } else if (isSelectedIdsChanged) { - //when node click doesn't trigger propagation - onNodeSelect(onNodeSelectProps); + if (toggledIds.size) { + onNodeSelect({ + element: getTreeNode(data, lastManuallyToggled), + isSelected: selectedIds.has(lastManuallyToggled), + isBranch: isBranchNode(data, lastManuallyToggled), + treeState: state, + }); + dispatch({ type: treeTypes.clearLastManuallyToggled }); } } } - }, [lastManuallyToggled, selectedIds, halfSelectedIds]); + }, [lastManuallyToggled, selectedIds, toggledIds]); const prevExpandedIds = usePrevious(expandedIds) || new Set(); useEffect(() => { diff --git a/src/TreeView/reducer.ts b/src/TreeView/reducer.ts index ab968124..a2727c85 100644 --- a/src/TreeView/reducer.ts +++ b/src/TreeView/reducer.ts @@ -17,6 +17,7 @@ export const treeTypes = { blur: "BLUR", disable: "DISABLE", enable: "ENABLE", + clearLastManuallyToggled: "CLEAR_MANUALLY_TOGGLED", } as const; export type TreeViewAction = @@ -78,7 +79,8 @@ export type TreeViewAction = | { type: "FOCUS"; id: NodeId; lastInteractedWith?: NodeId | null } | { type: "BLUR" } | { type: "DISABLE"; id: NodeId } - | { type: "ENABLE"; id: NodeId }; + | { type: "ENABLE"; id: NodeId } + | { type: "CLEAR_MANUALLY_TOGGLED" }; export interface ITreeViewState { /** Set of the ids of the expanded nodes */ @@ -329,6 +331,12 @@ export const treeReducer = ( disabledIds, }; } + case treeTypes.clearLastManuallyToggled: { + return { + ...state, + lastManuallyToggled: null, + }; + } default: throw new Error("Invalid action passed to the reducer"); } diff --git a/src/__tests__/TreeViewOnNodeSelect.test.tsx b/src/__tests__/TreeViewOnNodeSelect.test.tsx index 6980791c..5133fe91 100644 --- a/src/__tests__/TreeViewOnNodeSelect.test.tsx +++ b/src/__tests__/TreeViewOnNodeSelect.test.tsx @@ -101,16 +101,16 @@ test("onNodeSelect should be called when node manually seelcted/deselected", () const { queryAllByRole } = render(); const nodes = queryAllByRole("treeitem"); - const expected = { + const expectedSelected = { element: { children: [], id: 5, name: "Oranges", parent: 1 }, isBranch: false, isSelected: true, treeState: { disabledIds: new Set(), expandedIds: new Set([1, 7, 11, 16]), - halfSelectedIds: new Set([1]), // this not present currently + halfSelectedIds: new Set(), isFocused: true, - lastAction: "HALF_SELECT", + lastAction: "SELECT_MANY", lastInteractedWith: 5, lastManuallyToggled: 5, lastUserSelect: 5, @@ -118,6 +118,23 @@ test("onNodeSelect should be called when node manually seelcted/deselected", () tabbableId: 5, }, }; + const expectedDeselected = { + element: { children: [], id: 5, name: "Oranges", parent: 1 }, + isBranch: false, + isSelected: false, + treeState: { + disabledIds: new Set(), + expandedIds: new Set([1, 7, 11, 16]), + halfSelectedIds: new Set([1]), + isFocused: true, + lastAction: "SELECT_MANY", + lastInteractedWith: 5, + lastManuallyToggled: 5, + lastUserSelect: 5, + selectedIds: new Set(), + tabbableId: 5, + }, + }; if (document.activeElement == null) throw new Error( @@ -126,8 +143,13 @@ test("onNodeSelect should be called when node manually seelcted/deselected", () fireEvent.click(nodes[4].getElementsByClassName("checkbox-icon")[0]); - expect(console.log).toBeCalledWith(expected); + expect(console.log).toBeCalledWith(expectedSelected); expect(nodes[4]).toHaveAttribute("aria-checked", "true"); + + fireEvent.click(nodes[4].getElementsByClassName("checkbox-icon")[0]); + + expect(console.log).toBeCalledWith(expectedDeselected); + expect(nodes[4]).toHaveAttribute("aria-checked", "false"); }); test("onNodeSelect should be called only for interacted node", () => { @@ -182,9 +204,9 @@ describe("onNodeSelect should be called when node selected/deselected via keyboa treeState: { disabledIds: new Set(), expandedIds: new Set([1, 7, 11, 16]), - halfSelectedIds: new Set([1]), // this not present currently + halfSelectedIds: new Set(), isFocused: true, - lastAction: "HALF_SELECT", + lastAction: "SELECT_MANY", lastInteractedWith: 2, lastManuallyToggled: 2, lastUserSelect: 2, @@ -215,9 +237,9 @@ describe("onNodeSelect should be called when node selected/deselected via keyboa treeState: { disabledIds: new Set(), expandedIds: new Set([1, 7, 11, 16]), - halfSelectedIds: new Set([1]), // this not present currently + halfSelectedIds: new Set(), isFocused: true, - lastAction: "HALF_SELECT", + lastAction: "SELECT_MANY", lastInteractedWith: 2, lastManuallyToggled: 2, lastUserSelect: 2, @@ -284,9 +306,9 @@ describe("onNodeSelect should be called when node selected/deselected via keyboa treeState: { disabledIds: new Set(), expandedIds: new Set([1, 7, 11, 16]), - halfSelectedIds: new Set([1]), // this not present currently + halfSelectedIds: new Set(), isFocused: true, - lastAction: "HALF_SELECT", + lastAction: "FOCUS", lastInteractedWith: 2, lastManuallyToggled: 2, lastUserSelect: 1, diff --git a/website/docs/api.md b/website/docs/api.md index f7a32967..ce2cb0a6 100644 --- a/website/docs/api.md +++ b/website/docs/api.md @@ -104,7 +104,7 @@ the handleSelect and handleExpand for just a part of the component, as demonstra ## onNodeSelect - _Arguments_: `onNodeSelect({element, isBranch, isSelected, treeState })` - Note: the function uses the state _after_ the selection. + Note: the function uses the state right _after_ the selection before propagation. ## onExpand diff --git a/website/docs/examples/MultiSelectCheckboxControlled/index.js b/website/docs/examples/MultiSelectCheckboxControlled/index.js index 566d0c06..c1d43a16 100644 --- a/website/docs/examples/MultiSelectCheckboxControlled/index.js +++ b/website/docs/examples/MultiSelectCheckboxControlled/index.js @@ -95,6 +95,8 @@ function MultiSelectCheckboxControlled() { propagateSelect propagateSelectUpwards togglableSelect + onSelect={(props) => console.log('onSelect callback: ', props)} + onNodeSelect={(props) => console.log('onNodeSelect callback: ', props)} nodeRenderer={({ element, isBranch,