From 8359d2a960e57223484bb4716b26909385af7193 Mon Sep 17 00:00:00 2001 From: Sappharad Date: Thu, 3 Aug 2023 15:25:46 -0500 Subject: [PATCH 1/3] Prevent tree from stealing focus from the page - If the user is not interacting directly with the tree already, it should not be able to scroll itself into view or take over focus when it is being controlled by something else on the page. - In situations like a master + details view where the tree is the details and the master control changes what is selected in the tree, the tree should not steal focus from the other control whose actions were modifying it. Such as if a user is arrowing through a list at the top of the page and there is a tree below it. --- src/TreeView/index.tsx | 26 +++++++++++++++++--------- src/__tests__/ControlledTree.test.tsx | 18 ++++++++++++++++++ 2 files changed, 35 insertions(+), 9 deletions(-) diff --git a/src/TreeView/index.tsx b/src/TreeView/index.tsx index cb622c3..e34ff25 100644 --- a/src/TreeView/index.tsx +++ b/src/TreeView/index.tsx @@ -57,6 +57,7 @@ interface IUseTreeProps { onExpand?: (props: ITreeViewOnExpandProps) => void; multiSelect?: boolean; propagateSelectUpwards?: boolean; + treeRef?: React.MutableRefObject; propagateSelect?: boolean; // eslint-disable-next-line @typescript-eslint/no-explicit-any onLoadData?: (props: ITreeViewOnLoadDataProps) => Promise; @@ -80,6 +81,7 @@ const useTree = ({ multiSelect, propagateSelect, propagateSelectUpwards, + treeRef, }: IUseTreeProps) => { const treeParentNode = getTreeParent(data); const [state, dispatch] = useReducer(treeReducer, { @@ -417,10 +419,16 @@ const useTree = ({ nodeRefs?.current != null && leafRefs?.current != null ) { - const tabbableNode = nodeRefs.current[tabbableId]; - const leafNode = leafRefs.current[lastInteractedWith]; - scrollToRef(leafNode); - focusRef(tabbableNode); + const isTreeActive = (treeRef?.current == null) || + (document.activeElement && treeRef.current.contains(document.activeElement)); + if (isTreeActive) { + // Only scroll and focus on the tree when it is the active element on the page. + // This prevents controlled updates from scrolling to the tree and giving it focus. + const tabbableNode = nodeRefs.current[tabbableId]; + const leafNode = leafRefs.current[lastInteractedWith]; + scrollToRef(leafNode); + focusRef(tabbableNode); + } } }, [tabbableId, nodeRefs, leafRefs, lastInteractedWith]); @@ -543,6 +551,10 @@ const TreeView = React.forwardRef( validateTreeViewData(data); const nodeRefs = useRef({}); const leafRefs = useRef({}); + let innerRef = useRef(null); + if (ref != null) { + innerRef = ref as React.MutableRefObject; + } const [state, dispatch] = useTree({ data, controlledSelectedIds: selectedIds, @@ -560,14 +572,10 @@ const TreeView = React.forwardRef( multiSelect, propagateSelect, propagateSelectUpwards, + treeRef: innerRef, }); propagateSelect = propagateSelect && multiSelect; - let innerRef = useRef(null); - if (ref != null) { - innerRef = ref as React.MutableRefObject; - } - return (
    { expect(newNodes[4]).toHaveAttribute("aria-checked", "false"); expect(newNodes[5]).toHaveAttribute("aria-checked", "false"); }); + + test("SelectedIds should be not steal focus if another control has it", () => { + let selectedIds = [42]; + let renderContent = (
    + + +
    ); + const { rerender } = render(renderContent); + + const editorElement = document?.getElementById("editor"); + editorElement?.focus(); + selectedIds = [4]; + rerender(renderContent); + expect(document.activeElement).toEqual(editorElement); + }); }); From 3983beae65e48aff3bc68082b18eb1f5f1eab034 Mon Sep 17 00:00:00 2001 From: Sappharad Date: Thu, 3 Aug 2023 15:29:58 -0500 Subject: [PATCH 2/3] Grammatical failure --- src/__tests__/ControlledTree.test.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/__tests__/ControlledTree.test.tsx b/src/__tests__/ControlledTree.test.tsx index b7c8629..5143abd 100644 --- a/src/__tests__/ControlledTree.test.tsx +++ b/src/__tests__/ControlledTree.test.tsx @@ -754,7 +754,7 @@ describe("Data with ids", () => { expect(newNodes[5]).toHaveAttribute("aria-checked", "false"); }); - test("SelectedIds should be not steal focus if another control has it", () => { + test("SelectedIds should not steal focus if another control has it", () => { let selectedIds = [42]; let renderContent = (
    From b5e42c0955434c120eeb62c5862fb5863ecdeb2f Mon Sep 17 00:00:00 2001 From: Sappharad Date: Tue, 15 Aug 2023 07:53:25 -0500 Subject: [PATCH 3/3] Resolve linter complaints --- src/__tests__/ControlledTree.test.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/__tests__/ControlledTree.test.tsx b/src/__tests__/ControlledTree.test.tsx index 5143abd..5450d78 100644 --- a/src/__tests__/ControlledTree.test.tsx +++ b/src/__tests__/ControlledTree.test.tsx @@ -756,7 +756,7 @@ describe("Data with ids", () => { test("SelectedIds should not steal focus if another control has it", () => { let selectedIds = [42]; - let renderContent = (
    + const renderContent = (