Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[TreeView] Have down arrow go to first child #2840

Merged
merged 2 commits into from
Dec 15, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 4 additions & 5 deletions src/components/TreeView/TreeNavigator.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -23,7 +23,7 @@ export default function TreeNavigator(props: TreeNavigatorProps): ReactElement {
case Key.ArrowUp:
return getParent();
case Key.ArrowDown:
return getOnlyChild();
return getFirstChild();
}
};

Expand All @@ -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;
}
Expand All @@ -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,
};
Expand Down
30 changes: 14 additions & 16 deletions src/components/TreeView/tests/TreeNavigator.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,12 @@ const headNode: TreeNavigatorProps = {
currentDomain: domMap[mapIds.head],
animate: MOCK_ANIMATE,
};
// Current domain with a parent, two siblings, and multiple kids
// Current domain with a parent, no siblings, three kid
const parentOfThree: TreeNavigatorProps = {
currentDomain: domMap[mapIds.parent],
animate: MOCK_ANIMATE,
};
// Current domain with a parent, two siblings, three kids
const twoBrothersManyKids: TreeNavigatorProps = {
currentDomain: domMap[mapIds.middleKid],
animate: MOCK_ANIMATE,
Expand Down Expand Up @@ -56,26 +61,19 @@ 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", () => {
it("getFirstChild returns first if at least one child", () => {
const { current } = renderHook(() =>
useTreeNavigation(twoBrothersManyKids)
useTreeNavigation(parentOfThree)
).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])
expect(current.getFirstChild()).toEqual(
semDomFromTreeNode(domMap[mapIds.firstKid])
);
});

Expand Down Expand Up @@ -127,10 +125,10 @@ describe("TreeNavigator", () => {
const expectedDom = semDomFromTreeNode(domMap[mapIds.parent]);
expect(MOCK_ANIMATE).toHaveBeenCalledWith(expectedDom);
});
it("down arrow does nothing when multiple kids", () => {
it("down arrow moves when multiple kids", () => {
render(<TreeNavigator {...twoBrothersManyKids} />);
simulateKey(Key.ArrowDown);
expect(MOCK_ANIMATE).toHaveBeenCalledTimes(0);
expect(MOCK_ANIMATE).toHaveBeenCalledTimes(1);
});
it("down arrow moves to only child", () => {
render(<TreeNavigator {...noBrothersOneKid} />);
Expand Down