Remove table cell view focusedRecycleCallback (#2257)

# Pull Request

## 🤨 Rationale

Resolves #2202 

## 👩‍💻 Implementation

- Remove `TableCellView` `focusedRecycleCallback()` and `TableRow`
`closeOpenActionMenus()` (both were called on virtualizer updates).
- `KeyboardNavigationManager` focuses the active cell on scroll in
`handleChange()`, which has the same effect.
- Update column HLD template to remove section about focus recycling

## 🧪 Testing

- Manually tested table action menus and anchor + menu button columns
with scrolling, ensured behavior is the same as before
- Tests were already present to ensure that interactive cell content
(e.g. anchor) is blurred on data updates, and that menu buttons in cells
are closed on scroll.
- Added new tests to ensure cell action menus are still closed after a
scroll, and that interactive cell content (e.g. anchor) is blurred on
scroll.

## ✅ Checklist

- [x] I have updated the project documentation to reflect my changes or
determined no changes are needed.

---------

Co-authored-by: Milan Raj <[email protected]>
Co-authored-by: Jesse Attas <[email protected]>

change/@ni-nimble-components-b08c8f1b-bce8-4d5e-b9f0-a3fadb6c92c9.json

@@ -0,0 +1,7 @@
+{
+  "type": "patch",
+  "comment": "Table: remove TableCellView.focusedRecycleCallback() and TableRow.closeOpenActionMenus(). The table now focuses the active cell when the table is scrolled, which has the same effect (closing open menus and blurring active cell content).",
+  "packageName": "@ni/nimble-components",
+  "email": "[email protected]",
+  "dependentChangeType": "patch"
+}

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

@@ -60,10 +60,6 @@ TableColumnAnchorColumnConfig
         return typeof this.cellRecord?.href === 'string';
     }
 
-    public override focusedRecycleCallback(): void {
-        this.anchor?.blur();
-    }
-
     public override get tabbableChildren(): HTMLElement[] {
         if (this.showAnchor) {
             return [this.anchor!];

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

@@ -39,13 +39,6 @@ export abstract class TableCellView<
 
     private delegatedEvents: readonly string[] = [];
 
-    /**
-     * Called if an element inside this cell view has focus, and this row/cell is being recycled.
-     * Expected implementation is to commit changes as needed, and blur the focusable element (or close
-     * the menu/popup/etc).
-     */
-    public focusedRecycleCallback(): void {}
-
     public columnChanged(): void {
         for (const eventName of this.delegatedEvents) {
             this.removeEventListener(eventName, this.delegatedEventHandler);

packages/nimble-components/src/table/components/row/index.ts

@@ -21,7 +21,7 @@ import type {
 } from '../../types';
 import type { TableColumn } from '../../../table-column/base';
 import type { MenuButtonToggleEventDetail } from '../../../menu-button/types';
-import { TableCell, tableCellTag } from '../cell';
+import { tableCellTag } from '../cell';
 import {
     ColumnInternals,
     isColumnInternalsProperty
@@ -208,17 +208,6 @@ export class TableRow<
         );
     }
 
-    public closeOpenActionMenus(): void {
-        if (this.menuOpen) {
-            const cellWithMenuOpen = Array.from(
-                this.cellContainer.children
-            ).find(c => c instanceof TableCell && c.menuOpen) as TableCell;
-            if (cellWithMenuOpen?.actionMenuButton?.open) {
-                cellWithMenuOpen.actionMenuButton.toggleButton!.control.click();
-            }
-        }
-    }
-
     /** @internal */
     public handleChange(source: unknown, args: unknown): void {
         if (

packages/nimble-components/src/table/index.ts

@@ -54,7 +54,7 @@ import { Virtualizer } from './models/virtualizer';
 import { getTanStackSortingFunction } from './models/sort-operations';
 import { TableLayoutManager } from './models/table-layout-manager';
 import { TableUpdateTracker } from './models/table-update-tracker';
-import { TableRow } from './components/row';
+import type { TableRow } from './components/row';
 import type { TableGroupRow } from './components/group-row';
 import { ColumnInternals } from '../table-column/base/models/column-internals';
 import { InteractiveSelectionManager } from './models/interactive-selection-manager';
@@ -64,7 +64,6 @@ import { waitUntilCustomElementsDefinedAsync } from '../utilities/wait-until-cus
 import { ColumnValidator } from '../table-column/base/models/column-validator';
 import { uniquifySlotNameForColumnId } from './models/utilities';
 import { KeyboardNavigationManager } from './models/keyboard-navigation-manager';
-import { TableCellView } from '../table-column/base/cell-view';
 
 declare global {
     interface HTMLElementTagNameMap {
@@ -686,37 +685,6 @@ export class Table<
         return tanStackUpdates;
     }
 
-    /** @internal */
-    public handleFocusedCellRecycling(): void {
-        const hadActiveRowOrCellFocus = this.keyboardNavigationManager.hasActiveRowOrCellFocus;
-
-        let tableFocusedElement = this.shadowRoot!.activeElement;
-        while (
-            tableFocusedElement !== null
-            && !(tableFocusedElement instanceof TableCellView)
-        ) {
-            if (tableFocusedElement.shadowRoot) {
-                tableFocusedElement = tableFocusedElement.shadowRoot.activeElement;
-            } else {
-                break;
-            }
-        }
-        if (tableFocusedElement instanceof TableCellView) {
-            tableFocusedElement.focusedRecycleCallback();
-        }
-        if (this.openActionMenuRecordId !== undefined) {
-            const activeRow = this.rowElements.find(
-                row => row instanceof TableRow
-                    && row.recordId === this.openActionMenuRecordId
-            ) as TableRow | undefined;
-            activeRow?.closeOpenActionMenus();
-        }
-
-        this.keyboardNavigationManager.handleFocusedCellRecycling(
-            hadActiveRowOrCellFocus
-        );
-    }
-
     protected selectionModeChanged(
         _prev: string | undefined,
         _next: string | undefined

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

@@ -185,9 +185,10 @@ implements Subscriber {
         }
 
         if (focusRowAndCell) {
-            // Focusable elements in cells, and action menus, are both blurred on scroll. To maintain our row/cell focus state,
-            // we focus the cell instead here. (We also don't want to refocus the cell content when the focusedRecycleCallback just
-            // blurred it.)
+            // We want open action menus to be closed, and focused interactive cell content blurred, on scroll. We also don't want to
+            // refocus the interactive cell content after the scroll, as the element no longer represents the same table data at that
+            // point. So in both those cases, we focus the cell here. This also lets us maintain what row/ cell the user had focused
+            // previously.
             if (
                 this.focusType === TableFocusType.cellActionMenu
                 || this.focusType === TableFocusType.cellContent
@@ -215,12 +216,6 @@ implements Subscriber {
         }
     }
 
-    public handleFocusedCellRecycling(hadRowOrCellFocus: boolean): void {
-        if (hadRowOrCellFocus && !this.focusWithinTable) {
-            this.focusCurrentRow(false);
-        }
-    }
-
     public onRowFocusIn(event: FocusEvent): void {
         if (this.isCurrentlyFocusingElement) {
             return;

packages/nimble-components/src/table/models/virtualizer.ts

@@ -122,7 +122,6 @@ export class Virtualizer<TData extends TableRecord = TableRecord> {
     }
 
     private handleVirtualizerChange(): void {
-        this.table.handleFocusedCellRecycling();
         const virtualizer = this.virtualizer!;
         this.visibleItems = virtualizer.getVirtualItems();
         this.scrollHeight = virtualizer.getTotalSize();

packages/nimble-components/src/table/tests/table-action-menu.spec.ts

@@ -460,6 +460,34 @@ describe('Table action menu', () => {
         );
     });
 
+    it('when open, closes if the table data is updated', async () => {
+        const slot1 = 'my-action-menu';
+        column1.actionMenuSlot = slot1;
+        const menuItems = createAndSlotMenu(slot1).items;
+        await connect();
+        await waitForUpdatesAsync();
+        pageObject.setRowHoverState(0, true);
+        await pageObject.clickCellActionMenu(0, 0);
+        await toggleListener.promise;
+
+        const closeToggleListener = createEventListener(
+            element,
+            'action-menu-toggle'
+        );
+        const newTableData: SimpleTableRecord[] = [
+            {
+                stringData: 'new string 1',
+                numericData: 8,
+                moreStringData: 'new string 2'
+            }
+        ];
+        await element.setData(newTableData);
+        await closeToggleListener.promise;
+
+        expect(pageObject.getCell(0, 0).menuOpen).toBeFalse();
+        expect(document.activeElement).not.toBe(menuItems[0]!);
+    });
+
     describe('with single row selection', () => {
         beforeEach(async () => {
             const slot = 'my-action-menu';