Add keyboard navigation to table (#2172)

# Pull Request ## 🤨 Rationale Resolves #1137 ## 👩‍💻 Implementation See #2173 for the most up-to-date expected behavior of keyboard navigation that this PR implements. ([Direct link to updated HLD](https://github.com/ni/nimble/blob/96e1725fc7cc8b3fdf5f5197b090e8f95f86cd5f/packages/nimble-components/src/table/specs/table-keyboard-navigation-hld.md)) The `KeyboardNavigationManager` class does the bulk of the work: - Tracks what is currently focused / should be focused (via properties `focusType`, and `row/column/cellContent/headerActionIndex`) - Note: If the user clicks in the table away from the current keyboard focus, the focus state would end up out-of-date. There's code in `handleFocus()` to handle this (see additional comments there) - Listens for table keypresses to handle arrow key navigation, Tab/Shift-Tab, PgUp/PgDn/etc key presses - If the user unfocuses and refocuses the table, re-focuses the appropriate elements in the table (`handleFocus()` again) - Tab/Shift-Tab behavior should match the HLD / expectations for ARIA treegrid. (Basically, Tab/Shift-Tab go through the tabbable elements of the header row / current row, not including cells/column headers, until the end is reached, then the table blurs and elements before/after it will be focused with additional Tab presses.) The overall approach of only setting `tabindex=0` on the single element in the table we want focused is called [roving tabindex](https://www.freecodecamp.org/news/html-roving-tabindex-attribute-explained-with-examples/). To ensure that we control what elements in the table should get focused, this PR updates `tabindex` on many elements in the table: - The table itself is `tabindex=0` to ensure that it can be focused via `Tab` - Elements that are focusable by default such as buttons/anchors, are explicitly set to `tabindex=-1` to start with, until the keyboard navigation code wants to focus them (then they're set to `tabindex=0` just before being focused). ## 🧪 Testing - Manual testing (mostly Chrome, some Firefox). - Earlier builds had some Safari testing, but we should explicitly re-test there before submission. - Added autotests Keyboard navigation can be used on any/all of the [existing table Storybook pages](https://60e89457a987cf003efc0a5b-gjoayfsquj.chromatic.com/?path=/story/components-table--table), once the table is focused. ## ✅ Checklist - [x] I have updated the project documentation to reflect my changes or determined no changes are needed. - Added keyboard nav docs to [table Storybook docs page](https://60e89457a987cf003efc0a5b-gjoayfsquj.chromatic.com/?path=/docs/components-table--docs) (in Accessibility section)
ni · Jun 25, 2024 · 5be33e3 · 5be33e3
1 parent d37ca77
commit 5be33e3
Show file tree

Hide file tree

Showing 35 changed files with 3,641 additions and 97 deletions.
diff --git a/change/@ni-nimble-components-df1cc26c-9976-477a-8fa0-f9b72492eaa0.json b/change/@ni-nimble-components-df1cc26c-9976-477a-8fa0-f9b72492eaa0.json
@@ -0,0 +1,7 @@
+{
+  "type": "minor",
+  "comment": "Add keyboard navigation functionality to the table component",
+  "packageName": "@ni/nimble-components",
+  "email": "[email protected]",
+  "dependentChangeType": "patch"
+}
diff --git a/packages/nimble-components/src/table-column/anchor/cell-view/index.ts b/packages/nimble-components/src/table-column/anchor/cell-view/index.ts
@@ -54,9 +54,22 @@ TableColumnAnchorColumnConfig
         return '';
     }
 
+    /** @internal */
+    @volatile
+    public get showAnchor(): boolean {
+        return typeof this.cellRecord?.href === 'string';
+    }
+
     public override focusedRecycleCallback(): void {
         this.anchor?.blur();
     }
+
+    public override get tabbableChildren(): HTMLElement[] {
+        if (this.showAnchor) {
+            return [this.anchor!];
+        }
+        return [];
+    }
 }
 
 const anchorCellView = TableColumnAnchorCellView.compose({

diff --git a/packages/nimble-components/src/table-column/anchor/cell-view/template.ts b/packages/nimble-components/src/table-column/anchor/cell-view/template.ts
@@ -15,10 +15,12 @@ export const template = html<TableColumnAnchorCellView>`
         }}"
         class="${x => (x.isPlaceholder ? 'placeholder' : '')}"
     >
-        ${when(x => typeof x.cellRecord?.href === 'string', html<TableColumnAnchorCellView>`
+        ${when(x => x.showAnchor, html<TableColumnAnchorCellView>`
             <${anchorTag}
                 ${ref('anchor')}
                 ${overflow('hasOverflow')}
+                ${'' /* tabindex managed dynamically by KeyboardNavigationManager */}
+                tabindex="-1"
                 href="${x => x.cellRecord?.href}"
                 hreflang="${x => x.columnConfig?.hreflang}"
                 ping="${x => x.columnConfig?.ping}"
@@ -33,7 +35,7 @@ export const template = html<TableColumnAnchorCellView>`
             >
                 ${x => x.text}
             </${anchorTag}>`)}
-        ${when(x => typeof x.cellRecord?.href !== 'string', html<TableColumnAnchorCellView>`
+        ${when(x => !x.showAnchor, html<TableColumnAnchorCellView>`
             <span
                 ${overflow('hasOverflow')}
                 title=${x => (x.hasOverflow ? x.text : null)}

diff --git a/packages/nimble-components/src/table-column/anchor/tests/table-column-anchor.spec.ts b/packages/nimble-components/src/table-column/anchor/tests/table-column-anchor.spec.ts
@@ -1,5 +1,6 @@
 import { html, ref } from '@microsoft/fast-element';
 import { parameterizeSpec } from '@ni/jasmine-parameterized';
+import { keyArrowDown, keyEscape, keyTab } from '@microsoft/fast-web-utilities';
 import { tableTag, type Table } from '../../../table';
 import { TableColumnAnchor, tableColumnAnchorTag } from '..';
 import { waitForUpdatesAsync } from '../../../testing/async-helpers';
@@ -8,6 +9,7 @@ import { TableColumnSortDirection, TableRecord } from '../../../table/types';
 import { TablePageObject } from '../../../table/testing/table.pageobject';
 import { wackyStrings } from '../../../utilities/tests/wacky-strings';
 import type { Anchor } from '../../../anchor';
+import { sendKeyDownEvent } from '../../../utilities/tests/component';
 
 interface SimpleTableRecord extends TableRecord {
     label?: string | null;
@@ -164,6 +166,16 @@ describe('TableColumnAnchor', () => {
             expect(pageObject.getCellTitle(0, 0)).toBe('');
         });
 
+        it('cell view tabbableChildren is an empty array', async () => {
+            const cellContents = 'value';
+            await table.setData([{ label: cellContents }]);
+            await connect();
+            await waitForUpdatesAsync();
+
+            const cellView = pageObject.getRenderedCellView(0, 0);
+            expect(cellView.tabbableChildren).toEqual([]);
+        });
+
         describe('various string values render as expected', () => {
             parameterizeSpec(wackyStrings, (spec, name) => {
                 spec(`data "${name}" renders correctly`, async () => {
@@ -247,6 +259,16 @@ describe('TableColumnAnchor', () => {
             ).toBeFalse();
         });
 
+        it('cell view tabbableChildren returns the anchor', async () => {
+            await table.setData([{ link: 'foo' }]);
+            await connect();
+            await waitForUpdatesAsync();
+
+            const cellView = pageObject.getRenderedCellView(0, 0);
+            const anchor = pageObject.getRenderedCellAnchor(0, 0);
+            expect(cellView.tabbableChildren).toEqual([anchor]);
+        });
+
         const linkOptionData = [
             { name: 'hreflang', accessor: (x: Anchor) => x.hreflang },
             { name: 'ping', accessor: (x: Anchor) => x.ping },
@@ -616,5 +638,61 @@ describe('TableColumnAnchor', () => {
                 placeholder
             );
         });
+
+        it('for cells with placeholder, cellView tabbableChildren is an empty array', async () => {
+            await initializeColumnAndTable([{}], 'placeholder');
+
+            const cellView = pageObject.getRenderedCellView(0, 0);
+            expect(cellView.tabbableChildren).toEqual([]);
+        });
+    });
+
+    describe('keyboard navigation', () => {
+        beforeEach(async () => {
+            const tableData = [
+                {
+                    id: '1',
+                    label: 'Link 1a',
+                    link: 'http://www.ni.com/a1'
+                }
+            ];
+            await table.setData(tableData);
+            column.groupIndex = null;
+            await connect();
+            await waitForUpdatesAsync();
+            table.focus();
+            await waitForUpdatesAsync();
+        });
+
+        afterEach(async () => {
+            await disconnect();
+        });
+
+        function isAnchorFocused(anchor: Anchor): boolean {
+            return anchor.shadowRoot?.activeElement !== null;
+        }
+
+        describe('with cell[0, 0] focused,', () => {
+            beforeEach(async () => {
+                await sendKeyDownEvent(table, keyArrowDown);
+            });
+
+            it('anchors in cells are reachable via Tab', async () => {
+                await sendKeyDownEvent(table, keyTab);
+
+                expect(
+                    isAnchorFocused(pageObject.getRenderedCellAnchor(0, 0))
+                ).toBe(true);
+            });
+
+            it('when an anchor is focused, pressing Esc will blur the anchor', async () => {
+                await sendKeyDownEvent(table, keyTab);
+                await sendKeyDownEvent(table, keyEscape);
+
+                expect(
+                    isAnchorFocused(pageObject.getRenderedCellAnchor(0, 0))
+                ).toBe(false);
+            });
+        });
     });
 });
diff --git a/packages/nimble-components/src/table-column/base/cell-view/index.ts b/packages/nimble-components/src/table-column/base/cell-view/index.ts
@@ -29,6 +29,14 @@ export abstract class TableCellView<
     @observable
     public recordId?: string;
 
+    /**
+     * Gets the child elements in this cell view that should be able to be reached via Tab/ Shift-Tab,
+     * if any.
+     */
+    public get tabbableChildren(): HTMLElement[] {
+        return [];
+    }
+
     private delegatedEvents: readonly string[] = [];
 
     /**

diff --git a/packages/nimble-components/src/table/components/cell/index.ts b/packages/nimble-components/src/table/components/cell/index.ts
@@ -9,6 +9,7 @@ import type {
 } from '../../../table-column/base/types';
 import { styles } from './styles';
 import { template } from './template';
+import type { TableCellView } from '../../../table-column/base/cell-view';
 
 declare global {
     interface HTMLElementTagNameMap {
@@ -47,11 +48,21 @@ export class TableCell<
     @observable
     public cellViewTemplate?: ViewTemplate<TableCell>;
 
+    /** @internal */
+    @observable
+    public cellViewContainer!: HTMLElement;
+
     @observable
     public nestingLevel = 0;
 
     public readonly actionMenuButton?: MenuButton;
 
+    /** @internal */
+    public get cellView(): TableCellView<TCellRecord> {
+        return this.cellViewContainer
+            .firstElementChild as TableCellView<TCellRecord>;
+    }
+
     public onActionMenuBeforeToggle(
         event: CustomEvent<MenuButtonToggleEventDetail>
     ): void {
@@ -64,6 +75,22 @@ export class TableCell<
         this.menuOpen = event.detail.newState;
         this.$emit('cell-action-menu-toggle', event.detail);
     }
+
+    public onActionMenuBlur(): void {
+        this.$emit('cell-action-menu-blur', this);
+    }
+
+    public onCellViewFocusIn(): void {
+        this.$emit('cell-view-focus-in', this);
+    }
+
+    public onCellFocusIn(): void {
+        this.$emit('cell-focus-in', this);
+    }
+
+    public onCellBlur(): void {
+        this.$emit('cell-blur', this);
+    }
 }
 
 const nimbleTableCell = TableCell.compose({

diff --git a/packages/nimble-components/src/table/components/cell/styles.ts b/packages/nimble-components/src/table/components/cell/styles.ts
@@ -1,10 +1,13 @@
 import { css } from '@microsoft/fast-element';
 import { display } from '../../../utilities/style/display';
 import {
+    borderHoverColor,
+    borderWidth,
     controlHeight,
     controlSlimHeight,
     mediumPadding
 } from '../../../theme-provider/design-tokens';
+import { focusVisible } from '../../../utilities/style/focus';
 
 export const styles = css`
     ${display('flex')}
@@ -22,6 +25,15 @@ export const styles = css`
         --ni-private-table-cell-action-menu-display: block;
     }
 
+    :host(${focusVisible}) {
+        outline: calc(2 * ${borderWidth}) solid ${borderHoverColor};
+        outline-offset: -2px;
+    }
+
+    .cell-view-container {
+        display: contents;
+    }
+
     .cell-view {
         overflow: hidden;
     }
@@ -34,4 +46,11 @@ export const styles = css`
         height: ${controlSlimHeight};
         align-self: center;
     }
+
+    ${
+        /* This CSS class is applied dynamically by KeyboardNavigationManager */ ''
+    }
+    .action-menu.cell-action-menu-focused {
+        display: block;
+    }
 `;
diff --git a/packages/nimble-components/src/table/components/cell/template.ts b/packages/nimble-components/src/table/components/cell/template.ts
@@ -10,15 +10,23 @@ import { tableCellActionMenuLabel } from '../../../label-provider/table/label-to
 
 // prettier-ignore
 export const template = html<TableCell>`
-    <template role="cell" style="--ni-private-table-cell-nesting-level: ${x => x.nestingLevel}">
-        ${x => x.cellViewTemplate}
+    <template role="cell" style="--ni-private-table-cell-nesting-level: ${x => x.nestingLevel}"
+        @focusin="${x => x.onCellFocusIn()}"
+        @blur="${x => x.onCellBlur()}"
+    >
+        <div ${ref('cellViewContainer')} class="cell-view-container" @focusin="${x => x.onCellViewFocusIn()}">
+            ${x => x.cellViewTemplate}
+        </div>
         ${when(x => x.hasActionMenu, html<TableCell>`
             <${menuButtonTag} ${ref('actionMenuButton')}
                 content-hidden
                 appearance="${ButtonAppearance.ghost}"
+                ${'' /* tabindex managed dynamically by KeyboardNavigationManager */}
+                tabindex="-1"
                 @beforetoggle="${(x, c) => x.onActionMenuBeforeToggle(c.event as CustomEvent<MenuButtonToggleEventDetail>)}"
                 @toggle="${(x, c) => x.onActionMenuToggle(c.event as CustomEvent<MenuButtonToggleEventDetail>)}"
                 @click="${(_, c) => c.event.stopPropagation()}"
+                @blur="${x => x.onActionMenuBlur()}"
                 class="action-menu"
                 title="${x => (x.actionMenuLabel ? x.actionMenuLabel : tableCellActionMenuLabel.getValueFor(x))}"
             >

diff --git a/packages/nimble-components/src/table/components/cell/tests/table-cell.pageobject.ts b/packages/nimble-components/src/table/components/cell/tests/table-cell.pageobject.ts
@@ -10,7 +10,7 @@ export class TableCellPageObject<T extends TableCellRecord = TableCellRecord> {
     public constructor(private readonly tableCellElement: TableCell<T>) {}
 
     public getRenderedCellView(): TableCellView {
-        const cellView = this.tableCellElement.shadowRoot!.firstElementChild;
+        const cellView = this.tableCellElement.cellViewContainer.firstElementChild;
         if (!(cellView instanceof TableCellView)) {
             throw new Error(
                 'Cell view not found in cell - ensure cellViewTag is set for column'

diff --git a/packages/nimble-components/src/table/components/cell/tests/table-cell.spec.ts b/packages/nimble-components/src/table/components/cell/tests/table-cell.spec.ts
@@ -10,6 +10,7 @@ import type { TableCellRecord } from '../../../../table-column/base/types';
 import { TableCellPageObject } from './table-cell.pageobject';
 import { TableCellView } from '../../../../table-column/base/cell-view';
 import { createCellViewTemplate } from '../../../../table-column/base/cell-view/template';
+import { createEventListener } from '../../../../utilities/tests/component';
 
 interface SimpleTableCellRecord extends TableCellRecord {
     stringData: string;
@@ -79,4 +80,67 @@ describe('TableCell', () => {
         const renderedContent = pageObject.getRenderedCellContent();
         expect(renderedContent).toBe('bar');
     });
+
+    it('fires cell-view-focus-in/cell-focus-in/cell-blur events when cell contents are focused/blurred', async () => {
+        await connect();
+        await waitForUpdatesAsync();
+
+        const cellViewFocusInListener = createEventListener(
+            element,
+            'cell-view-focus-in'
+        );
+        const cellFocusInListener = createEventListener(
+            element,
+            'cell-focus-in'
+        );
+        const cellBlurListener = createEventListener(element, 'cell-blur');
+        const renderedCellView = pageObject.getRenderedCellView();
+        const span = renderedCellView.shadowRoot
+            ?.firstElementChild as HTMLSpanElement;
+        span.tabIndex = 0;
+        span.focus();
+        await cellViewFocusInListener.promise;
+        await cellFocusInListener.promise;
+
+        expect(cellViewFocusInListener.spy).toHaveBeenCalledOnceWith(
+            jasmine.objectContaining({ detail: element })
+        );
+        expect(cellFocusInListener.spy).toHaveBeenCalledOnceWith(
+            jasmine.objectContaining({ detail: element })
+        );
+        expect(cellBlurListener.spy).not.toHaveBeenCalled();
+
+        span.blur();
+        await cellBlurListener.promise;
+
+        expect(cellBlurListener.spy).toHaveBeenCalledOnceWith(
+            jasmine.objectContaining({ detail: element })
+        );
+    });
+
+    it('fires the cell-view-focus-in event when cell contents are focused, after the cell is already focused', async () => {
+        await connect();
+        await waitForUpdatesAsync();
+
+        const cellViewFocusInListener = createEventListener(
+            element,
+            'cell-view-focus-in'
+        );
+        const renderedCellView = pageObject.getRenderedCellView();
+        element.tabIndex = 0;
+        element.focus();
+        await waitForUpdatesAsync();
+
+        expect(cellViewFocusInListener.spy).not.toHaveBeenCalled();
+
+        const span = renderedCellView.shadowRoot
+            ?.firstElementChild as HTMLSpanElement;
+        span.tabIndex = 0;
+        span.focus();
+        await cellViewFocusInListener.promise;
+
+        expect(cellViewFocusInListener.spy).toHaveBeenCalledOnceWith(
+            jasmine.objectContaining({ detail: element })
+        );
+    });
 });