From 07bdb89599a58d4e7892e4cb80c6d3fad6f4fc7c Mon Sep 17 00:00:00 2001 From: Daniel Lu Date: Fri, 19 Aug 2022 12:54:26 -0700 Subject: [PATCH 1/9] disabling grid navigation if collection is empty --- packages/@react-aria/grid/src/useGrid.ts | 2 +- .../table/src/useTableSelectionCheckbox.ts | 2 +- .../card/stories/GridCardView.stories.tsx | 4 +- .../@react-spectrum/table/src/TableView.tsx | 1 - .../table/stories/Table.stories.tsx | 38 +++++++++++++------ 5 files changed, 30 insertions(+), 17 deletions(-) diff --git a/packages/@react-aria/grid/src/useGrid.ts b/packages/@react-aria/grid/src/useGrid.ts index badf65ddea2..8c1838f3ef2 100644 --- a/packages/@react-aria/grid/src/useGrid.ts +++ b/packages/@react-aria/grid/src/useGrid.ts @@ -114,7 +114,7 @@ export function useGrid(props: GridProps, state: GridState(state: TableState): TableSelectA checkboxProps: { 'aria-label': stringFormatter.format(selectionMode === 'single' ? 'select' : 'selectAll'), isSelected: isSelectAll, - isDisabled: selectionMode !== 'multiple', + isDisabled: selectionMode !== 'multiple' || state.collection.size === 0, isIndeterminate: !isEmpty && !isSelectAll, onChange: () => state.selectionManager.toggleSelectAll() } diff --git a/packages/@react-spectrum/card/stories/GridCardView.stories.tsx b/packages/@react-spectrum/card/stories/GridCardView.stories.tsx index 9d3c9176750..77e6bebef05 100644 --- a/packages/@react-spectrum/card/stories/GridCardView.stories.tsx +++ b/packages/@react-spectrum/card/stories/GridCardView.stories.tsx @@ -21,6 +21,7 @@ import {GridLayoutOptions} from '../src/GridLayout'; import {Heading, Text} from '@react-spectrum/text'; import {IllustratedMessage} from '@react-spectrum/illustratedmessage'; import {Image} from '@react-spectrum/image'; +import {Link} from '@react-spectrum/link'; import React, {Key, useMemo, useState} from 'react'; import {Size} from '@react-stately/virtualizer'; import {SpectrumCardViewProps} from '@react-types/card'; @@ -67,7 +68,7 @@ function renderEmptyState() { No results - No results found + No results found, press here for more info. ); } @@ -527,4 +528,3 @@ export function CustomLayout(props: SpectrumCardViewProps & LayoutOption ); } - diff --git a/packages/@react-spectrum/table/src/TableView.tsx b/packages/@react-spectrum/table/src/TableView.tsx index 3a2483c0d5f..6982f66e309 100644 --- a/packages/@react-spectrum/table/src/TableView.tsx +++ b/packages/@react-spectrum/table/src/TableView.tsx @@ -685,7 +685,6 @@ function TableSelectAllCell({column}) { } diff --git a/packages/@react-spectrum/table/stories/Table.stories.tsx b/packages/@react-spectrum/table/stories/Table.stories.tsx index 7ce6b67fe37..fac9412a94a 100644 --- a/packages/@react-spectrum/table/stories/Table.stories.tsx +++ b/packages/@react-spectrum/table/stories/Table.stories.tsx @@ -96,7 +96,7 @@ function renderEmptyState() { No results - No results found + No results found, press here for more info. ); } @@ -935,16 +935,7 @@ storiesOf('TableView', module) .add( 'renderEmptyState', () => ( - - - {column => - {column.name} - } - - - {[]} - - + ) ) .add( @@ -1651,7 +1642,6 @@ export function TableWithBreadcrumbs() { const [loadingState, setLoadingState] = useState('idle' as 'idle'); const [selection, setSelection] = useState<'all' | Iterable>(new Set([])); const [items, setItems] = useState(() => fs.filter(item => !item.parent)); - const changeFolder = (folder) => { setItems([]); setLoadingState('loading' as 'loading'); @@ -1709,3 +1699,27 @@ export function TableWithBreadcrumbs() { ); } + +function EmptyStateTable() { + let [show, setShow] = useState(false); + + return ( + + setShow(show => !show)}>Toggle items + + + {column => + {column.name} + } + + + {item => + ( + {key => {item[key]}} + ) + } + + + + ); +} From e1d27e4a8ede205e37a334bc159296353fea50f2 Mon Sep 17 00:00:00 2001 From: Daniel Lu Date: Mon, 22 Aug 2022 17:58:51 -0700 Subject: [PATCH 2/9] disabling grid cell, rows, and other actions when table is empty this makes resizing, clicking on column header and other interactions all inert if the table is empty. Partially makes these changes TableView specific instead of grid wide, to confirm with team. --- .../spectrum-css-temp/components/table/skin.css | 2 +- packages/@react-aria/grid/src/useGrid.ts | 2 +- packages/@react-aria/grid/src/useGridCell.ts | 5 ++++- packages/@react-aria/grid/src/useGridRow.ts | 5 ++++- packages/@react-aria/gridlist/src/useGridList.ts | 2 +- .../list/stories/ListView.stories.tsx | 3 ++- packages/@react-spectrum/table/src/TableView.tsx | 12 ++++++++---- .../@react-spectrum/table/stories/Table.stories.tsx | 2 +- packages/@react-stately/table/src/useTableState.ts | 2 +- 9 files changed, 23 insertions(+), 12 deletions(-) diff --git a/packages/@adobe/spectrum-css-temp/components/table/skin.css b/packages/@adobe/spectrum-css-temp/components/table/skin.css index 1c20d9b89b7..4f83b09fd11 100644 --- a/packages/@adobe/spectrum-css-temp/components/table/skin.css +++ b/packages/@adobe/spectrum-css-temp/components/table/skin.css @@ -41,7 +41,7 @@ governing permissions and limitations under the License. } } - &:active { + &.is-active { color: var(--spectrum-table-header-text-color-down); .spectrum-Table-sortedIcon { diff --git a/packages/@react-aria/grid/src/useGrid.ts b/packages/@react-aria/grid/src/useGrid.ts index 8c1838f3ef2..badf65ddea2 100644 --- a/packages/@react-aria/grid/src/useGrid.ts +++ b/packages/@react-aria/grid/src/useGrid.ts @@ -114,7 +114,7 @@ export function useGrid(props: GridProps, state: GridState>(props: GridCellProps isVirtualized, focus, shouldSelectOnPressUp, - onAction: onCellAction ? () => onCellAction(node.key) : onAction + onAction: onCellAction ? () => onCellAction(node.key) : onAction, + // TODO: this will affect all grids, maybe make it specific to Table via useTableState? + // something like isInteractionDisabled: state.collection.size === 0 + isDisabled: state.collection.size === 0 }); let onKeyDownCapture = (e: ReactKeyboardEvent) => { diff --git a/packages/@react-aria/grid/src/useGridRow.ts b/packages/@react-aria/grid/src/useGridRow.ts index 0785d3b453f..5c0838334df 100644 --- a/packages/@react-aria/grid/src/useGridRow.ts +++ b/packages/@react-aria/grid/src/useGridRow.ts @@ -59,7 +59,10 @@ export function useGridRow, S extends GridState onRowAction(node.key) : onAction + onAction: onRowAction ? () => onRowAction(node.key) : onAction, + // TODO: this will affect all grids, maybe make it specific to Table via useTableState? + // something like isInteractionDisabled: state.collection.size === 0 + isDisabled: state.collection.size === 0 }); let isSelected = state.selectionManager.isSelected(node.key); diff --git a/packages/@react-aria/gridlist/src/useGridList.ts b/packages/@react-aria/gridlist/src/useGridList.ts index 1f14c11d67d..69fbdd176f6 100644 --- a/packages/@react-aria/gridlist/src/useGridList.ts +++ b/packages/@react-aria/gridlist/src/useGridList.ts @@ -84,7 +84,7 @@ export function useGridList(props: AriaGridListOptions, state: ListState No results - No results found + No results found, press here for more info. ); } diff --git a/packages/@react-spectrum/table/src/TableView.tsx b/packages/@react-spectrum/table/src/TableView.tsx index 6982f66e309..fd27e6a2dff 100644 --- a/packages/@react-spectrum/table/src/TableView.tsx +++ b/packages/@react-spectrum/table/src/TableView.tsx @@ -513,8 +513,9 @@ function TableColumnHeader(props) { } let _TableColumnHeaderButton = (props, ref: FocusableRef) => { + let {state} = useTableContext(); let domRef = useFocusableRef(ref); - let {buttonProps} = useButton({...props, elementType: 'div'}, domRef); + let {buttonProps} = useButton({...props, elementType: 'div', isDisabled: state.collection.size === 0}, domRef); return (
@@ -532,14 +533,16 @@ function ResizableTableColumnHeader(props) { let resizingRef = useRef(null); let {state, columnState, headerRowHovered} = useTableContext(); let stringFormatter = useLocalizedStringFormatter(intlMessages); + let isEmpty = state.collection.size === 0; + let {pressProps, isPressed} = usePress({isDisabled: isEmpty}); let {columnHeaderProps} = useTableColumnHeader({ node: column, isVirtualized: true, hasMenu: true }, state, ref); - let {hoverProps, isHovered} = useHover(props); + let {hoverProps, isHovered} = useHover({...props, isDisabled: isEmpty}); - const allProps = [columnHeaderProps, hoverProps]; + const allProps = [columnHeaderProps, hoverProps, pressProps]; let columnProps = column.props as SpectrumColumnProps; @@ -589,7 +592,7 @@ function ResizableTableColumnHeader(props) { } }, [columnState.currentlyResizingColumn, column.key]); - let showResizer = headerRowHovered || columnState.currentlyResizingColumn != null; + let showResizer = !isEmpty && (headerRowHovered || columnState.currentlyResizingColumn != null); return ( @@ -601,6 +604,7 @@ function ResizableTableColumnHeader(props) { styles, 'spectrum-Table-headCell', { + 'is-active': isPressed, 'is-resizable': columnProps.allowsResizing, 'is-sortable': columnProps.allowsSorting, 'is-sorted-desc': state.sortDescriptor?.column === column.key && state.sortDescriptor?.direction === 'descending', diff --git a/packages/@react-spectrum/table/stories/Table.stories.tsx b/packages/@react-spectrum/table/stories/Table.stories.tsx index fac9412a94a..8374dd2bf05 100644 --- a/packages/@react-spectrum/table/stories/Table.stories.tsx +++ b/packages/@react-spectrum/table/stories/Table.stories.tsx @@ -1709,7 +1709,7 @@ function EmptyStateTable() { {column => - {column.name} + {column.name} } diff --git a/packages/@react-stately/table/src/useTableState.ts b/packages/@react-stately/table/src/useTableState.ts index f88d3c16a25..4466db543ee 100644 --- a/packages/@react-stately/table/src/useTableState.ts +++ b/packages/@react-stately/table/src/useTableState.ts @@ -76,7 +76,7 @@ export function useTableState(props: TableStateProps): Tabl selectionManager, showSelectionCheckboxes: props.showSelectionCheckboxes || false, sortDescriptor: props.sortDescriptor, - isKeyboardNavigationDisabled, + isKeyboardNavigationDisabled: collection.size === 0 || isKeyboardNavigationDisabled, setKeyboardNavigationDisabled, sort(columnKey: Key, direction?: 'ascending' | 'descending') { props.onSortChange({ From 754ed24525fcb7ca2722a0035d8e7b9d36864d82 Mon Sep 17 00:00:00 2001 From: Daniel Lu Date: Tue, 23 Aug 2022 10:58:40 -0700 Subject: [PATCH 3/9] add focus ring to empty state table --- .../spectrum-css-temp/components/table/skin.css | 4 ++++ packages/@react-spectrum/table/src/TableView.tsx | 16 +++++++++++++--- .../table/stories/Table.stories.tsx | 2 +- 3 files changed, 18 insertions(+), 4 deletions(-) diff --git a/packages/@adobe/spectrum-css-temp/components/table/skin.css b/packages/@adobe/spectrum-css-temp/components/table/skin.css index 4f83b09fd11..67f307319ee 100644 --- a/packages/@adobe/spectrum-css-temp/components/table/skin.css +++ b/packages/@adobe/spectrum-css-temp/components/table/skin.css @@ -77,6 +77,10 @@ governing permissions and limitations under the License. border-color: var(--spectrum-table-border-color); background-color: var(--spectrum-table-background-color); + &:focus-ring { + box-shadow: inset 0 0 0 2px var(--spectrum-table-cell-border-color-key-focus); + } + &.is-drop-target { border-color: var(--spectrum-alias-border-color-focus); box-shadow: 0 0 0 1px var(--spectrum-alias-border-color-focus); diff --git a/packages/@react-spectrum/table/src/TableView.tsx b/packages/@react-spectrum/table/src/TableView.tsx index fd27e6a2dff..aff6f706170 100644 --- a/packages/@react-spectrum/table/src/TableView.tsx +++ b/packages/@react-spectrum/table/src/TableView.tsx @@ -304,11 +304,12 @@ function TableView(props: SpectrumTableProps, ref: DOMRef (props: SpectrumTableProps, ref: DOMRef ); } // This is a custom Virtualizer that also has a header that syncs its scroll position with the body. -function TableVirtualizer({layout, collection, focusedKey, renderView, renderWrapper, domRef, bodyRef, setTableWidth, getColumnWidth, onVisibleRectChange: onVisibleRectChangeProp, ...otherProps}) { +function TableVirtualizer({layout, collection, focusedKey, renderView, renderWrapper, domRef, bodyRef, setTableWidth, getColumnWidth, onVisibleRectChange: onVisibleRectChangeProp, isFocusVisible, ...otherProps}) { let {direction} = useLocale(); let headerRef = useRef(); let loadingState = collection.body.props.loadingState; @@ -433,7 +435,15 @@ function TableVirtualizer({layout, collection, focusedKey, renderView, renderWra
setShow(show => !show)}>Toggle items - + {column => {column.name} } From 378542d4c5ff6ff5b1325cedbc1955d81e9a903e Mon Sep 17 00:00:00 2001 From: Daniel Lu Date: Tue, 23 Aug 2022 14:35:52 -0700 Subject: [PATCH 4/9] adding tests for ListView and TableView empty state also fixes some small things the tests revealed like how the hidden inputs in the resizer werent disabled --- .../table/src/useTableColumnResize.ts | 8 +- .../list/stories/ListView.stories.tsx | 2 +- .../list/test/ListView.test.js | 14 +- .../@react-spectrum/table/src/Resizer.tsx | 4 +- .../@react-spectrum/table/src/TableView.tsx | 13 +- .../table/stories/Table.stories.tsx | 4 +- .../@react-spectrum/table/test/Table.test.js | 216 ++++++++++-------- 7 files changed, 155 insertions(+), 106 deletions(-) diff --git a/packages/@react-aria/table/src/useTableColumnResize.ts b/packages/@react-aria/table/src/useTableColumnResize.ts index 5adfb7c51c2..7054c98a53a 100644 --- a/packages/@react-aria/table/src/useTableColumnResize.ts +++ b/packages/@react-aria/table/src/useTableColumnResize.ts @@ -30,11 +30,12 @@ export interface TableColumnResizeAria { export interface AriaTableColumnResizeProps { column: GridNode, label: string, - triggerRef: RefObject + triggerRef: RefObject, + isDisabled?: boolean } export function useTableColumnResize(props: AriaTableColumnResizeProps, state: TableState, columnState: TableColumnResizeState, ref: RefObject): TableColumnResizeAria { - let {column: item, triggerRef} = props; + let {column: item, triggerRef, isDisabled} = props; const stateRef = useRef>(null); // keep track of what the cursor on the body is so it can be restored back to that when done resizing const cursor = useRef(null); @@ -159,7 +160,8 @@ export function useTableColumnResize(props: AriaTableColumnResizeProps, st stateRef.current.onColumnResizeEnd(item); state.setKeyboardNavigationDisabled(false); }, - onChange + onChange, + disabled: isDisabled }, ariaProps ) diff --git a/packages/@react-spectrum/list/stories/ListView.stories.tsx b/packages/@react-spectrum/list/stories/ListView.stories.tsx index b07b4b87625..081941025c5 100644 --- a/packages/@react-spectrum/list/stories/ListView.stories.tsx +++ b/packages/@react-spectrum/list/stories/ListView.stories.tsx @@ -108,7 +108,7 @@ const itemsWithThumbs = [ {key: '9', title: 'file of great boi', illustration: } ]; -function renderEmptyState() { +export function renderEmptyState() { return ( diff --git a/packages/@react-spectrum/list/test/ListView.test.js b/packages/@react-spectrum/list/test/ListView.test.js index 3875e23c49b..2dd1f4baeb8 100644 --- a/packages/@react-spectrum/list/test/ListView.test.js +++ b/packages/@react-spectrum/list/test/ListView.test.js @@ -17,6 +17,7 @@ import {announce} from '@react-aria/live-announcer'; import {Item, ListView} from '../src'; import {Provider} from '@react-spectrum/provider'; import React from 'react'; +import {renderEmptyState} from '../stories/ListView.stories'; import {Text} from '@react-spectrum/text'; import {theme} from '@react-spectrum/theme-default'; import userEvent from '@testing-library/user-event'; @@ -570,13 +571,20 @@ describe('ListView', function () { }); it('should render empty state', function () { - function renderEmptyState() { - return
No results
; - } let {getByText} = render(); expect(getByText('No results')).toBeTruthy(); }); + it('should allow you to tab into ListView body if empty', function () { + let {getByRole} = render(); + let grid = getByRole('grid'); + let link = getByRole('link'); + userEvent.tab(); + expect(document.activeElement).toBe(grid); + userEvent.tab(); + expect(document.activeElement).toBe(link); + }); + it('supports custom data attributes', () => { let {getByRole} = render( diff --git a/packages/@react-spectrum/table/src/Resizer.tsx b/packages/@react-spectrum/table/src/Resizer.tsx index fe7e33ffe53..910b432bf61 100644 --- a/packages/@react-spectrum/table/src/Resizer.tsx +++ b/packages/@react-spectrum/table/src/Resizer.tsx @@ -19,11 +19,11 @@ interface ResizerProps { function Resizer(props: ResizerProps, ref: RefObject) { let {column, showResizer} = props; - let {state, columnState} = useTableContext(); + let {state, columnState, isEmpty} = useTableContext(); let stringFormatter = useLocalizedStringFormatter(intlMessages); let {direction} = useLocale(); - let {inputProps, resizerProps} = useTableColumnResize({...props, label: stringFormatter.format('columnResizer')}, state, columnState, ref); + let {inputProps, resizerProps} = useTableColumnResize({...props, label: stringFormatter.format('columnResizer'), isDisabled: isEmpty}, state, columnState, ref); let style = { cursor: undefined, diff --git a/packages/@react-spectrum/table/src/TableView.tsx b/packages/@react-spectrum/table/src/TableView.tsx index aff6f706170..9514e781b4f 100644 --- a/packages/@react-spectrum/table/src/TableView.tsx +++ b/packages/@react-spectrum/table/src/TableView.tsx @@ -81,7 +81,8 @@ interface TableContextValue { state: TableState, layout: TableLayout, columnState: TableColumnResizeState, - headerRowHovered: boolean + headerRowHovered: boolean, + isEmpty: boolean } const TableContext = React.createContext>(null); @@ -305,9 +306,10 @@ function TableView(props: SpectrumTableProps, ref: DOMRef + ) => { - let {state} = useTableContext(); + let {isEmpty} = useTableContext(); let domRef = useFocusableRef(ref); - let {buttonProps} = useButton({...props, elementType: 'div', isDisabled: state.collection.size === 0}, domRef); + let {buttonProps} = useButton({...props, elementType: 'div', isDisabled: isEmpty}, domRef); return (
@@ -541,9 +543,8 @@ function ResizableTableColumnHeader(props) { let ref = useRef(null); let triggerRef = useRef(null); let resizingRef = useRef(null); - let {state, columnState, headerRowHovered} = useTableContext(); + let {state, columnState, headerRowHovered, isEmpty} = useTableContext(); let stringFormatter = useLocalizedStringFormatter(intlMessages); - let isEmpty = state.collection.size === 0; let {pressProps, isPressed} = usePress({isDisabled: isEmpty}); let {columnHeaderProps} = useTableColumnHeader({ node: column, diff --git a/packages/@react-spectrum/table/stories/Table.stories.tsx b/packages/@react-spectrum/table/stories/Table.stories.tsx index 98893456c6a..01bdd1e1047 100644 --- a/packages/@react-spectrum/table/stories/Table.stories.tsx +++ b/packages/@react-spectrum/table/stories/Table.stories.tsx @@ -1700,14 +1700,14 @@ export function TableWithBreadcrumbs() { ); } -function EmptyStateTable() { +export function EmptyStateTable() { let [show, setShow] = useState(false); return ( setShow(show => !show)}>Toggle items - + {column => {column.name} } diff --git a/packages/@react-spectrum/table/test/Table.test.js b/packages/@react-spectrum/table/test/Table.test.js index 39cbe5dc6eb..8c453a4d8e2 100644 --- a/packages/@react-spectrum/table/test/Table.test.js +++ b/packages/@react-spectrum/table/test/Table.test.js @@ -21,13 +21,13 @@ import {Content} from '@react-spectrum/view'; import {CRUDExample} from '../stories/CRUDExample'; import {Dialog, DialogTrigger} from '@react-spectrum/dialog'; import {Divider} from '@react-spectrum/divider'; +import {EmptyStateTable, TableWithBreadcrumbs} from '../stories/Table.stories'; import {getFocusableTreeWalker} from '@react-aria/focus'; import {Heading} from '@react-spectrum/text'; 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 userEvent from '@testing-library/user-event'; @@ -93,6 +93,25 @@ function ExampleSortTable() { ); } +let defaultTable = ( + + + Foo + Bar + + + + Foo 1 + Bar 1 + + + Foo 2 + Bar 2 + + + +); + function pointerEvent(type, opts) { let evt = new Event(type, {bubbles: true, cancelable: true}); Object.assign(evt, { @@ -3682,25 +3701,6 @@ describe('TableView', function () { }); describe('async loading', function () { - let defaultTable = ( - - - Foo - Bar - - - - Foo 1 - Bar 1 - - - Foo 2 - Bar 2 - - - - ); - it('should display a spinner when loading', function () { let tree = render( @@ -3871,75 +3871,6 @@ describe('TableView', function () { // this is a good candidate for storybook interactions test expect(onLoadMoreSpy).toHaveBeenCalledTimes(4); }); - - it('should display an empty state when there are no items', function () { - let tree = render( -

No results

}> - - Foo - Bar - - - {[]} - -
- ); - - let table = tree.getByRole('grid'); - let rows = within(table).getAllByRole('row'); - expect(rows).toHaveLength(2); - expect(rows[1]).toHaveAttribute('aria-rowindex', '2'); - - let cell = within(rows[1]).getByRole('rowheader'); - expect(cell).toHaveAttribute('aria-colspan', '2'); - - let heading = within(cell).getByRole('heading'); - expect(heading).toBeVisible(); - expect(heading).toHaveTextContent('No results'); - - rerender(tree, defaultTable); - act(() => jest.runAllTimers()); - - rows = within(table).getAllByRole('row'); - expect(rows).toHaveLength(3); - expect(heading).not.toBeInTheDocument(); - }); - - it('empty table select all should do nothing', function () { - let onSelectionChange = jest.fn(); - let tree = render( -
-

No results

}> - - Foo - Bar - - - {[]} - -
- -
- ); - - let table = tree.getByRole('grid'); - let selectAll = tree.getByRole('checkbox'); - let rows = within(table).getAllByRole('row'); - expect(rows).toHaveLength(2); - expect(rows[1]).toHaveAttribute('aria-rowindex', '2'); - - userEvent.tab(); - expect(document.activeElement).toBe(table); - userEvent.tab(); - // shift tab with userEvent doesn't bring you to the right place - fireEvent.keyDown(document.activeElement, {key: 'Tab', shift: true}); - fireEvent.keyUp(document.activeElement, {key: 'Tab', shift: true}); - expect(document.activeElement).toBe(selectAll); - - userEvent.type(document.activeElement, '{space}'); - expect(onSelectionChange).toHaveBeenCalledWith('all'); - expect(selectAll).toHaveAttribute('aria-checked', 'true'); - }); }); describe('sorting', function () { @@ -4206,4 +4137,111 @@ describe('TableView', function () { expect(onSortChange).toHaveBeenCalledWith({column: 'bar', direction: 'ascending'}); }); }); + + describe('empty state', function () { + it('should display an empty state when there are no items', function () { + let tree = render( +

No results

}> + + Foo + Bar + + + {[]} + +
+ ); + + let table = tree.getByRole('grid'); + let rows = within(table).getAllByRole('row'); + expect(rows).toHaveLength(2); + expect(rows[1]).toHaveAttribute('aria-rowindex', '2'); + + let cell = within(rows[1]).getByRole('rowheader'); + expect(cell).toHaveAttribute('aria-colspan', '2'); + + let heading = within(cell).getByRole('heading'); + expect(heading).toBeVisible(); + expect(heading).toHaveTextContent('No results'); + + rerender(tree, defaultTable); + act(() => jest.runAllTimers()); + + rows = within(table).getAllByRole('row'); + expect(rows).toHaveLength(3); + expect(heading).not.toBeInTheDocument(); + }); + + it('empty table select all should be disabled', function () { + let onSelectionChange = jest.fn(); + let tree = render( +
+

No results

}> + + Foo + Bar + + + {[]} + +
+ +
+ ); + + let table = tree.getByRole('grid'); + let selectAll = tree.getByRole('checkbox'); + + userEvent.tab(); + expect(document.activeElement).toBe(table); + userEvent.tab(); + expect(document.activeElement).not.toBe(selectAll); + expect(selectAll).toHaveAttribute('disabled'); + }); + + it('should allow the user to tab into the table body', function () { + let tree = render(); + let table = tree.getByRole('grid'); + let toggleButton = tree.getAllByRole('button')[0]; + let link = tree.getByRole('link'); + + userEvent.tab(); + expect(document.activeElement).toBe(toggleButton); + userEvent.tab(); + expect(document.activeElement).toBe(table); + userEvent.tab(); + expect(document.activeElement).toBe(link); + }); + + it('should disable keyboard navigation within the table', function () { + let tree = render(); + let table = tree.getByRole('grid'); + let headers = within(table).getAllByRole('columnheader'); + // Programatically focus the column header since we can't tab to it + act(() => { + headers[2].focus(); + }); + expect(document.activeElement).toBe(headers[2]); + + fireEvent.keyDown(document.activeElement, {key: 'ArrowLeft', code: 37, charCode: 37}); + fireEvent.keyUp(document.activeElement, {key: 'ArrowLeft', code: 37, charCode: 37}); + expect(document.activeElement).toBe(headers[2]); + }); + + it('should disable press interactions with the column headers', function () { + let tree = render(); + let table = tree.getByRole('grid'); + let headers = within(table).getAllByRole('columnheader'); + let toggleButton = tree.getAllByRole('button')[0]; + + userEvent.tab(); + expect(document.activeElement).toBe(toggleButton); + triggerPress(headers[2]); + expect(document.activeElement).toBe(toggleButton); + expect(tree.queryByRole('menuitem')).toBeFalsy(); + fireEvent.mouseEnter(headers[2]); + act(() => {jest.runAllTimers();}); + expect(tree.queryByRole('slider')).toBeFalsy(); + }); + }); }); From 1ddb88498c4957a46ad52eae0c7140a057d939c8 Mon Sep 17 00:00:00 2001 From: Daniel Lu Date: Tue, 23 Aug 2022 15:34:46 -0700 Subject: [PATCH 5/9] adding extra test test is skipped for now due to outstanding bug that is fixed in separate PR --- .../@react-spectrum/table/test/Table.test.js | 57 +++++++++++++++++-- 1 file changed, 52 insertions(+), 5 deletions(-) diff --git a/packages/@react-spectrum/table/test/Table.test.js b/packages/@react-spectrum/table/test/Table.test.js index 8c453a4d8e2..f610be666b8 100644 --- a/packages/@react-spectrum/table/test/Table.test.js +++ b/packages/@react-spectrum/table/test/Table.test.js @@ -4216,16 +4216,18 @@ describe('TableView', function () { it('should disable keyboard navigation within the table', function () { let tree = render(); let table = tree.getByRole('grid'); - let headers = within(table).getAllByRole('columnheader'); + let header = within(table).getAllByRole('columnheader')[2]; + let headerButton = within(header).getByRole('button'); + expect(headerButton).toHaveAttribute('aria-disabled', 'true'); // Programatically focus the column header since we can't tab to it act(() => { - headers[2].focus(); + header.focus(); }); - expect(document.activeElement).toBe(headers[2]); + expect(document.activeElement).toBe(header); fireEvent.keyDown(document.activeElement, {key: 'ArrowLeft', code: 37, charCode: 37}); fireEvent.keyUp(document.activeElement, {key: 'ArrowLeft', code: 37, charCode: 37}); - expect(document.activeElement).toBe(headers[2]); + expect(document.activeElement).toBe(header); }); it('should disable press interactions with the column headers', function () { @@ -4236,12 +4238,57 @@ describe('TableView', function () { userEvent.tab(); expect(document.activeElement).toBe(toggleButton); - triggerPress(headers[2]); + + let columnButton = within(headers[2]).getByRole('button'); + triggerPress(columnButton); expect(document.activeElement).toBe(toggleButton); expect(tree.queryByRole('menuitem')).toBeFalsy(); fireEvent.mouseEnter(headers[2]); act(() => {jest.runAllTimers();}); expect(tree.queryByRole('slider')).toBeFalsy(); }); + + it.skip('should re-enable functionality when the table recieves items', function () { + let tree = render(); + let table = tree.getByRole('grid'); + let headers = within(table).getAllByRole('columnheader'); + let toggleButton = tree.getAllByRole('button')[0]; + let selectAll = tree.getByRole('checkbox'); + + userEvent.tab(); + expect(document.activeElement).toBe(toggleButton); + triggerPress(toggleButton); + act(() => {jest.runAllTimers();}); + + expect(selectAll).not.toHaveAttribute('disabled'); + triggerPress(selectAll); + act(() => {jest.runAllTimers();}); + expect(selectAll.checked).toBeTruthy(); + expect(document.activeElement).toBe(selectAll); + + fireEvent.mouseEnter(headers[2]); + act(() => {jest.runAllTimers();}); + expect(tree.queryAllByRole('slider')).toBeTruthy(); + + let column1Button = within(headers[1]).getByRole('button'); + let column2Button = within(headers[2]).getByRole('button'); + triggerPress(column2Button); + act(() => {jest.runAllTimers();}); + expect(tree.queryAllByRole('menuitem')).toBeTruthy(); + fireEvent.keyDown(document.activeElement, {key: 'Escape'}); + fireEvent.keyUp(document.activeElement, {key: 'Escape'}); + act(() => {jest.runAllTimers();}); + act(() => {jest.runAllTimers();}); + expect(document.activeElement).toBe(column2Button); + fireEvent.keyDown(document.activeElement, {key: 'ArrowLeft', code: 37, charCode: 37}); + fireEvent.keyUp(document.activeElement, {key: 'ArrowLeft', code: 37, charCode: 37}); + expect(document.activeElement).toBe(column1Button); + + triggerPress(toggleButton); + act(() => {jest.runAllTimers();}); + expect(selectAll).toHaveAttribute('disabled'); + triggerPress(headers[2]); + expect(document.activeElement).toBe(toggleButton); + }); }); }); From 1abf609da49d26c99cd5650ce6c16941bb4f00cf Mon Sep 17 00:00:00 2001 From: Daniel Lu Date: Tue, 6 Sep 2022 18:02:59 -0700 Subject: [PATCH 6/9] addressing review comments --- packages/@react-aria/grid/src/useGridCell.ts | 2 -- packages/@react-aria/grid/src/useGridRow.ts | 2 -- packages/@react-aria/table/src/useTable.ts | 2 ++ .../table/src/useTableColumnHeader.ts | 9 +++++++- .../table/src/useTableColumnResize.ts | 6 ++--- .../@react-spectrum/table/src/Resizer.tsx | 4 ++-- .../@react-spectrum/table/src/TableView.tsx | 22 ++++++++++--------- 7 files changed, 26 insertions(+), 21 deletions(-) diff --git a/packages/@react-aria/grid/src/useGridCell.ts b/packages/@react-aria/grid/src/useGridCell.ts index 45bb63051a9..312e1765aeb 100644 --- a/packages/@react-aria/grid/src/useGridCell.ts +++ b/packages/@react-aria/grid/src/useGridCell.ts @@ -94,8 +94,6 @@ export function useGridCell>(props: GridCellProps focus, shouldSelectOnPressUp, onAction: onCellAction ? () => onCellAction(node.key) : onAction, - // TODO: this will affect all grids, maybe make it specific to Table via useTableState? - // something like isInteractionDisabled: state.collection.size === 0 isDisabled: state.collection.size === 0 }); diff --git a/packages/@react-aria/grid/src/useGridRow.ts b/packages/@react-aria/grid/src/useGridRow.ts index 5c0838334df..7ade74fd97f 100644 --- a/packages/@react-aria/grid/src/useGridRow.ts +++ b/packages/@react-aria/grid/src/useGridRow.ts @@ -60,8 +60,6 @@ export function useGridRow, S extends GridState onRowAction(node.key) : onAction, - // TODO: this will affect all grids, maybe make it specific to Table via useTableState? - // something like isInteractionDisabled: state.collection.size === 0 isDisabled: state.collection.size === 0 }); diff --git a/packages/@react-aria/table/src/useTable.ts b/packages/@react-aria/table/src/useTable.ts index e56c09b9d55..267a58669ac 100644 --- a/packages/@react-aria/table/src/useTable.ts +++ b/packages/@react-aria/table/src/useTable.ts @@ -119,6 +119,8 @@ export function useTable(props: AriaTableProps, state: TableState, ref: gridProps: mergeProps( gridProps, descriptionProps, + // If table is empty, make sure the table is tabbable + state.collection.size === 0 && {tabIndex: 0}, { // merge sort description with long press information 'aria-describedby': [descriptionProps['aria-describedby'], gridProps['aria-describedby']].filter(Boolean).join(' ') diff --git a/packages/@react-aria/table/src/useTableColumnHeader.ts b/packages/@react-aria/table/src/useTableColumnHeader.ts index 6b59c04d7e2..efbae0ed932 100644 --- a/packages/@react-aria/table/src/useTableColumnHeader.ts +++ b/packages/@react-aria/table/src/useTableColumnHeader.ts @@ -90,7 +90,14 @@ export function useTableColumnHeader(props: AriaTableColumnHeaderProps, state return { columnHeaderProps: { - ...mergeProps(gridCellProps, pressProps, focusableProps, descriptionProps), + ...mergeProps( + gridCellProps, + pressProps, + focusableProps, + descriptionProps, + // If the table is empty, make all column headers untabbable or programatically focusable + state.collection.size === 0 && {tabIndex: null} + ), role: 'columnheader', id: getColumnHeaderId(state, node.key), 'aria-colspan': node.colspan && node.colspan > 1 ? node.colspan : null, diff --git a/packages/@react-aria/table/src/useTableColumnResize.ts b/packages/@react-aria/table/src/useTableColumnResize.ts index 7054c98a53a..57058f1232f 100644 --- a/packages/@react-aria/table/src/useTableColumnResize.ts +++ b/packages/@react-aria/table/src/useTableColumnResize.ts @@ -30,8 +30,7 @@ export interface TableColumnResizeAria { export interface AriaTableColumnResizeProps { column: GridNode, label: string, - triggerRef: RefObject, - isDisabled?: boolean + triggerRef: RefObject } export function useTableColumnResize(props: AriaTableColumnResizeProps, state: TableState, columnState: TableColumnResizeState, ref: RefObject): TableColumnResizeAria { @@ -160,8 +159,7 @@ export function useTableColumnResize(props: AriaTableColumnResizeProps, st stateRef.current.onColumnResizeEnd(item); state.setKeyboardNavigationDisabled(false); }, - onChange, - disabled: isDisabled + onChange }, ariaProps ) diff --git a/packages/@react-spectrum/table/src/Resizer.tsx b/packages/@react-spectrum/table/src/Resizer.tsx index 910b432bf61..fe7e33ffe53 100644 --- a/packages/@react-spectrum/table/src/Resizer.tsx +++ b/packages/@react-spectrum/table/src/Resizer.tsx @@ -19,11 +19,11 @@ interface ResizerProps { function Resizer(props: ResizerProps, ref: RefObject) { let {column, showResizer} = props; - let {state, columnState, isEmpty} = useTableContext(); + let {state, columnState} = useTableContext(); let stringFormatter = useLocalizedStringFormatter(intlMessages); let {direction} = useLocale(); - let {inputProps, resizerProps} = useTableColumnResize({...props, label: stringFormatter.format('columnResizer'), isDisabled: isEmpty}, state, columnState, ref); + let {inputProps, resizerProps} = useTableColumnResize({...props, label: stringFormatter.format('columnResizer')}, state, columnState, ref); let style = { cursor: undefined, diff --git a/packages/@react-spectrum/table/src/TableView.tsx b/packages/@react-spectrum/table/src/TableView.tsx index 9514e781b4f..631cfba3759 100644 --- a/packages/@react-spectrum/table/src/TableView.tsx +++ b/packages/@react-spectrum/table/src/TableView.tsx @@ -253,6 +253,7 @@ function TableView(props: SpectrumTableProps, ref: DOMRef; } + // TODO: consider this case, what if we have hidden headers and a empty table if (item.props.hideHeader) { return ( @@ -262,7 +263,7 @@ function TableView(props: SpectrumTableProps, ref: DOMRef 0) { return ; } @@ -473,7 +474,8 @@ function TableHeader({children, ...otherProps}) { function TableColumnHeader(props) { let {column} = props; let ref = useRef(null); - let {state} = useTableContext(); + let {state, isEmpty} = useTableContext(); + let {pressProps, isPressed} = usePress({isDisabled: isEmpty}); let {columnHeaderProps} = useTableColumnHeader({ node: column, isVirtualized: true @@ -481,9 +483,9 @@ function TableColumnHeader(props) { let columnProps = column.props as SpectrumColumnProps; - let {hoverProps, isHovered} = useHover(props); + let {hoverProps, isHovered} = useHover({...props, isDisabled: isEmpty}); - const allProps = [columnHeaderProps, hoverProps]; + const allProps = [columnHeaderProps, hoverProps, pressProps]; return ( @@ -495,6 +497,7 @@ function TableColumnHeader(props) { styles, 'spectrum-Table-headCell', { + 'is-active': isPressed, 'is-resizable': columnProps.allowsResizing, 'is-sortable': columnProps.allowsSorting, 'is-sorted-desc': state.sortDescriptor?.column === column.key && state.sortDescriptor?.direction === 'descending', @@ -525,9 +528,8 @@ function TableColumnHeader(props) { } let _TableColumnHeaderButton = (props, ref: FocusableRef) => { - let {isEmpty} = useTableContext(); let domRef = useFocusableRef(ref); - let {buttonProps} = useButton({...props, elementType: 'div', isDisabled: isEmpty}, domRef); + let {buttonProps} = useButton({...props, elementType: 'div'}, domRef); return (
@@ -543,15 +545,15 @@ function ResizableTableColumnHeader(props) { let ref = useRef(null); let triggerRef = useRef(null); let resizingRef = useRef(null); - let {state, columnState, headerRowHovered, isEmpty} = useTableContext(); + let {state, columnState, headerRowHovered} = useTableContext(); let stringFormatter = useLocalizedStringFormatter(intlMessages); - let {pressProps, isPressed} = usePress({isDisabled: isEmpty}); + let {pressProps, isPressed} = usePress({}); let {columnHeaderProps} = useTableColumnHeader({ node: column, isVirtualized: true, hasMenu: true }, state, ref); - let {hoverProps, isHovered} = useHover({...props, isDisabled: isEmpty}); + let {hoverProps, isHovered} = useHover(props); const allProps = [columnHeaderProps, hoverProps, pressProps]; @@ -603,7 +605,7 @@ function ResizableTableColumnHeader(props) { } }, [columnState.currentlyResizingColumn, column.key]); - let showResizer = !isEmpty && (headerRowHovered || columnState.currentlyResizingColumn != null); + let showResizer = headerRowHovered || columnState.currentlyResizingColumn != null; return ( From ad9cc9b55fc30fec190d9952b4e5a7adb6805560 Mon Sep 17 00:00:00 2001 From: Daniel Lu Date: Wed, 7 Sep 2022 12:00:22 -0700 Subject: [PATCH 7/9] fixing resizer bug when going back to empty state the columns in view arent updated from TableColumnHeaders to ResizableTableColumnHeader when going from no items to items in the empty TableView story, so I went back to disabling the resize column button instead --- .../@react-aria/table/src/useTableColumnResize.ts | 6 ++++-- packages/@react-spectrum/table/src/Resizer.tsx | 4 ++-- packages/@react-spectrum/table/src/TableView.tsx | 13 +++++++------ .../@react-spectrum/table/stories/Table.stories.tsx | 4 ++-- 4 files changed, 15 insertions(+), 12 deletions(-) diff --git a/packages/@react-aria/table/src/useTableColumnResize.ts b/packages/@react-aria/table/src/useTableColumnResize.ts index 57058f1232f..7054c98a53a 100644 --- a/packages/@react-aria/table/src/useTableColumnResize.ts +++ b/packages/@react-aria/table/src/useTableColumnResize.ts @@ -30,7 +30,8 @@ export interface TableColumnResizeAria { export interface AriaTableColumnResizeProps { column: GridNode, label: string, - triggerRef: RefObject + triggerRef: RefObject, + isDisabled?: boolean } export function useTableColumnResize(props: AriaTableColumnResizeProps, state: TableState, columnState: TableColumnResizeState, ref: RefObject): TableColumnResizeAria { @@ -159,7 +160,8 @@ export function useTableColumnResize(props: AriaTableColumnResizeProps, st stateRef.current.onColumnResizeEnd(item); state.setKeyboardNavigationDisabled(false); }, - onChange + onChange, + disabled: isDisabled }, ariaProps ) diff --git a/packages/@react-spectrum/table/src/Resizer.tsx b/packages/@react-spectrum/table/src/Resizer.tsx index fe7e33ffe53..910b432bf61 100644 --- a/packages/@react-spectrum/table/src/Resizer.tsx +++ b/packages/@react-spectrum/table/src/Resizer.tsx @@ -19,11 +19,11 @@ interface ResizerProps { function Resizer(props: ResizerProps, ref: RefObject) { let {column, showResizer} = props; - let {state, columnState} = useTableContext(); + let {state, columnState, isEmpty} = useTableContext(); let stringFormatter = useLocalizedStringFormatter(intlMessages); let {direction} = useLocale(); - let {inputProps, resizerProps} = useTableColumnResize({...props, label: stringFormatter.format('columnResizer')}, state, columnState, ref); + let {inputProps, resizerProps} = useTableColumnResize({...props, label: stringFormatter.format('columnResizer'), isDisabled: isEmpty}, state, columnState, ref); let style = { cursor: undefined, diff --git a/packages/@react-spectrum/table/src/TableView.tsx b/packages/@react-spectrum/table/src/TableView.tsx index 631cfba3759..9bf26568f8a 100644 --- a/packages/@react-spectrum/table/src/TableView.tsx +++ b/packages/@react-spectrum/table/src/TableView.tsx @@ -263,7 +263,7 @@ function TableView(props: SpectrumTableProps, ref: DOMRef 0) { + if (item.props.allowsResizing) { return ; } @@ -528,8 +528,9 @@ function TableColumnHeader(props) { } let _TableColumnHeaderButton = (props, ref: FocusableRef) => { + let {isEmpty} = useTableContext(); let domRef = useFocusableRef(ref); - let {buttonProps} = useButton({...props, elementType: 'div'}, domRef); + let {buttonProps} = useButton({...props, elementType: 'div', isDisabled: isEmpty}, domRef); return (
@@ -545,15 +546,15 @@ function ResizableTableColumnHeader(props) { let ref = useRef(null); let triggerRef = useRef(null); let resizingRef = useRef(null); - let {state, columnState, headerRowHovered} = useTableContext(); + let {state, columnState, headerRowHovered, isEmpty} = useTableContext(); let stringFormatter = useLocalizedStringFormatter(intlMessages); - let {pressProps, isPressed} = usePress({}); + let {pressProps, isPressed} = usePress({isDisabled: isEmpty}); let {columnHeaderProps} = useTableColumnHeader({ node: column, isVirtualized: true, hasMenu: true }, state, ref); - let {hoverProps, isHovered} = useHover(props); + let {hoverProps, isHovered} = useHover({...props, isDisabled: isEmpty}); const allProps = [columnHeaderProps, hoverProps, pressProps]; @@ -605,7 +606,7 @@ function ResizableTableColumnHeader(props) { } }, [columnState.currentlyResizingColumn, column.key]); - let showResizer = headerRowHovered || columnState.currentlyResizingColumn != null; + let showResizer = !isEmpty && (headerRowHovered || columnState.currentlyResizingColumn != null); return ( diff --git a/packages/@react-spectrum/table/stories/Table.stories.tsx b/packages/@react-spectrum/table/stories/Table.stories.tsx index 01bdd1e1047..8031ba027aa 100644 --- a/packages/@react-spectrum/table/stories/Table.stories.tsx +++ b/packages/@react-spectrum/table/stories/Table.stories.tsx @@ -1702,11 +1702,11 @@ export function TableWithBreadcrumbs() { export function EmptyStateTable() { let [show, setShow] = useState(false); - + let [sortDescriptor, setSortDescriptor] = useState({}); return ( setShow(show => !show)}>Toggle items - + {column => {column.name} From 3e116d6e79be8483f0d9c2a395246d6dd3b97a50 Mon Sep 17 00:00:00 2001 From: Daniel Lu Date: Wed, 7 Sep 2022 12:16:58 -0700 Subject: [PATCH 8/9] fixing test --- packages/@react-spectrum/table/test/Table.test.js | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/packages/@react-spectrum/table/test/Table.test.js b/packages/@react-spectrum/table/test/Table.test.js index f610be666b8..67ae6a28839 100644 --- a/packages/@react-spectrum/table/test/Table.test.js +++ b/packages/@react-spectrum/table/test/Table.test.js @@ -4217,17 +4217,14 @@ describe('TableView', function () { let tree = render(); let table = tree.getByRole('grid'); let header = within(table).getAllByRole('columnheader')[2]; + expect(header).not.toHaveAttribute('tabindex'); let headerButton = within(header).getByRole('button'); expect(headerButton).toHaveAttribute('aria-disabled', 'true'); - // Programatically focus the column header since we can't tab to it + // Can't progamatically focus the column headers since they have no tab index when table is empty act(() => { header.focus(); }); - expect(document.activeElement).toBe(header); - - fireEvent.keyDown(document.activeElement, {key: 'ArrowLeft', code: 37, charCode: 37}); - fireEvent.keyUp(document.activeElement, {key: 'ArrowLeft', code: 37, charCode: 37}); - expect(document.activeElement).toBe(header); + expect(document.activeElement).toBe(document.body); }); it('should disable press interactions with the column headers', function () { From 7c60e9dc479b27f7a8735eaa7d053dcf620e88c3 Mon Sep 17 00:00:00 2001 From: Daniel Lu Date: Wed, 7 Sep 2022 13:07:18 -0700 Subject: [PATCH 9/9] overriding virtualizer provided tabindex when table is empty virtualizer will provide tabindex=-1 if the focused item is still in view which means a empty table will have tabindex -1 if the column header was focused last. We want the table to be tabbable when empty so we override this --- packages/@react-spectrum/table/src/TableView.tsx | 3 ++- packages/@react-spectrum/table/test/Table.test.js | 3 +++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/@react-spectrum/table/src/TableView.tsx b/packages/@react-spectrum/table/src/TableView.tsx index 9bf26568f8a..1f5614c5c1d 100644 --- a/packages/@react-spectrum/table/src/TableView.tsx +++ b/packages/@react-spectrum/table/src/TableView.tsx @@ -420,7 +420,8 @@ function TableVirtualizer({layout, collection, focusedKey, renderView, renderWra return (