Announce removed selections due to table navigation (#2441)

* Announce removed selections due to table navigation

* Update our last selected when not focused

* code review

* renaming table breadcrumb story and component

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

Author: 3 people

packages/@react-aria/grid/src/useGrid.ts

@@ -117,6 +117,8 @@ export function useGrid<T>(props: GridProps, state: GridState<T, GridCollection<
     // Do not do this when using selectionBehavior = 'replace' to avoid selection announcements
     // every time the user presses the arrow keys.
     if (!state.selectionManager.isFocused || state.selectionManager.selectionBehavior === 'replace') {
+      lastSelection.current = selection;
+
       return;
     }
 
@@ -131,9 +133,11 @@ export function useGrid<T>(props: GridProps, state: GridState<T, GridCollection<
         messages.push(formatMessage('selectedItem', {item: addedText}));
       }
     } else if (removedKeys.size === 1 && addedKeys.size === 0) {
-      let removedText = getRowText(removedKeys.keys().next().value);
-      if (removedText) {
-        messages.push(formatMessage('deselectedItem', {item: removedText}));
+      if (state.collection.getItem(removedKeys.keys().next().value)) {
+        let removedText = getRowText(removedKeys.keys().next().value);
+        if (removedText) {
+          messages.push(formatMessage('deselectedItem', {item: removedText}));
+        }
       }
     }
 

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

@@ -64,8 +64,11 @@ export function useTable<T>(props: TableProps<T>, state: TableState<T>, ref: Ref
     ...props,
     id,
     keyboardDelegate: delegate,
-    getRowText(key) {
+    getRowText(key): string {
       let added = state.collection.getItem(key);
+      if (!added) {
+        return '';
+      }
 
       // If the row has a textValue, use that.
       if (added.textValue != null) {

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

@@ -13,6 +13,7 @@
 import {action} from '@storybook/addon-actions';
 import {ActionButton, Button} from '@react-spectrum/button';
 import Add from '@spectrum-icons/workflow/Add';
+import {Breadcrumbs, Item} from '@react-spectrum/breadcrumbs';
 import {ButtonGroup} from '@react-spectrum/buttongroup';
 import {Cell, Column, Row, TableBody, TableHeader, TableView} from '../';
 import {Content} from '@react-spectrum/view';
@@ -25,10 +26,10 @@ import {Heading} from '@react-spectrum/text';
 import {HidingColumns} from './HidingColumns';
 import {IllustratedMessage} from '@react-spectrum/illustratedmessage';
 import {Link} from '@react-spectrum/link';
+import {LoadingState, SelectionMode} from '@react-types/shared';
 import {Radio, RadioGroup} from '@react-spectrum/radio';
 import React, {Key, useState} from 'react';
 import {SearchField} from '@react-spectrum/searchfield';
-import {SelectionMode} from '@react-types/shared';
 import {storiesOf} from '@storybook/react';
 import {Switch} from '@react-spectrum/switch';
 import {TextField} from '@react-spectrum/textfield';
@@ -1020,7 +1021,8 @@ storiesOf('TableView', module)
         </TableBody>
       </TableView>
     )
-  );
+  )
+  .add('table with breadcrumb navigation', () => <TableWithBreadcrumbs />);
 
 function AsyncLoadingExample() {
   interface Item {
@@ -1299,3 +1301,73 @@ function ChangableSelectionMode() {
     </Flex>
   );
 }
+
+export function TableWithBreadcrumbs() {
+  const fs = [
+    {key: 'a', name: 'Folder A', type: 'folder'},
+    {key: 'b', name: 'File B', value: '10 MB'},
+    {key: 'c', name: 'File C', value: '10 MB', parent: 'a'},
+    {key: 'd', name: 'File D', value: '10 MB', parent: 'a'}
+  ];
+
+  const [loadingState, setLoadingState] = useState<LoadingState>('idle' as 'idle');
+  const [selection, setSelection] = useState<'all' | Iterable<Key>>(new Set([]));
+  const [items, setItems] = useState(() => fs.filter(item => !item.parent));
+
+  const changeFolder = (folder) => {
+    setItems([]);
+    setLoadingState('loading' as 'loading');
+
+    // mimic loading behavior
+    setTimeout(() => {
+      setLoadingState('idle');
+      setItems(fs.filter(item =>  folder ? item.parent === folder : !item.parent));
+    }, 700);
+    setSelection(new Set([]));
+  };
+
+  return (
+    <Flex direction="column" width="400px">
+      <div>The TableView should not error if row selection changes due to items changes from external navigation (breadcrumbs).</div>
+      <Breadcrumbs
+        onAction={item => {
+          if (item === 'root') {
+            changeFolder('');
+          }
+        }}>
+        <Item key="root">root</Item>
+        <Item>a</Item>
+      </Breadcrumbs>
+      <TableView
+        width="400px"
+        aria-label="table"
+        selectedKeys={selection}
+        selectionMode="multiple"
+        onSelectionChange={(sel) => setSelection(sel)}>
+        <TableHeader>
+          <Column key="name" isRowHeader>Name</Column>
+          <Column key="value">Value</Column>
+        </TableHeader>
+        <TableBody items={items} loadingState={loadingState}>
+          {(item) => (
+            <Row key={item.key}>
+              {(column) => {
+                if (item.type === 'folder' && column === 'name') {
+                  return (
+                    <Cell textValue={item[column]}>
+                      <Link onPress={() => changeFolder(item.key)}>
+                        {item[column]}
+                      </Link>
+                    </Cell>
+                  );
+                }
+                return <Cell>{item[column]}</Cell>;
+              }}
+            </Row>
+          )}
+        </TableBody>
+      </TableView>
+      <ActionButton onPress={() => setSelection(items.some(item => item.key === 'd') ? new Set(['d']) : new Set([]))}>Select D</ActionButton>
+    </Flex>
+  );
+}

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

@@ -29,6 +29,7 @@ import {Link} from '@react-spectrum/link';
 import {Provider} from '@react-spectrum/provider';
 import React from 'react';
 import {Switch} from '@react-spectrum/switch';
+import {TableWithBreadcrumbs} from '../stories/Table.stories';
 import {TextField} from '@react-spectrum/textfield';
 import {theme} from '@react-spectrum/theme-default';
 import {typeText} from '@react-spectrum/test-utils';
@@ -2280,6 +2281,85 @@ describe('TableView', function () {
       });
     });
 
+    it('can announce deselect even when items are swapped out completely', () => {
+      let tree = render(<TableWithBreadcrumbs />);
+
+      let row = tree.getAllByRole('row')[2];
+      triggerPress(row);
+      expect(announce).toHaveBeenLastCalledWith('File B selected.');
+
+      let link = tree.getAllByRole('link')[1];
+      triggerPress(link);
+
+      expect(announce).toHaveBeenLastCalledWith('No items selected.');
+      expect(announce).toHaveBeenCalledTimes(2);
+    });
+
+    it('will not announce deselect caused by breadcrumb navigation', () => {
+      let tree = render(<TableWithBreadcrumbs />);
+
+      let link = tree.getAllByRole('link')[1];
+      triggerPress(link);
+
+      act(() => {
+        // TableWithBreadcrumbs has a setTimeout to load the results of the link navigation on Folder A
+        jest.runAllTimers();
+      });
+      let row = tree.getAllByRole('row')[1];
+      triggerPress(row);
+      expect(announce).toHaveBeenLastCalledWith('File C selected.');
+      expect(announce).toHaveBeenCalledTimes(2);
+
+      // breadcrumb root
+      link = tree.getAllByRole('link')[0];
+      triggerPress(link);
+
+      // focus isn't on the table, so we don't announce that it has been deselected
+      expect(announce).toHaveBeenCalledTimes(2);
+    });
+
+    it('updates even if not focused', () => {
+      let tree = render(<TableWithBreadcrumbs />);
+
+      let link = tree.getAllByRole('link')[1];
+      triggerPress(link);
+
+      act(() => {
+        // TableWithBreadcrumbs has a setTimeout to load the results of the link navigation on Folder A
+        jest.runAllTimers();
+      });
+      let row = tree.getAllByRole('row')[1];
+      triggerPress(row);
+      expect(announce).toHaveBeenLastCalledWith('File C selected.');
+      expect(announce).toHaveBeenCalledTimes(2);
+      let button = tree.getAllByRole('button')[0];
+      triggerPress(button);
+      expect(announce).toHaveBeenCalledTimes(2);
+
+      // breadcrumb root
+      link = tree.getAllByRole('menuitemradio')[0];
+      triggerPress(link);
+
+      act(() => {
+        // TableWithBreadcrumbs has a setTimeout to load the results of the link navigation on Folder A
+        jest.runAllTimers();
+      });
+
+      // focus isn't on the table, so we don't announce that it has been deselected
+      expect(announce).toHaveBeenCalledTimes(2);
+
+      link = tree.getAllByRole('link')[1];
+      triggerPress(link);
+
+      act(() => {
+        // TableWithBreadcrumbs has a setTimeout to load the results of the link navigation on Folder A
+        jest.runAllTimers();
+      });
+
+      expect(announce).toHaveBeenCalledTimes(3);
+      expect(announce).toHaveBeenLastCalledWith('No items selected.');
+    });
+
     describe('selectionStyle highlight', function () {
       installPointerEvent();
 
@@ -2747,7 +2827,7 @@ describe('TableView', function () {
   });
 
   describe('press/hover interactions and selection mode', function () {
-    let TableExample = (props) => (
+    let TableWithBreadcrumbs = (props) => (
       <TableView aria-label="Table" {...props}>
         <TableHeader columns={columns}>
           {column => <Column>{column.name}</Column>}
@@ -2763,15 +2843,15 @@ describe('TableView', function () {
     );
 
     it('displays pressed/hover styles when row is pressed/hovered and selection mode is not "none"', function () {
-      let tree = render(<TableExample selectionMode="multiple" />);
+      let tree = render(<TableWithBreadcrumbs selectionMode="multiple" />);
 
       let row = tree.getAllByRole('row')[1];
       fireEvent.mouseDown(row, {detail: 1});
       expect(row.className.includes('is-active')).toBeTruthy();
       fireEvent.mouseEnter(row);
       expect(row.className.includes('is-hovered')).toBeTruthy();
 
-      rerender(tree, <TableExample selectionMode="single" />);
+      rerender(tree, <TableWithBreadcrumbs selectionMode="single" />);
       row = tree.getAllByRole('row')[1];
       fireEvent.mouseDown(row, {detail: 1});
       expect(row.className.includes('is-active')).toBeTruthy();
@@ -2780,7 +2860,7 @@ describe('TableView', function () {
     });
 
     it('doesn\'t show pressed/hover styles when row is pressed/hovered and selection mode is "none"', function () {
-      let tree = render(<TableExample selectionMode="none" />);
+      let tree = render(<TableWithBreadcrumbs selectionMode="none" />);
 
       let row = tree.getAllByRole('row')[1];
       fireEvent.mouseDown(row, {detail: 1});
@@ -3084,7 +3164,7 @@ describe('TableView', function () {
   });
 
   describe('with dialog trigger', function () {
-    let TableExample = (props) => (
+    let TableWithBreadcrumbs = (props) => (
       <TableView aria-label="TableView with static contents" selectionMode="multiple" width={300} height={200} {...props}>
         <TableHeader>
           <Column key="foo">Foo</Column>
@@ -3119,7 +3199,7 @@ describe('TableView', function () {
     );
 
     it('arrow keys interactions don\'t move the focus away from the textfield in the dialog', function () {
-      let tree = render(<TableExample />);
+      let tree = render(<TableWithBreadcrumbs />);
       let table = tree.getByRole('grid');
       let rows = within(table).getAllByRole('row');
       expect(rows).toHaveLength(2);