fix: prevent disabled Tag from being removed with keyboard nav (#7394)

* fix: useTag hook

* chore: revert useTag hook

* fix: useTag hook(2)

* refactor: useTag

* test: useTag hook

* rename variable and test behaviour

---------

Co-authored-by: GitHub <[email protected]>
Co-authored-by: Yihui Liao <[email protected]>

Author: 3 people

packages/@react-aria/tag/src/useTag.ts

@@ -61,8 +61,13 @@ export function useTag<T>(props: AriaTagProps<T>, state: ListState<T>, ref: RefO
   // eslint-disable-next-line @typescript-eslint/no-unused-vars
   let {descriptionProps: _, ...stateWithoutDescription} = states;
 
+  let isDisabled = state.disabledKeys.has(item.key) || item.props.isDisabled;
   let onKeyDown = (e: KeyboardEvent) => {
     if (e.key === 'Delete' || e.key === 'Backspace') {
+      if (isDisabled) {
+        return;
+      }
+
       e.preventDefault();
       if (state.selectionManager.isSelected(item.key)) {
         onRemove?.(new Set(state.selectionManager.selectedKeys));
@@ -79,20 +84,26 @@ export function useTag<T>(props: AriaTagProps<T>, state: ListState<T>, ref: RefO
   let description = onRemove && (modality === 'keyboard' || modality === 'virtual') ? stringFormatter.format('removeDescription') : '';
   let descProps = useDescription(description);
 
-  let isFocused = item.key === state.selectionManager.focusedKey;
+  let isItemFocused = item.key === state.selectionManager.focusedKey;
+  let isFocused = state.selectionManager.focusedKey != null;
+  let tabIndex = -1;
+  if (!isDisabled && (isItemFocused || !isFocused)) {
+    tabIndex = 0;
+  }
+
   let domProps = filterDOMProps(item.props);
   let linkProps = useSyntheticLinkProps(item.props);
   return {
     removeButtonProps: {
       'aria-label': stringFormatter.format('removeButtonLabel'),
       'aria-labelledby': `${buttonId} ${rowProps.id}`,
-      isDisabled: state.disabledKeys.has(item.key) || item.props.isDisabled,
+      isDisabled,
       id: buttonId,
       onPress: () => onRemove ? onRemove(new Set([item.key])) : null,
       excludeFromTabOrder: true
     },
     rowProps: mergeProps(rowProps, domProps, linkProps, {
-      tabIndex: (isFocused || state.selectionManager.focusedKey == null) ? 0 : -1,
+      tabIndex,
       onKeyDown: onRemove ? onKeyDown : undefined,
       'aria-describedby': descProps['aria-describedby']
     }),

packages/react-aria-components/test/TagGroup.test.js

@@ -10,8 +10,8 @@
  * governing permissions and limitations under the License.
  */
 
+import {act, fireEvent, mockClickDefault, pointerMap, render} from '@react-spectrum/test-utils-internal';
 import {Button, Label, RouterProvider, Tag, TagGroup, TagList, Text} from '../';
-import {fireEvent, mockClickDefault, pointerMap, render} from '@react-spectrum/test-utils-internal';
 import React from 'react';
 import {useListData} from '@react-stately/data';
 import userEvent from '@testing-library/user-event';
@@ -419,4 +419,34 @@ describe('TagGroup', () => {
     await user.keyboard('{Backspace}');
     expect(grid).toHaveFocus();
   });
+
+  it('disabled tags should not be deletable', async () => {
+    let onRemove = jest.fn();
+    let tree = render(
+      <>
+        <TagGroup data-testid="group" onRemove={onRemove} disabledKeys={['cat', 'dog', 'kangaroo']}>
+          <Label>Test</Label>
+          <TagList>
+            <RemovableTag id="cat">Cat</RemovableTag>
+            <RemovableTag id="dog">Dog</RemovableTag>
+            <RemovableTag id="kangaroo">Kangaroo</RemovableTag>
+          </TagList>
+          <Text slot="description">Description</Text>
+          <Text slot="errorMessage">Error</Text>
+        </TagGroup>
+        <Button>Click here first</Button>
+      </>
+    );
+    let items = tree.getAllByRole('row');
+
+    act(() => items[2].focus());
+    await user.keyboard('{Backspace}');
+
+    expect(onRemove).not.toHaveBeenCalled();
+
+    await user.click(tree.getAllByRole('button')[3]);
+    await user.tab({shift: true});
+
+    expect(document.activeElement).toBe(tree.getByRole('grid'));
+  });
 });