Keep focus on last pressed column header through loading state (#4249)

* Keep focus on last pressed column header through loading state

* fix lint

* add comment

* add manager focused condition

* no tabbing to column headers when table is empty

* cleanup focus if table becomes empty

* fix lint

* fix focus target on blur

* update story to trigger empty state on 2nd press of table column header

* simplify

* fix tests

* fix issue where items cutoff focus ring on entire collection

* Add test

* revert logic to focus first item

* fix test

* fix box shadow cutoff

---------

Co-authored-by: Daniel Lu <[email protected]>
Co-authored-by: Robert Snow <[email protected]>
Co-authored-by: Danni <[email protected]>

Author: 4 people

packages/@adobe/spectrum-css-temp/components/table/skin.css

@@ -103,7 +103,15 @@ governing permissions and limitations under the License.
   background-color: var(--spectrum-table-background-color);
 
   &:focus-ring {
-    box-shadow: inset 0 0 0 2px var(--spectrum-table-cell-border-color-key-focus);
+    &::after {
+      content: "";
+      height: 100%;
+      position: absolute;
+      width: 100%;
+      left: 0px;
+      top: 0px;
+      box-shadow: inset 0 0 0 2px var(--spectrum-table-cell-border-color-key-focus);
+    }
   }
 
   &.is-drop-target {

packages/@react-aria/table/src/useTableColumnHeader.ts

@@ -16,7 +16,7 @@ import {GridNode} from '@react-types/grid';
 // @ts-ignore
 import intlMessages from '../intl/*.json';
 import {isAndroid, mergeProps, useDescription} from '@react-aria/utils';
-import {RefObject} from 'react';
+import {RefObject, useEffect} from 'react';
 import {TableState} from '@react-stately/table';
 import {useFocusable} from '@react-aria/focus';
 import {useGridCell} from '@react-aria/grid';
@@ -80,15 +80,22 @@ export function useTableColumnHeader<T>(props: AriaTableColumnHeaderProps<T>, st
 
   let descriptionProps = useDescription(sortDescription);
 
+  let shouldDisableFocus = state.collection.size === 0;
+  useEffect(() => {
+    if (shouldDisableFocus && state.selectionManager.focusedKey === node.key) {
+      state.selectionManager.setFocusedKey(null);
+    }
+  }, [shouldDisableFocus, state.selectionManager, node.key]);
+
   return {
     columnHeaderProps: {
       ...mergeProps(
         gridCellProps,
         pressProps,
         focusableProps,
         descriptionProps,
-        // If the table is empty, make all column headers untabbable or programatically focusable
-        state.collection.size === 0 && {tabIndex: null}
+        // If the table is empty, make all column headers untabbable
+        shouldDisableFocus && {tabIndex: -1}
       ),
       role: 'columnheader',
       id: getColumnHeaderId(state, node.key),

packages/@react-spectrum/table/stories/Table.stories.tsx

@@ -1805,6 +1805,7 @@ let typeAheadRows = [
   ...Array.from({length: 100}, (v, i) => ({id: i, firstname: 'Aubrey', lastname: 'Sheppard', birthday: 'May 7'})),
   {id: 101, firstname: 'John', lastname: 'Doe', birthday: 'May 7'}
 ];
+
 export const TypeaheadWithDialog: TableStory = {
   render: (args) => (
     <div style={{height: '90vh'}}>
@@ -1844,3 +1845,48 @@ export const TypeaheadWithDialog: TableStory = {
     </div>
   )
 };
+
+export const ColumnHeaderFocusRingTable = {
+  render: () => <LoadingTable />,
+  storyName: 'column header focus after loading',
+  parameters: {
+    description: {
+      data: 'Column header should remain focused even if the table collections empties/loading state changes to loading'
+    }
+  }
+};
+
+const allItems = [
+  {key: 'sam', name: 'Sam', height: 66, birthday: 'May 3'},
+  {key: 'julia', name: 'Julia', height: 70, birthday: 'February 10'}
+];
+
+function LoadingTable() {
+  const [items, setItems] = useState(allItems);
+  const [loadingState, setLoadingState] = useState(undefined);
+  const onSortChange = () => {
+    setItems([]);
+    setLoadingState('loading');
+    setTimeout(() => {
+      setItems(items.length > 1 ? [...items.slice(0, 1)] : []);
+      setLoadingState(undefined);
+    }, 1000);
+  };
+
+  return (
+    <TableView aria-label="Table" selectionMode="multiple" onSortChange={onSortChange} height={300}>
+      <TableHeader>
+        <Column key="name">Name</Column>
+        <Column key="height" allowsSorting>Height</Column>
+        <Column key="birthday">Birthday</Column>
+      </TableHeader>
+      <TableBody items={items} loadingState={loadingState}>
+        {item => (
+          <Row key={item.key}>
+            {column => <Cell>{item[column]}</Cell>}
+          </Row>
+        )}
+      </TableBody>
+    </TableView>
+  );
+}

packages/@react-spectrum/table/test/Table.test.js

@@ -38,7 +38,8 @@ let {
   InlineDeleteButtons: DeletableRowsTable,
   EmptyStateStory: EmptyStateTable,
   WithBreadcrumbNavigation: TableWithBreadcrumbs,
-  TypeaheadWithDialog: TypeaheadWithDialog
+  TypeaheadWithDialog: TypeaheadWithDialog,
+  ColumnHeaderFocusRingTable
 } = composeStories(stories);
 
 
@@ -185,6 +186,9 @@ export let tableTests = () => {
     return el;
   };
 
+  let focusCell = (tree, text) => act(() => getCell(tree, text).focus());
+  let moveFocus = (key, opts = {}) => {fireEvent.keyDown(document.activeElement, {key, ...opts});};
+
   it('renders a static table', function () {
     let {getByRole} = render(
       <TableView aria-label="Table" data-testid="test">
@@ -926,9 +930,6 @@ export let tableTests = () => {
       </TableView>
     );
 
-    let focusCell = (tree, text) => act(() => getCell(tree, text).focus());
-    let moveFocus = (key, opts = {}) => {fireEvent.keyDown(document.activeElement, {key, ...opts});};
-
     describe('ArrowRight', function () {
       it('should move focus to the next cell in a row with ArrowRight', function () {
         let tree = renderTable();
@@ -4449,14 +4450,27 @@ export let tableTests = () => {
       await act(() => Promise.resolve());
       let table = tree.getByRole('grid');
       let header = within(table).getAllByRole('columnheader')[2];
-      expect(header).not.toHaveAttribute('tabindex');
+      expect(header).toHaveAttribute('tabindex', '-1');
       let headerButton = within(header).getByRole('button');
       expect(headerButton).toHaveAttribute('aria-disabled', 'true');
-      // Can't progamatically focus the column headers since they have no tab index when table is empty
-      act(() => {
-        header.focus();
-      });
-      expect(document.activeElement).toBe(document.body);
+    });
+
+    it('should shift focus to the table if table becomes empty via column sort', function () {
+      let tree = render(<ColumnHeaderFocusRingTable />);
+      let rows = tree.getAllByRole('row');
+      expect(rows).toHaveLength(3);
+      focusCell(tree, 'Height');
+      expect(document.activeElement).toHaveTextContent('Height');
+      fireEvent.keyDown(document.activeElement, {key: 'Enter'});
+      fireEvent.keyUp(document.activeElement, {key: 'Enter'});
+      act(() => jest.advanceTimersByTime(500));
+      let table = tree.getByRole('grid');
+      expect(document.activeElement).toBe(table);
+      // Run the rest of the timeout and run the transitions
+      act(() => {jest.runAllTimers();});
+      act(() => {jest.runAllTimers();});
+      rows = tree.getAllByRole('row');
+      expect(rows).toHaveLength(2);
     });
 
     it('should disable press interactions with the column headers', async function () {