From aa157daf1e4d34143f634ad569bdfe5ae52de21a Mon Sep 17 00:00:00 2001 From: Danny Rorabaugh Date: Fri, 15 Dec 2023 10:32:19 -0500 Subject: [PATCH 1/2] [TreeView] Have down arrow move to first kid if more than one --- src/components/TreeView/TreeNavigator.tsx | 9 ++-- .../TreeView/tests/SemanticDomainMock.ts | 9 ---- .../TreeView/tests/TreeNavigator.test.tsx | 42 +++++++++---------- 3 files changed, 23 insertions(+), 37 deletions(-) diff --git a/src/components/TreeView/TreeNavigator.tsx b/src/components/TreeView/TreeNavigator.tsx index 69a8bcc882..2d623f2bb9 100644 --- a/src/components/TreeView/TreeNavigator.tsx +++ b/src/components/TreeView/TreeNavigator.tsx @@ -9,7 +9,7 @@ export interface TreeNavigatorProps { } export default function TreeNavigator(props: TreeNavigatorProps): ReactElement { - const { getNextSibling, getOnlyChild, getParent, getPrevSibling } = + const { getFirstChild, getNextSibling, getParent, getPrevSibling } = useTreeNavigation(props); // Navigate tree via arrow keys. @@ -23,7 +23,7 @@ export default function TreeNavigator(props: TreeNavigatorProps): ReactElement { case Key.ArrowUp: return getParent(); case Key.ArrowDown: - return getOnlyChild(); + return getFirstChild(); } }; @@ -43,8 +43,8 @@ export default function TreeNavigator(props: TreeNavigatorProps): ReactElement { } interface TreeNavigation { + getFirstChild: () => SemanticDomain | undefined; getNextSibling: () => SemanticDomain | undefined; - getOnlyChild: () => SemanticDomain | undefined; getParent: () => SemanticDomain | undefined; getPrevSibling: () => SemanticDomain | undefined; } @@ -53,9 +53,8 @@ interface TreeNavigation { export function useTreeNavigation(props: TreeNavigatorProps): TreeNavigation { const dom = props.currentDomain; return { + getFirstChild: () => (dom.children.length ? dom.children[0] : undefined), getNextSibling: () => dom.next, - getOnlyChild: () => - dom.children.length === 1 ? dom.children[0] : undefined, getParent: () => dom.parent, getPrevSibling: () => dom.previous, }; diff --git a/src/components/TreeView/tests/SemanticDomainMock.ts b/src/components/TreeView/tests/SemanticDomainMock.ts index e79f5a5657..9011072c57 100644 --- a/src/components/TreeView/tests/SemanticDomainMock.ts +++ b/src/components/TreeView/tests/SemanticDomainMock.ts @@ -57,15 +57,6 @@ for (let i = 0; i < 4; i++) { nodeMap[id] = { ...subdom, parent: semDomFromTreeNode(firstKid) }; } -// Give middleKid an odd # of subdomains -for (let i = 0; i < 3; i++) { - const id = middleKid.id + "." + i; - const subdom = newSemanticDomainTreeNode(id, `oddData${i}`); - subdom.parent = semDomFromTreeNode(middleKid); - middleKid.children.push(semDomFromTreeNode(subdom)); - nodeMap[id] = { ...subdom, parent: parentDom }; -} - // Give lastKid one subdomain with total depth of 5 let id = mapIds.depth3; const domDepth3 = newSemanticDomainTreeNode(id, "depth=3"); diff --git a/src/components/TreeView/tests/TreeNavigator.test.tsx b/src/components/TreeView/tests/TreeNavigator.test.tsx index 30e2a3e3ae..fa0850c1ff 100644 --- a/src/components/TreeView/tests/TreeNavigator.test.tsx +++ b/src/components/TreeView/tests/TreeNavigator.test.tsx @@ -18,8 +18,13 @@ const headNode: TreeNavigatorProps = { currentDomain: domMap[mapIds.head], animate: MOCK_ANIMATE, }; -// Current domain with a parent, two siblings, and multiple kids -const twoBrothersManyKids: TreeNavigatorProps = { +// Current domain with a parent, no siblings, three kid +const parent: TreeNavigatorProps = { + currentDomain: domMap[mapIds.parent], + animate: MOCK_ANIMATE, +}; +// Current domain with a parent, two siblings, no kids +const twoBrothers: TreeNavigatorProps = { currentDomain: domMap[mapIds.middleKid], animate: MOCK_ANIMATE, }; @@ -56,33 +61,24 @@ describe("TreeNavigator", () => { expect(current.getPrevSibling()).toBeUndefined(); }); - it("getOnlyChild returns undefined if no children", () => { + it("getFirstChild returns undefined if no children", () => { const { current } = renderHook(() => useTreeNavigation(noBrothersNoKids) ).result; - expect(current.getOnlyChild()).toBeUndefined(); + expect(current.getFirstChild()).toBeUndefined(); }); - it("getOnlyChild returns undefined if more than one child", () => { - const { current } = renderHook(() => - useTreeNavigation(twoBrothersManyKids) - ).result; - expect(current.getOnlyChild()).toBeUndefined(); - }); - - it("getOnlyChild returns child if only one", () => { - const { current } = renderHook(() => - useTreeNavigation(noBrothersOneKid) - ).result; - expect(current.getOnlyChild()).toEqual( - semDomFromTreeNode(domMap[mapIds.depth4]) + it("getFirstChild returns first if at least one child", () => { + const { current } = renderHook(() => useTreeNavigation(parent)).result; + expect(current.getFirstChild()).toEqual( + semDomFromTreeNode(domMap[mapIds.firstKid]) ); }); it("returns the expected parent and siblings", () => { - // The domain twoBrothersManyKids is the middle child of parentDomain. + // The domain twoBrothers is the middle child of parentDomain. const { current } = renderHook(() => - useTreeNavigation(twoBrothersManyKids) + useTreeNavigation(twoBrothers) ).result; expect(current.getNextSibling()).toEqual( semDomFromTreeNode(domMap[mapIds.lastKid]) @@ -98,7 +94,7 @@ describe("TreeNavigator", () => { describe("typing arrow key", () => { it("left arrow moves to left sibling", () => { - render(); + render(); simulateKey(Key.ArrowLeft); expect(MOCK_ANIMATE).toHaveBeenCalled(); const expectedDom = semDomFromTreeNode(domMap[mapIds.firstKid]); @@ -110,7 +106,7 @@ describe("TreeNavigator", () => { expect(MOCK_ANIMATE).toHaveBeenCalledTimes(0); }); it("right arrow moves to right sibling", () => { - render(); + render(); simulateKey(Key.ArrowRight); const expectedDom = semDomFromTreeNode(domMap[mapIds.lastKid]); expect(MOCK_ANIMATE).toHaveBeenCalledWith(expectedDom); @@ -121,14 +117,14 @@ describe("TreeNavigator", () => { expect(MOCK_ANIMATE).toHaveBeenCalledTimes(0); }); it("up arrow moves to parent domain", () => { - render(); + render(); simulateKey(Key.ArrowUp); expect(MOCK_ANIMATE).toHaveBeenCalled(); const expectedDom = semDomFromTreeNode(domMap[mapIds.parent]); expect(MOCK_ANIMATE).toHaveBeenCalledWith(expectedDom); }); it("down arrow does nothing when multiple kids", () => { - render(); + render(); simulateKey(Key.ArrowDown); expect(MOCK_ANIMATE).toHaveBeenCalledTimes(0); }); From b13872cfc8ed922fc1a937d077e6c09b4c8501d2 Mon Sep 17 00:00:00 2001 From: Danny Rorabaugh Date: Fri, 15 Dec 2023 10:49:29 -0500 Subject: [PATCH 2/2] Fix tests --- .../TreeView/tests/SemanticDomainMock.ts | 9 +++++++ .../TreeView/tests/TreeNavigator.test.tsx | 26 ++++++++++--------- 2 files changed, 23 insertions(+), 12 deletions(-) diff --git a/src/components/TreeView/tests/SemanticDomainMock.ts b/src/components/TreeView/tests/SemanticDomainMock.ts index 9011072c57..e79f5a5657 100644 --- a/src/components/TreeView/tests/SemanticDomainMock.ts +++ b/src/components/TreeView/tests/SemanticDomainMock.ts @@ -57,6 +57,15 @@ for (let i = 0; i < 4; i++) { nodeMap[id] = { ...subdom, parent: semDomFromTreeNode(firstKid) }; } +// Give middleKid an odd # of subdomains +for (let i = 0; i < 3; i++) { + const id = middleKid.id + "." + i; + const subdom = newSemanticDomainTreeNode(id, `oddData${i}`); + subdom.parent = semDomFromTreeNode(middleKid); + middleKid.children.push(semDomFromTreeNode(subdom)); + nodeMap[id] = { ...subdom, parent: parentDom }; +} + // Give lastKid one subdomain with total depth of 5 let id = mapIds.depth3; const domDepth3 = newSemanticDomainTreeNode(id, "depth=3"); diff --git a/src/components/TreeView/tests/TreeNavigator.test.tsx b/src/components/TreeView/tests/TreeNavigator.test.tsx index fa0850c1ff..2beb298ebc 100644 --- a/src/components/TreeView/tests/TreeNavigator.test.tsx +++ b/src/components/TreeView/tests/TreeNavigator.test.tsx @@ -19,12 +19,12 @@ const headNode: TreeNavigatorProps = { animate: MOCK_ANIMATE, }; // Current domain with a parent, no siblings, three kid -const parent: TreeNavigatorProps = { +const parentOfThree: TreeNavigatorProps = { currentDomain: domMap[mapIds.parent], animate: MOCK_ANIMATE, }; -// Current domain with a parent, two siblings, no kids -const twoBrothers: TreeNavigatorProps = { +// Current domain with a parent, two siblings, three kids +const twoBrothersManyKids: TreeNavigatorProps = { currentDomain: domMap[mapIds.middleKid], animate: MOCK_ANIMATE, }; @@ -69,16 +69,18 @@ describe("TreeNavigator", () => { }); it("getFirstChild returns first if at least one child", () => { - const { current } = renderHook(() => useTreeNavigation(parent)).result; + const { current } = renderHook(() => + useTreeNavigation(parentOfThree) + ).result; expect(current.getFirstChild()).toEqual( semDomFromTreeNode(domMap[mapIds.firstKid]) ); }); it("returns the expected parent and siblings", () => { - // The domain twoBrothers is the middle child of parentDomain. + // The domain twoBrothersManyKids is the middle child of parentDomain. const { current } = renderHook(() => - useTreeNavigation(twoBrothers) + useTreeNavigation(twoBrothersManyKids) ).result; expect(current.getNextSibling()).toEqual( semDomFromTreeNode(domMap[mapIds.lastKid]) @@ -94,7 +96,7 @@ describe("TreeNavigator", () => { describe("typing arrow key", () => { it("left arrow moves to left sibling", () => { - render(); + render(); simulateKey(Key.ArrowLeft); expect(MOCK_ANIMATE).toHaveBeenCalled(); const expectedDom = semDomFromTreeNode(domMap[mapIds.firstKid]); @@ -106,7 +108,7 @@ describe("TreeNavigator", () => { expect(MOCK_ANIMATE).toHaveBeenCalledTimes(0); }); it("right arrow moves to right sibling", () => { - render(); + render(); simulateKey(Key.ArrowRight); const expectedDom = semDomFromTreeNode(domMap[mapIds.lastKid]); expect(MOCK_ANIMATE).toHaveBeenCalledWith(expectedDom); @@ -117,16 +119,16 @@ describe("TreeNavigator", () => { expect(MOCK_ANIMATE).toHaveBeenCalledTimes(0); }); it("up arrow moves to parent domain", () => { - render(); + render(); simulateKey(Key.ArrowUp); expect(MOCK_ANIMATE).toHaveBeenCalled(); const expectedDom = semDomFromTreeNode(domMap[mapIds.parent]); expect(MOCK_ANIMATE).toHaveBeenCalledWith(expectedDom); }); - it("down arrow does nothing when multiple kids", () => { - render(); + it("down arrow moves when multiple kids", () => { + render(); simulateKey(Key.ArrowDown); - expect(MOCK_ANIMATE).toHaveBeenCalledTimes(0); + expect(MOCK_ANIMATE).toHaveBeenCalledTimes(1); }); it("down arrow moves to only child", () => { render();