Support dynamic tabbableChildren for TableCellView

change/@ni-nimble-components-8499a62b-64ea-41e0-877c-6bacca7ae0c8.json

@@ -0,0 +1,7 @@
+{
+  "type": "patch",
+  "comment": "Update TableCellView tabbableChildren to be an observable property",
+  "packageName": "@ni/nimble-components",
+  "email": "[email protected]",
+  "dependentChangeType": "patch"
+}

packages/nimble-components/src/table-column/anchor/cell-view/index.ts

@@ -31,6 +31,7 @@ TableColumnAnchorColumnConfig
     public isPlaceholder = false;
 
     /** @internal */
+    @observable
     public anchor?: Anchor;
 
     @volatile
@@ -60,11 +61,8 @@ TableColumnAnchorColumnConfig
         return typeof this.cellRecord?.href === 'string';
     }
 
-    public override get tabbableChildren(): HTMLElement[] {
-        if (this.showAnchor) {
-            return [this.anchor!];
-        }
-        return [];
+    private anchorChanged(): void {
+        this.tabbableChildren = this.anchor ? [this.anchor] : [];
     }
 }
 

packages/nimble-components/src/table-column/anchor/cell-view/template.ts

@@ -1,8 +1,9 @@
 /* eslint-disable @typescript-eslint/indent */
-import { html, ref, when } from '@microsoft/fast-element';
+import { html, when } from '@microsoft/fast-element';
 import type { TableColumnAnchorCellView } from '.';
 import { anchorTag } from '../../../anchor';
 import { overflow } from '../../../utilities/directive/overflow';
+import { dynamicRef } from '../../../utilities/directive/dynamic-ref';
 
 // prettier-ignore
 export const template = html<TableColumnAnchorCellView>`
@@ -17,7 +18,7 @@ export const template = html<TableColumnAnchorCellView>`
     >
         ${when(x => x.showAnchor, html<TableColumnAnchorCellView>`
             <${anchorTag}
-                ${ref('anchor')}
+                ${dynamicRef('anchor')}
                 ${overflow('hasOverflow')}
                 ${'' /* tabindex managed dynamically by KeyboardNavigationManager */}
                 tabindex="-1"

packages/nimble-components/src/table-column/base/cell-view/index.ts

@@ -30,12 +30,11 @@ export abstract class TableCellView<
     public recordId?: string;
 
     /**
-     * Gets the child elements in this cell view that should be able to be reached via Tab/ Shift-Tab,
+     * 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 [];
-    }
+    @observable
+    public tabbableChildren: Element[] = [];
 
     private delegatedEvents: readonly string[] = [];
 
@@ -72,4 +71,8 @@ export abstract class TableCellView<
     }
 
     private delegatedEventHandler: (event: Event) => void = () => {};
+
+    private tabbableChildrenChanged(): void {
+        this.$emit('cell-tabbable-children-change', this);
+    }
 }

packages/nimble-components/src/table-column/menu-button/cell-view/index.ts

@@ -26,6 +26,7 @@ TableColumnMenuButtonCellRecord,
 TableColumnMenuButtonColumnConfig
 > {
     /** @internal */
+    @observable
     public menuButton?: MenuButton;
 
     /** @internal */
@@ -41,13 +42,6 @@ TableColumnMenuButtonColumnConfig
         return !!this.cellRecord?.value;
     }
 
-    public override get tabbableChildren(): HTMLElement[] {
-        if (this.showMenuButton) {
-            return [this.menuButton!];
-        }
-        return [];
-    }
-
     /** @internal */
     public onMenuButtonBeforeToggle(
         event: CustomEvent<MenuButtonToggleEventDetail>
@@ -82,6 +76,10 @@ TableColumnMenuButtonColumnConfig
         // from affecting row selection.
         e.stopPropagation();
     }
+
+    private menuButtonChanged(): void {
+        this.tabbableChildren = this.menuButton ? [this.menuButton] : [];
+    }
 }
 
 const menuButtonCellView = TableColumnMenuButtonCellView.compose({

packages/nimble-components/src/table-column/menu-button/cell-view/templates.ts

@@ -7,12 +7,13 @@ import {
 } from '../../../menu-button/types';
 import { iconArrowExpanderDownTag } from '../../../icons/arrow-expander-down';
 import { cellViewMenuSlotName } from '../types';
+import { dynamicRef } from '../../../utilities/directive/dynamic-ref';
 
 // prettier-ignore
 export const template = html<TableColumnMenuButtonCellView>`
     ${when(x => x.showMenuButton, html<TableColumnMenuButtonCellView>`
         <${menuButtonTag}
-            ${ref('menuButton')}
+            ${dynamicRef('menuButton')}
             appearance="${ButtonAppearance.ghost}"
             @beforetoggle="${(x, c) => x.onMenuButtonBeforeToggle(c.event as CustomEvent<MenuButtonToggleEventDetail>)}"
             @mouseover="${x => x.onMenuButtonMouseOver()}"

packages/nimble-components/src/table/models/keyboard-navigation-manager.ts

@@ -40,6 +40,12 @@ interface TableFocusState {
     cellContentIndex?: number;
 }
 
+interface TableCellContentFocusState {
+    index: number;
+    columnId?: string;
+    recordId?: string;
+}
+
 /**
  * Manages the keyboard navigation and focus within the table.
  * @internal
@@ -49,13 +55,16 @@ implements Subscriber {
     private focusType: TableFocusType = TableFocusType.none;
     private headerActionIndex = -1;
     private rowIndex = -1;
-    private cellContentIndex = -1;
     private columnIndex = -1;
     private focusWithinTable = false;
     private isCurrentlyFocusingElement = false;
     private readonly tableNotifier: Notifier;
     private readonly virtualizerNotifier: Notifier;
     private visibleRowNotifiers: Notifier[] = [];
+    private readonly cellContentState: TableCellContentFocusState = {
+        index: -1
+    };
+
     private get inNavigationMode(): boolean {
         return (
             this.focusType !== TableFocusType.cellActionMenu
@@ -117,6 +126,10 @@ implements Subscriber {
             'cell-blur',
             this.onCellBlur as EventListener
         );
+        this.table.viewport.addEventListener(
+            'cell-tabbable-children-change',
+            this.onCellTabbableChildrenChange as EventListener
+        );
     }
 
     public disconnect(): void {
@@ -157,6 +170,10 @@ implements Subscriber {
             'cell-blur',
             this.onCellBlur as EventListener
         );
+        this.table.viewport.removeEventListener(
+            'cell-tabbable-children-change',
+            this.onCellTabbableChildrenChange as EventListener
+        );
     }
 
     public handleChange(source: unknown, args: unknown): void {
@@ -323,6 +340,28 @@ implements Subscriber {
         this.setElementFocusable(cell, false);
     };
 
+    private readonly onCellTabbableChildrenChange = (
+        event: CustomEvent<TableCellView>
+    ): void => {
+        event.stopPropagation();
+        const cellView = event.detail;
+        if (
+            this.focusType === TableFocusType.cellContent
+            && cellView.recordId === this.cellContentState.recordId
+            && cellView?.column?.columnId === this.cellContentState.columnId
+        ) {
+            if (
+                this.cellContentState.index >= cellView.tabbableChildren.length
+            ) {
+                this.setCellFocusState(
+                    this.columnIndex,
+                    this.rowIndex,
+                    this.focusWithinTable
+                );
+            }
+        }
+    };
+
     private readonly onCaptureKeyDown = (event: KeyboardEvent): void => {
         let handled = false;
         if (event.key === keyTab) {
@@ -622,7 +661,14 @@ implements Subscriber {
             this.rowIndex = nextFocusState.rowIndex ?? this.rowIndex;
             this.columnIndex = nextFocusState.columnIndex ?? this.columnIndex;
             this.headerActionIndex = nextFocusState.headerActionIndex ?? this.headerActionIndex;
-            this.cellContentIndex = nextFocusState.cellContentIndex ?? this.cellContentIndex;
+            if (nextFocusState.cellContentIndex !== undefined) {
+                this.cellContentState.index = nextFocusState.cellContentIndex;
+                const row = this.getCurrentRow() as TableRow;
+                const elements = row?.getFocusableElements();
+                this.cellContentState.columnId = elements?.cells[this.columnIndex]?.cell.columnId;
+                this.cellContentState.recordId = row?.recordId;
+            }
+
             if (this.hasRowOrCellFocusType()) {
                 this.focusCurrentRow(false);
             } else {
@@ -666,7 +712,7 @@ implements Subscriber {
                 if (
                     this.focusType === TableFocusType.cellContent
                     && this.columnIndex === cellIndex
-                    && this.cellContentIndex === i
+                    && this.cellContentState.index === i
                 ) {
                     startIndex = focusStates.length - 1;
                 }
@@ -853,6 +899,7 @@ implements Subscriber {
                         );
                         if (contentIndex > -1) {
                             this.setCellContentFocusState(
+                                cell,
                                 contentIndex,
                                 row.resolvedRowIndex!,
                                 columnIndex,
@@ -1009,8 +1056,10 @@ implements Subscriber {
                 break;
             }
             case TableFocusType.cellContent: {
-                focusableElement = rowElements.cells[this.columnIndex]?.cell.cellView
-                    .tabbableChildren[this.cellContentIndex];
+                focusableElement = rowElements.cells[this.columnIndex]?.cell
+                    .cellView.tabbableChildren[
+                        this.cellContentState.index
+                    ] as HTMLElement;
                 break;
             }
             default:
@@ -1273,22 +1322,21 @@ implements Subscriber {
         }
         const newColumnIndex = columnIndex ?? this.columnIndex;
         const newRowIndex = rowIndex ?? this.rowIndex;
-
-        if (
-            newColumnIndex >= 0
-            && newColumnIndex < rowElements.cells.length
-            && cellContentIndex >= 0
-            && cellContentIndex
-                < rowElements.cells[newColumnIndex]!.cell.cellView
-                    .tabbableChildren.length
-        ) {
-            this.setCellContentFocusState(
-                cellContentIndex,
-                newRowIndex,
-                newColumnIndex,
-                true
-            );
-            return true;
+        if (newColumnIndex >= 0 && newColumnIndex < rowElements.cells.length) {
+            const cell = rowElements.cells[newColumnIndex]!.cell;
+            if (
+                cellContentIndex >= 0
+                && cellContentIndex < cell.cellView.tabbableChildren.length
+            ) {
+                this.setCellContentFocusState(
+                    cell,
+                    cellContentIndex,
+                    newRowIndex,
+                    newColumnIndex,
+                    true
+                );
+                return true;
+            }
         }
 
         return false;
@@ -1324,13 +1372,16 @@ implements Subscriber {
     }
 
     private setCellContentFocusState(
+        cell: TableCell,
         cellContentIndex: number,
         rowIndex: number,
         columnIndex: number,
         focusElement: boolean
     ): void {
+        this.cellContentState.recordId = cell.recordId;
+        this.cellContentState.columnId = cell.columnId;
         this.focusType = TableFocusType.cellContent;
-        this.cellContentIndex = cellContentIndex;
+        this.cellContentState.index = cellContentIndex;
         this.setRowCellFocusState(columnIndex, rowIndex, focusElement);
     }
 

packages/nimble-components/src/table/tests/table-keyboard-navigation.spec.ts

@@ -1,5 +1,5 @@
 /* eslint-disable no-await-in-loop */
-import { customElement, html, observable, ref } from '@microsoft/fast-element';
+import { customElement, html, observable, when } from '@microsoft/fast-element';
 import { FoundationElement } from '@microsoft/fast-foundation';
 import {
     keyArrowDown,
@@ -49,6 +49,7 @@ import type { ColumnInternalsOptions } from '../../table-column/base/models/colu
 import { ColumnValidator } from '../../table-column/base/models/column-validator';
 import { mixinSortableColumnAPI } from '../../table-column/mixins/sortable-column';
 import { MenuButtonPageObject } from '../../menu-button/testing/menu-button.pageobject';
+import { dynamicRef } from '../../utilities/directive/dynamic-ref';
 
 interface SimpleTableRecord extends TableRecord {
     id: string;
@@ -1235,15 +1236,18 @@ describe('Table keyboard navigation', () => {
         // prettier-ignore
         @customElement({
             name: interactiveCellViewName,
-            template: html<TestInteractiveCellView>`<span tabindex="-1" ${ref('spanElement')}>Test</span>`
+            template: html<TestInteractiveCellView>`${when(x => x.isTabbable, html<TestInteractiveCellView>`<span tabindex="-1" ${dynamicRef('spanElement')}>Test</span>`)}`
         })
         // eslint-disable-next-line @typescript-eslint/no-unused-vars
         class TestInteractiveCellView extends TableCellView {
             @observable
-            public spanElement!: HTMLSpanElement;
+            public isTabbable = true;
 
-            public override get tabbableChildren(): HTMLElement[] {
-                return [this.spanElement];
+            @observable
+            public spanElement?: HTMLSpanElement;
+
+            private spanElementChanged(): void {
+                this.tabbableChildren = this.spanElement ? [this.spanElement] : [];
             }
         }
         // prettier-ignore
@@ -1283,7 +1287,7 @@ describe('Table keyboard navigation', () => {
                     rowIndex,
                     columnIndex
                 ) as TestInteractiveCellView
-            ).spanElement;
+            ).spanElement!;
         }
 
         beforeEach(async () => {
@@ -1422,6 +1426,28 @@ describe('Table keyboard navigation', () => {
                     expect(blurSpy).toHaveBeenCalledTimes(1);
                     expect(currentFocusedElement()).not.toBe(cellContent);
                 });
+
+                it('and then the cell updates to no longer have tabbableChildren, the cell is focused instead', async () => {
+                    const cellView = pageObject.getRenderedCellView(
+                        0,
+                        1
+                    ) as TestInteractiveCellView;
+                    cellView.isTabbable = false;
+                    await waitForUpdatesAsync();
+
+                    // Note: At this point, the focused element in the cell has been already removed from the DOM.
+                    // KeyboardNavigationManager will only set focus to new elements when the table is already focused (so we don't
+                    // steal focus from elsewhere on the page if the table isn't being interacted with).
+                    // In Chrome, the table loses focus entirely, but not in Firefox/WebKit.
+                    if (element.shadowRoot!.activeElement === null) {
+                        element.focus(); // Refocus table if it's been lost
+                        await waitForUpdatesAsync();
+                    }
+
+                    expect(currentFocusedElement()).toBe(
+                        pageObject.getCell(0, 1)
+                    );
+                });
             });
         });
     });