Skip to content

Commit

Permalink
fix(component): only focus on TreeItem when focus within Tree compone…
Browse files Browse the repository at this point in the history
…nt (#1087)

* fix(component): only focus on TreeItem when focus within Tree component

* chore(component): apply better testing practices to Tree component

* test(component): add tests to capture change
  • Loading branch information
chanceaclark authored Jan 5, 2023
1 parent e53f820 commit 64d102d
Show file tree
Hide file tree
Showing 5 changed files with 147 additions and 68 deletions.
20 changes: 19 additions & 1 deletion packages/big-design/src/components/StatefulTree/spec.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import { theme } from '@bigcommerce/big-design-theme';
import userEvent from '@testing-library/user-event';
import React from 'react';

import { fireEvent, render } from '@test/utils';
import { fireEvent, render, screen } from '@test/utils';

import { StatefulTree, StatefulTreeProps, TreeNodeProps } from '.';

Expand Down Expand Up @@ -182,3 +183,20 @@ describe('selectable = radio', () => {
}
});
});

test('should focus on TreeItem on arrow down', async () => {
render(getSimpleTree());

const node0 = await screen.findByRole('treeitem', { name: 'Test Node 0' });
const node1 = await screen.findByRole('treeitem', { name: 'Test Node 1' });

await userEvent.tab();

expect(node0).toHaveFocus();
expect(node1).not.toHaveFocus();

await userEvent.keyboard('{ArrowDown}');

expect(node0).not.toHaveFocus();
expect(node1).toHaveFocus();
});
7 changes: 6 additions & 1 deletion packages/big-design/src/components/Tree/Tree.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import React, { createContext, useMemo } from 'react';
import React, { createContext, useMemo, useRef } from 'react';

import { StyledUl } from './styled';
import { TreeNode } from './TreeNode';
Expand All @@ -14,6 +14,7 @@ export const TreeContext = createContext<TreeContextState<any>>({
onFocus: () => null,
},
onKeyDown: () => null,
treeRef: { current: null },
});

export const Tree = <T,>({
Expand All @@ -27,6 +28,8 @@ export const Tree = <T,>({
onNodeClick,
selectable,
}: TreeProps<T>): React.ReactElement<TreeProps<T>> => {
const treeRef = useRef<HTMLUListElement>(null);

const initialTreeContext: TreeContextState<T> = {
disabledNodes,
expandable,
Expand All @@ -35,6 +38,7 @@ export const Tree = <T,>({
onKeyDown,
onNodeClick,
selectable,
treeRef,
};

const renderedItems = useMemo(
Expand All @@ -47,6 +51,7 @@ export const Tree = <T,>({
<StyledUl
aria-multiselectable={selectable?.type === 'multi'}
id={id}
ref={treeRef}
role="tree"
style={{ overflow: 'hidden' }}
>
Expand Down
17 changes: 13 additions & 4 deletions packages/big-design/src/components/Tree/TreeNode/TreeNode.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,16 @@ const InternalTreeNode = <T,>({
value,
id,
}: TreeNodeProps<T>): React.ReactElement<TreeNodeProps<T>> => {
const { disabledNodes, expandable, focusable, iconless, onKeyDown, onNodeClick, selectable } =
useContext(TreeContext);
const {
disabledNodes,
expandable,
focusable,
iconless,
onKeyDown,
onNodeClick,
selectable,
treeRef,
} = useContext(TreeContext);
const nodeRef = useRef<HTMLLIElement | null>(null);
const selectableRef = useRef<HTMLLabelElement | null>(null);
const isExpanded = expandable.expandedNodes.includes(id);
Expand All @@ -46,11 +54,12 @@ const InternalTreeNode = <T,>({
if (
focusable.focusedNode === id &&
nodeRef.current !== document.activeElement &&
document.activeElement !== document.body
document.activeElement !== document.body &&
treeRef.current?.contains(document.activeElement)
) {
nodeRef.current?.focus();
}
}, [focusable, id]);
}, [focusable, id, treeRef]);

// Could be multiple elements in which are clicked.
// Typing to generic Element type since all other elements extend from it.
Expand Down
Loading

0 comments on commit 64d102d

Please sign in to comment.