Optimize column state sync

CHANGELOG.md

@@ -5,6 +5,7 @@
 ### 🐞 Bug Fixes
 
 * Fixed `ViewManagerModel` unique name validation.
+* Fixed un-optimal column state synchronization between `GridModel` and Ag-Grid
 
 ### ⚙️ Technical
 

cmp/ag-grid/AgGridModel.ts

@@ -395,7 +395,7 @@ export class AgGridModel extends HoistModel {
             ];
 
         let {isPivot, columns} = colState;
-        agApi.setPivotMode(isPivot);
+        agApi.setGridOption('pivotMode', isPivot);
 
         if (isPivot && columns.some(it => !isNil(it.pivotIndex) && it.pivotIndex >= 0)) {
             // Exclude the auto group column as this causes issues with ag-grid when in pivot mode

cmp/grid/Grid.ts

@@ -4,9 +4,10 @@
  *
  * Copyright © 2025 Extremely Heavy Industries Inc.
  */
+import {ColumnState as AgColumnState, GridApi} from '@ag-grid-community/core';
 import composeRefs from '@seznam/compose-react-refs';
 import {agGrid, AgGrid} from '@xh/hoist/cmp/ag-grid';
-import {getTreeStyleClasses} from '@xh/hoist/cmp/grid';
+import {ColumnState, getTreeStyleClasses} from '@xh/hoist/cmp/grid';
 import {gridHScrollbar} from '@xh/hoist/cmp/grid/impl/GridHScrollbar';
 import {getAgGridMenuItems} from '@xh/hoist/cmp/grid/impl/MenuSupport';
 import {div, fragment, frame, vframe} from '@xh/hoist/cmp/layout';
@@ -17,6 +18,7 @@ import {
     LayoutProps,
     lookup,
     PlainObject,
+    ReactionSpec,
     TestSupportProps,
     useLocalModel,
     uses,
@@ -44,7 +46,7 @@ import {wait} from '@xh/hoist/promise';
 import {consumeEvent, isDisplayed, logWithDebug} from '@xh/hoist/utils/js';
 import {createObservableRef, getLayoutProps} from '@xh/hoist/utils/react';
 import classNames from 'classnames';
-import {debounce, isEmpty, isEqual, isNil, max, maxBy, merge} from 'lodash';
+import {compact, debounce, isBoolean, isEmpty, isEqual, isNil, max, maxBy, merge} from 'lodash';
 import './Grid.scss';
 import {GridModel} from './GridModel';
 import {columnGroupHeader} from './impl/ColumnGroupHeader';
@@ -463,7 +465,7 @@ export class GridLocalModel extends HoistModel {
         };
     }
 
-    columnStateReaction() {
+    columnStateReaction(): ReactionSpec<[GridApi, ColumnState[]]> {
         const {model} = this;
         return {
             track: () => [model.agApi, model.columnState],
@@ -472,65 +474,57 @@ export class GridLocalModel extends HoistModel {
 
                 const agColState = api.getColumnState();
 
-                // 0) Insert the auto group col state if it exists, since we won't have it in our column state list
+                // Insert the auto group col state if it exists, since we won't have it in our column state list
                 const autoColState = agColState.find(c => c.colId === 'ag-Grid-AutoColumn');
                 if (autoColState) {
-                    colState.splice(agColState.indexOf(autoColState), 0, autoColState);
+                    const {colId, width, hide, pinned} = autoColState;
+                    colState.splice(agColState.indexOf(autoColState), 0, {
+                        colId,
+                        width,
+                        hidden: hide,
+                        pinned: isBoolean(pinned) ? (pinned ? 'left' : null) : pinned
+                    });
                 }
 
-                // 1) Columns all in right place -- simply update incorrect props we maintain
-                if (
-                    isEqual(
-                        colState.map(c => c.colId),
-                        agColState.map(c => c.colId)
-                    )
-                ) {
-                    let hasChanges = false;
-                    colState.forEach((col, index) => {
-                        const agCol = agColState[index],
-                            id = col.colId;
-
-                        if (agCol.width !== col.width) {
-                            api.setColumnWidths([{key: id, newWidth: col.width}]);
+                // Determine if column order has changed
+                const applyOrder = !isEqual(
+                    colState.map(c => c.colId),
+                    agColState.map(c => c.colId)
+                );
+
+                // Build a list of column state changes
+                colState = compact(
+                    colState.map(({colId, width, hidden, pinned}) => {
+                        const agCol: AgColumnState = agColState.find(c => c.colId === colId) || {
+                                colId
+                            },
+                            ret: any = {colId};
+
+                        let hasChanges = applyOrder;
+
+                        if (agCol.width !== width) {
+                            ret.width = width;
                             hasChanges = true;
                         }
-                        if (agCol.hide !== col.hidden) {
-                            api.setColumnsVisible([id], !col.hidden);
+
+                        if (agCol.hide !== hidden) {
+                            ret.hide = hidden;
                             hasChanges = true;
                         }
-                        if (agCol.pinned !== col.pinned) {
-                            api.setColumnsPinned([id], col.pinned);
+
+                        if (agCol.pinned !== pinned) {
+                            ret.pinned = pinned;
                             hasChanges = true;
                         }
-                    });
-
-                    // We need to tell agGrid to refresh its flexed column sizes due to
-                    // a regression introduced in 25.1.0.  See #2341
-                    if (hasChanges) {
-                        api.columnModel.refreshFlexedColumns({
-                            updateBodyWidths: true,
-                            fireResizedEvent: true
-                        });
-                    }
 
-                    return;
-                }
+                        return hasChanges ? ret : null;
+                    })
+                );
 
-                // 2) Otherwise do an (expensive) full refresh of column state
-                // Merge our state onto the ag column state to get any state which we do not yet support
-                colState = colState.map(({colId, width, hidden, pinned}) => {
-                    const agCol = agColState.find(c => c.colId === colId) || {};
-                    return {
-                        colId,
-                        ...agCol,
-                        width,
-                        pinned,
-                        hide: hidden
-                    };
-                });
+                if (isEmpty(colState)) return;
 
                 this.doWithPreservedState({expansion: false}, () => {
-                    api.applyColumnState({state: colState, applyOrder: true});
+                    api.applyColumnState({state: colState, applyOrder});
                 });
             }
         };
@@ -870,6 +864,7 @@ export class GridLocalModel extends HoistModel {
      * by conditionally stopping the focus event from propagating.
      */
     private static didAddFocusFixListener = false;
+
     static addFocusFixListener() {
         if (this.didAddFocusFixListener) return;
         document.addEventListener(

cmp/grid/GridModel.ts

@@ -9,6 +9,7 @@ import {
     CellContextMenuEvent,
     CellDoubleClickedEvent,
     ColumnEvent,
+    ColumnState as AgColumnState,
     RowClickedEvent,
     RowDoubleClickedEvent
 } from '@ag-grid-community/core';
@@ -77,6 +78,7 @@ import {
     first,
     forEach,
     isArray,
+    isBoolean,
     isEmpty,
     isFunction,
     isNil,
@@ -1072,17 +1074,19 @@ export class GridModel extends HoistModel {
         (this.colChooserModel as any)?.open();
     }
 
-    noteAgColumnStateChanged(agColState) {
-        const colStateChanges = agColState.map(({colId, width, hide, pinned}) => {
-            const col = this.findColumn(this.columns, colId);
-            if (!col) return null;
-            return {
-                colId,
-                pinned: pinned ?? null,
-                hidden: !!hide,
-                width: col.flex ? undefined : width
-            };
-        });
+    noteAgColumnStateChanged(agColState: AgColumnState[]) {
+        const colStateChanges: Partial<ColumnState>[] = agColState.map(
+            ({colId, width, hide, pinned}) => {
+                const col = this.findColumn(this.columns, colId);
+                if (!col) return null;
+                return {
+                    colId,
+                    pinned: isBoolean(pinned) ? (pinned ? 'left' : null) : pinned,
+                    hidden: !!hide,
+                    width: col.flex ? undefined : width
+                };
+            }
+        );
 
         pull(colStateChanges, null);
         this.applyColumnStateChanges(colStateChanges);