Table: Move row height calculation from virtualizer, monitor for toke…

…n changes (#2388)

# Pull Request

## 🤨 Rationale

#2212 

## 👩‍💻 Implementation

- Moved row height calculation from virtualizer to table class (new
`internal` property there for row height)
- Added subscribers to listen for changes to the `controlHeight` and
`borderWidth` design tokens, which are used for row height calculation

## 🧪 Testing

- No table visual diffs with this change
- Added autotests to ensure that rowHeight/pageSize are recomputed when
the `controlHeight` and `borderWidth` tokens change
- Manual testing of PageUp/PageDown/Home/End key behavior after tokens
change

## ✅ Checklist

<!--- Review the list and put an x in the boxes that apply or ~~strike
through~~ around items that don't (along with an explanation). -->

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

change/@ni-nimble-components-49250ef8-26bc-404b-bba7-342b1f3b6a8e.json

@@ -0,0 +1,7 @@
+{
+  "type": "patch",
+  "comment": "Table: move row height computation out of virtualizer; row height recomputed if dependent tokens change.",
+  "packageName": "@ni/nimble-components",
+  "email": "[email protected]",
+  "dependentChangeType": "patch"
+}

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

@@ -9,6 +9,7 @@ import {
 import {
     Checkbox,
     DesignSystem,
+    DesignTokenSubscriber,
     FoundationElement
 } from '@microsoft/fast-foundation';
 import {
@@ -65,6 +66,7 @@ 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 { borderWidth, controlHeight } from '../theme-provider/design-tokens';
 
 declare global {
     interface HTMLElementTagNameMap {
@@ -251,6 +253,13 @@ export class Table<
     @observable
     public windowShiftKeyDown = false;
 
+    /**
+     * @internal
+     */
+    public get rowHeight(): number {
+        return this._rowHeight;
+    }
+
     private readonly table: TanStackTable<TableNode<TData>>;
     private options: TanStackTableOptionsResolved<TableNode<TData>>;
     private readonly tableValidator = new TableValidator<TData>();
@@ -260,6 +269,7 @@ export class Table<
     private readonly expansionManager: ExpansionManager<TData>;
     private columnNotifiers: Notifier[] = [];
     private readonly layoutManagerNotifier: Notifier;
+    private _rowHeight = 0;
     private isInitialized = false;
     // Programmatically updating the selection state of a checkbox fires the 'change' event.
     // Therefore, selection change events that occur due to programmatically updating
@@ -273,6 +283,22 @@ export class Table<
     { recordId: string, uniqueSlot: string }
     > = new Map();
 
+    private readonly borderWidthSubscriber: DesignTokenSubscriber<
+        typeof borderWidth
+    > = {
+            handleChange: () => {
+                this.updateRowHeight();
+            }
+        };
+
+    private readonly controlHeightSubscriber: DesignTokenSubscriber<
+        typeof controlHeight
+    > = {
+            handleChange: () => {
+                this.updateRowHeight();
+            }
+        };
+
     private actionMenuSlots: string[] = [];
 
     public constructor() {
@@ -362,6 +388,7 @@ export class Table<
     public override connectedCallback(): void {
         super.connectedCallback();
         this.initialize();
+        this.updateRowHeight();
         this.virtualizer.connect();
         this.viewport.addEventListener('scroll', this.onViewPortScroll, {
             passive: true
@@ -370,6 +397,8 @@ export class Table<
         window.addEventListener('keydown', this.onKeyDown);
         window.addEventListener('keyup', this.onKeyUp);
         window.addEventListener('blur', this.onBlur);
+        borderWidth.subscribe(this.borderWidthSubscriber, this);
+        controlHeight.subscribe(this.controlHeightSubscriber, this);
     }
 
     public override disconnectedCallback(): void {
@@ -380,6 +409,8 @@ export class Table<
         window.removeEventListener('keydown', this.onKeyDown);
         window.removeEventListener('keyup', this.onKeyUp);
         window.removeEventListener('blur', this.onBlur);
+        borderWidth.unsubscribe(this.borderWidthSubscriber);
+        controlHeight.unsubscribe(this.controlHeightSubscriber);
     }
 
     public checkValidity(): boolean {
@@ -1332,6 +1363,12 @@ export class Table<
 
         return tanstackSelectionState;
     }
+
+    private updateRowHeight(): void {
+        this._rowHeight = parseFloat(controlHeight.getValueFor(this))
+            + 2 * parseFloat(borderWidth.getValueFor(this));
+        this.virtualizer?.updateRowHeight();
+    }
 }
 
 const nimbleTable = Table.compose({

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

@@ -9,7 +9,6 @@ import {
     VirtualItem,
     ScrollToOptions
 } from '@tanstack/virtual-core';
-import { borderWidth, controlHeight } from '../../theme-provider/design-tokens';
 import type { Table } from '..';
 import type { TableNode, TableRecord } from '../types';
 
@@ -38,13 +37,6 @@ export class Virtualizer<TData extends TableRecord = TableRecord> {
         return this._pageSize;
     }
 
-    private get rowHeight(): number {
-        return (
-            parseFloat(controlHeight.getValueFor(this.table))
-            + 2 * parseFloat(borderWidth.getValueFor(this.table))
-        );
-    }
-
     private readonly table: Table<TData>;
     private readonly tanStackTable: TanStackTable<TableNode<TData>>;
     private readonly viewportResizeObserver: ResizeObserver;
@@ -93,6 +85,14 @@ export class Virtualizer<TData extends TableRecord = TableRecord> {
         this.virtualizer?.scrollToIndex(index, options);
     }
 
+    public updateRowHeight(): void {
+        this.updatePageSize();
+        if (this.virtualizer) {
+            this.updateVirtualizer();
+            this.virtualizer.measure();
+        }
+    }
+
     private updateVirtualizer(): void {
         const options = this.createVirtualizerOptions();
         if (this.virtualizer) {
@@ -108,7 +108,7 @@ export class Virtualizer<TData extends TableRecord = TableRecord> {
     HTMLElement,
     HTMLElement
     > {
-        const rowHeight = this.rowHeight;
+        const rowHeight = this.table.rowHeight;
         return {
             count: this.tanStackTable.getRowModel().rows.length,
             getScrollElement: () => {
@@ -154,7 +154,7 @@ export class Virtualizer<TData extends TableRecord = TableRecord> {
 
     private updatePageSize(): void {
         this._pageSize = Math.round(
-            this.table.viewport.clientHeight / this.rowHeight
+            this.table.viewport.clientHeight / this.table.rowHeight
         );
     }
 }

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

@@ -6,6 +6,7 @@ import { TableColumnText, tableColumnTextTag } from '../../table-column/text';
 import { TableColumnTextCellView } from '../../table-column/text/cell-view';
 import { waitForUpdatesAsync } from '../../testing/async-helpers';
 import {
+    borderWidth,
     controlHeight,
     tableFitRowsHeight
 } from '../../theme-provider/design-tokens';
@@ -2442,6 +2443,49 @@ describe('Table', () => {
                 expect(tokenValue).toBe(expectedHeight);
             });
         });
+
+        describe('row height calculation', () => {
+            it('is computed with a positive value', async () => {
+                await connect();
+                await waitForUpdatesAsync();
+
+                expect(element.rowHeight).toBeGreaterThan(0);
+            });
+
+            it('rowHeight and pageSize are recomputed when the borderWidth design token changes', async () => {
+                await connect();
+                await waitForUpdatesAsync();
+
+                const initialRowHeight = element.rowHeight;
+                const initialPageSize = element.virtualizer.pageSize;
+                const initialBorderWidth = borderWidth.getValueFor(element);
+                const newBorderWidth = parseFloat(initialBorderWidth) + 8;
+                borderWidth.setValueFor(element, `${newBorderWidth}px`);
+                await waitForUpdatesAsync();
+
+                expect(element.rowHeight).toBeGreaterThan(initialRowHeight);
+                expect(element.virtualizer.pageSize).toBeLessThan(
+                    initialPageSize
+                );
+            });
+
+            it('rowHeight and pageSize are recomputed when the controlHeight design token changes', async () => {
+                await connect();
+                await waitForUpdatesAsync();
+
+                const initialRowHeight = element.rowHeight;
+                const initialPageSize = element.virtualizer.pageSize;
+                const initialControlHeight = controlHeight.getValueFor(element);
+                const newControlHeight = parseFloat(initialControlHeight) * 2;
+                controlHeight.setValueFor(element, `${newControlHeight}px`);
+                await waitForUpdatesAsync();
+
+                expect(element.rowHeight).toBeGreaterThan(initialRowHeight);
+                expect(element.virtualizer.pageSize).toBeLessThan(
+                    initialPageSize
+                );
+            });
+        });
     });
 
     describe('without connection', () => {

packages/storybook/.storybook/preview.js

@@ -105,5 +105,5 @@ configureActions({
     depth: 1
 });
 
-// Update the GUID on this line to trigger a turbosnap full rebuild: c250c9b3-0bdf-4f27-a923-672573a79aaa
+// Update the GUID on this line to trigger a turbosnap full rebuild: 35fca962-ed23-4476-979e-094c9a0b48f3
 // See https://www.chromatic.com/docs/turbosnap/#full-rebuilds