fix(treeview): fix switching to Focus Mode inside TreeView (#1453)

cengage · Sep 25, 2024 · 631d010 · 631d010 · github-actions · Sep 25, 2024
1 parent 141d24a
commit 631d010
Show file tree

Hide file tree

Showing 4 changed files with 101 additions and 57 deletions.
diff --git a/.changeset/a11y-treeview-navigation.md b/.changeset/a11y-treeview-navigation.md
@@ -0,0 +1,5 @@
+---
+'react-magma-dom': patch
+---
+
+fix(TreeView): Fix switching to Focus Mode inside `TreeView`.
diff --git a/packages/react-magma-dom/src/components/TreeView/TreeItem.tsx b/packages/react-magma-dom/src/components/TreeView/TreeItem.tsx
@@ -64,6 +64,19 @@ const StyledTreeItem = styled.li<{
   padding-inline-start: ${props =>
     calculateOffset(props.nodeType, props.depth)};
 
+  &:focus {
+    outline: none;
+
+    & > *:first-child {
+      outline-offset: -2px;
+      outline: 2px solid
+      ${props =>
+        props.isInverse
+          ? props.theme.colors.focusInverse
+          : props.theme.colors.focus};
+}
+  }
+
   > div:first-of-type {
     background: ${props =>
       props.selected && props.isInverse
@@ -171,14 +184,6 @@ const StyledItemWrapper = styled.div<{
       props.selectable,
       props.nodeType
     )};
-  &:focus {
-    outline-offset: -2px;
-    outline: 2px solid
-      ${props =>
-        props.isInverse
-          ? props.theme.colors.focusInverse
-          : props.theme.colors.focus};
-  }
 `;
 
 export const TreeItem = React.forwardRef<HTMLLIElement, TreeItemProps>(
@@ -310,6 +315,13 @@ export const TreeItem = React.forwardRef<HTMLLIElement, TreeItemProps>(
           selectableType={selectable}
           selected={selectedItem}
           theme={theme}
+          tabIndex={itemToFocus === itemId ? 0 : -1}
+          onKeyDown={handleKeyDown}
+          onClick={event => {
+            if (selectable===TreeViewSelectable.off && hasOwnTreeItems) {
+              onExpandedClicked(event);
+            }
+          }}
         >
           <StyledItemWrapper
             data-testid={`${testId || itemId}-itemwrapper`}
@@ -318,19 +330,10 @@ export const TreeItem = React.forwardRef<HTMLLIElement, TreeItemProps>(
             isDisabled={isDisabled}
             isInverse={isInverse}
             nodeType={nodeType}
-            onClick={event => {
-              if (selectable === TreeViewSelectable.off && hasOwnTreeItems) {
-                onExpandedClicked(event);
-              }
-            }}
-            onKeyDown={e => {
-              handleKeyDown(e);
-            }}
-            ref={ref}
             selectable={selectable}
             style={style}
-            tabIndex={itemToFocus === itemId ? 0 : -1}
             theme={theme}
+            ref={ref}
           >
             {hasOwnTreeItems && (
               <StyledExpandWrapper

diff --git a/packages/react-magma-dom/src/components/TreeView/TreeView.test.js b/packages/react-magma-dom/src/components/TreeView/TreeView.test.js
@@ -687,8 +687,17 @@ describe('TreeView', () => {
           })
         );
 
-        userEvent.click(getByTestId('item1-label'));
+        const item1 = getByTestId('item1');
+        const item1Label = getByTestId('item1-label');
+
+        userEvent.click(item1Label);
+        expect(onSelectedItemChange).toHaveBeenCalledTimes(1)
         expect(onSelectedItemChange).toHaveBeenCalled();
+        expect(item1).toHaveAttribute('aria-selected', 'true');
+
+        userEvent.click(item1Label);
+        expect(onSelectedItemChange).toHaveBeenCalledTimes(1);
+        expect(item1).toHaveAttribute('aria-selected', 'true');
       });
 
       it('function gets called when it has a preselected item', () => {
@@ -1318,9 +1327,9 @@ describe('TreeView', () => {
           })
         );
 
-        const item1 = getByTestId('item1-itemwrapper');
-        const item2 = getByTestId('item2-itemwrapper');
-        const item3 = getByTestId('item3-itemwrapper');
+        const item1 = getByTestId('item1');
+        const item2 = getByTestId('item2');
+        const item3 = getByTestId('item3');
 
         userEvent.tab();
         expect(item1).toHaveFocus();
@@ -1339,7 +1348,7 @@ describe('TreeView', () => {
 
         // expand item
         fireEvent.keyDown(item1, { key: 'ArrowRight' });
-        const item1child = getByTestId('item-child1-itemwrapper');
+        const item1child = getByTestId('item-child1');
         expect(getByTestId('item1')).toHaveAttribute('aria-expanded', 'true');
         expect(item1).toHaveFocus();
 
@@ -1361,8 +1370,8 @@ describe('TreeView', () => {
           </TreeView>
         );
 
-        const item0 = getByTestId('item0-itemwrapper');
-        const item1 = getByTestId('item1-itemwrapper');
+        const item0 = getByTestId('item0');
+        const item1 = getByTestId('item1');
 
         userEvent.tab();
         expect(item0).toHaveFocus();
@@ -1382,8 +1391,8 @@ describe('TreeView', () => {
           </TreeView>
         );
 
-        const item0 = getByTestId('item0-itemwrapper');
-        const item1 = getByTestId('item1-itemwrapper');
+        const item0 = getByTestId('item0');
+        const item1 = getByTestId('item1');
 
         userEvent.tab();
         expect(item0).toHaveFocus();
@@ -1398,8 +1407,8 @@ describe('TreeView', () => {
       it('should expand the focused branch item when pressing ArrowRight', () => {
         const { getByTestId } = render(getTreeItemsOneLevelSmall({}));
 
-        const item0Wrapper = getByTestId('item0-itemwrapper');
-        const item1Wrapper = getByTestId('item1-itemwrapper');
+        const item0Wrapper = getByTestId('item0');
+        const item1Wrapper = getByTestId('item1');
         const item1 = getByTestId('item1');
 
         userEvent.tab();
@@ -1416,9 +1425,9 @@ describe('TreeView', () => {
           getTreeItemsOneLevelSmall({ initialExpandedItems: ['item1'] })
         );
 
-        const item0 = getByTestId('item0-itemwrapper');
-        const item1 = getByTestId('item1-itemwrapper');
-        const item1child = getByTestId('item-child1-itemwrapper');
+        const item0 = getByTestId('item0');
+        const item1 = getByTestId('item1');
+        const item1child = getByTestId('item-child1');
 
         userEvent.tab();
         fireEvent.keyDown(item0, { key: 'ArrowDown' });
@@ -1430,7 +1439,7 @@ describe('TreeView', () => {
       it('should maintain focus when pressing ArrowRight on a leaf item', () => {
         const { getByTestId } = render(getTreeItemsOneLevelSmall({}));
 
-        const item0 = getByTestId('item0-itemwrapper');
+        const item0 = getByTestId('item0');
 
         userEvent.tab();
         fireEvent.keyDown(item0, { key: 'ArrowRight' });
@@ -1443,8 +1452,8 @@ describe('TreeView', () => {
           getTreeItemsOneLevelSmall({ initialExpandedItems: ['item1'] })
         );
 
-        const item0 = getByTestId('item0-itemwrapper');
-        const item1 = getByTestId('item1-itemwrapper');
+        const item0 = getByTestId('item0');
+        const item1 = getByTestId('item1');
 
         userEvent.tab();
 
@@ -1458,7 +1467,7 @@ describe('TreeView', () => {
       it('should maintain focus when pressing ArrowLeft on a leaf item', () => {
         const { getByTestId } = render(getTreeItemsOneLevelSmall({}));
 
-        const item0 = getByTestId('item0-itemwrapper');
+        const item0 = getByTestId('item0');
 
         userEvent.tab();
         fireEvent.keyDown(item0, { key: 'ArrowLeft' });
@@ -1469,8 +1478,8 @@ describe('TreeView', () => {
       it('should focus to the first item when pressing the Home key', () => {
         const { getByTestId } = render(getTreeItemsOneLevelSmall({}));
 
-        const item0 = getByTestId('item0-itemwrapper');
-        const item1 = getByTestId('item1-itemwrapper');
+        const item0 = getByTestId('item0');
+        const item1 = getByTestId('item1');
 
         userEvent.tab();
         fireEvent.focus(item1);
@@ -1484,8 +1493,8 @@ describe('TreeView', () => {
           getTreeItemsOneLevelSmall({ initialExpandedItems: ['item1'] })
         );
 
-        const item0 = getByTestId('item0-itemwrapper');
-        const item1Child = getByTestId('item-child1-itemwrapper');
+        const item0 = getByTestId('item0');
+        const item1Child = getByTestId('item-child1');
 
         userEvent.tab();
         fireEvent.focus(item0);
@@ -1499,9 +1508,9 @@ describe('TreeView', () => {
           getTreeItemsOneLevel({ initialExpandedItems: ['item-3'] })
         );
 
-        const item0 = getByTestId('item0-itemwrapper');
-        const item2 = getByTestId('item2-itemwrapper');
-        const item3 = getByTestId('item3-itemwrapper');
+        const item0 = getByTestId('item0');
+        const item2 = getByTestId('item2');
+        const item3 = getByTestId('item3');
 
         userEvent.tab();
         fireEvent.keyDown(item0, { key: 'End' });
@@ -1543,7 +1552,7 @@ describe('TreeView', () => {
               <TreeItem label="Node 1" itemId="item1" testId="item1" />
             </TreeView>
           );
-          const item0 = getByTestId('item0-itemwrapper');
+          const item0 = getByTestId('item0');
 
           userEvent.tab();
           expect(item0).toHaveFocus();
@@ -1553,7 +1562,7 @@ describe('TreeView', () => {
           const { getByTestId } = render(
             getTreeItemsOneLevelSmall({ selectable: TreeViewSelectable.off })
           );
-          const item1 = getByTestId('item1-itemwrapper');
+          const item1 = getByTestId('item1');
 
           userEvent.tab();
           expect(item1).toHaveFocus();
@@ -1567,7 +1576,7 @@ describe('TreeView', () => {
           const { getByTestId } = render(
             getTreeItemsOneLevelSmall({ selectable: TreeViewSelectable.single })
           );
-          const item0 = getByTestId('item0-itemwrapper');
+          const item0 = getByTestId('item0');
 
           userEvent.tab();
           expect(item0).toHaveFocus();
@@ -1586,7 +1595,7 @@ describe('TreeView', () => {
               initialExpandedItems: ['item1'],
             })
           );
-          const item1Child = getByTestId('item-child1-itemwrapper');
+          const item1Child = getByTestId('item-child1');
 
           userEvent.tab();
           expect(item1Child).toHaveFocus();
@@ -1612,12 +1621,18 @@ describe('TreeView', () => {
           fireEvent.keyDown(item1wrapper, { key: 'Enter' });
 
           expect(item1).toHaveAttribute('aria-selected', 'true');
+          expect(onSelectedItemChange).toHaveBeenCalledTimes(1)
           expect(onSelectedItemChange).toHaveBeenCalledWith([
             {
               itemId: 'item1',
               checkedStatus: IndeterminateCheckboxStatus.checked,
             },
           ]);
+
+          fireEvent.keyDown(item1wrapper, { key: 'Enter' });
+
+          expect(item1).toHaveAttribute('aria-selected', 'true');
+          expect(onSelectedItemChange).toHaveBeenCalledTimes(1)
         });
 
         it('should select the leaf item when pressing the Space key', () => {
@@ -1638,12 +1653,18 @@ describe('TreeView', () => {
           fireEvent.keyDown(item0wrapper, { key: ' ' });
 
           expect(item0).toHaveAttribute('aria-selected', 'true');
+          expect(onSelectedItemChange).toHaveBeenCalledTimes(1)
           expect(onSelectedItemChange).toHaveBeenCalledWith([
             {
               itemId: 'item0',
               checkedStatus: IndeterminateCheckboxStatus.checked,
             },
           ]);
+
+          fireEvent.keyDown(item0wrapper, { key: ' ' });
+
+          expect(item0).toHaveAttribute('aria-selected', 'true');
+          expect(onSelectedItemChange).toHaveBeenCalledTimes(1)
         });
 
         it('should toggle expand the branch item when pressing the Space key', () => {
@@ -1686,7 +1707,7 @@ describe('TreeView', () => {
           const { getByTestId } = render(
             getTreeItemsOneLevelSmall({ selectable: TreeViewSelectable.multi })
           );
-          const item0 = getByTestId('item0-itemwrapper');
+          const item0 = getByTestId('item0');
 
           userEvent.tab();
           expect(item0).toHaveFocus();
@@ -1708,7 +1729,7 @@ describe('TreeView', () => {
               ],
             })
           );
-          const item1 = getByTestId('item1-itemwrapper');
+          const item1 = getByTestId('item1');
 
           userEvent.tab();
           expect(item1).toHaveFocus();