CardView - onOptionChanged is not triggered when column properties are updated (T1306434) (DevExpress#33681)

Author: markallenramirez

packages/devextreme/js/__internal/grids/new/card_view/header_panel/controller.ts

@@ -47,7 +47,7 @@ export class HeaderPanelController {
       const columnsCount = this.columnsController.columns.peek().length;
 
       this.columnsController.columnOption(column, 'visible', true);
-      this.columnsController.columnOption(column, 'visibleIndex', columnsCount);
+      this.columnsController.columnOption(column, 'visibleIndex', columnsCount - 1);
 
       return;
     }

packages/devextreme/js/__internal/grids/new/grid_core/columns_controller/columns_controller.test.ts

@@ -1,4 +1,6 @@
-import { describe, expect, it } from '@jest/globals';
+import {
+  describe, expect, it, jest,
+} from '@jest/globals';
 
 import { DataController } from '../data_controller';
 import { getContext } from '../di.test_utils';
@@ -443,4 +445,86 @@ describe('ColumnsController', () => {
       expect(resultThird.sortIndex).toBeUndefined();
     });
   });
+
+  describe('onOptionChanged', () => {
+    it('should be called when a column option changes', () => {
+      const onOptionChanged = jest.fn();
+      const { columnsController } = setup({
+        columns: ['a', 'b'],
+        onOptionChanged,
+      });
+
+      const [col] = columnsController.columns.peek();
+      columnsController.columnOption(col, 'visible', false);
+
+      expect(onOptionChanged).toHaveBeenCalledTimes(1);
+      expect(onOptionChanged).toHaveBeenCalledWith(
+        expect.objectContaining({
+          fullName: 'columns[0].visible',
+          name: 'columns',
+          previousValue: true,
+          value: false,
+        }),
+      );
+    });
+
+    it('should not be called when value is unchanged', () => {
+      const onOptionChanged = jest.fn();
+      const { columnsController } = setup({
+        columns: ['a', 'b'],
+        onOptionChanged,
+      });
+
+      const [col] = columnsController.columns.peek();
+      columnsController.columnOption(col, 'visible', true);
+
+      expect(onOptionChanged).not.toHaveBeenCalled();
+    });
+
+    it('should be called for each changed column in updateColumns', () => {
+      const onOptionChanged = jest.fn();
+      const { columnsController } = setup({
+        columns: ['a', 'b', 'c'],
+        onOptionChanged,
+      });
+
+      let sortIndex = -1;
+      columnsController.updateColumns((columns) => columns.map((col, idx) => {
+        if (idx === 1) {
+          return col;
+        }
+
+        sortIndex += 1;
+
+        return {
+          ...col,
+          sortOrder: 'asc',
+          sortIndex,
+        };
+      }));
+
+      const optionChangeCalls = onOptionChanged.mock.calls;
+      expect(optionChangeCalls).toHaveLength(4);
+      expect(optionChangeCalls[0][0]).toMatchObject({
+        fullName: 'columns[0].sortOrder',
+        name: 'columns',
+        value: 'asc',
+      });
+      expect(optionChangeCalls[1][0]).toMatchObject({
+        fullName: 'columns[0].sortIndex',
+        name: 'columns',
+        value: 0,
+      });
+      expect(optionChangeCalls[2][0]).toMatchObject({
+        fullName: 'columns[2].sortOrder',
+        name: 'columns',
+        value: 'asc',
+      });
+      expect(optionChangeCalls[3][0]).toMatchObject({
+        fullName: 'columns[2].sortIndex',
+        name: 'columns',
+        value: 1,
+      });
+    });
+  });
 });

packages/devextreme/js/__internal/grids/new/grid_core/columns_controller/columns_controller.ts

@@ -1,3 +1,5 @@
+import { equalByValue } from '@js/core/utils/common';
+import { getPathParts } from '@js/core/utils/data';
 import type { ReadonlySignal, Signal } from '@preact/signals-core';
 import { computed, effect, signal } from '@preact/signals-core';
 import type { DataObject } from '@ts/grids/new/grid_core/data_controller/types';
@@ -6,11 +8,14 @@ import { isColumnFilterable, mergeColumnHeaderFilterOptions } from '@ts/grids/ne
 import type { OptionWithChanges } from '@ts/grids/new/grid_core/options_controller/types';
 
 import { OptionsController } from '../options_controller/options_controller';
+import { getTreeNodeByPath } from '../utils/tree/index';
 import { updateColumnSettings } from './columns_settings/index';
+import { IGNORE_COLUMN_OPTION_NAMES } from './const';
 import type { ColumnProperties, ColumnSettings, PreNormalizedColumn } from './options';
 import type { Column, ColumnsConfigurationFromData, VisibleColumn } from './types';
 import {
   columnOptionUpdate,
+  getColumnByIndexOrName,
   getColumnIndexByName,
   getColumnOptionsFromDataItem,
   normalizeColumns,
@@ -121,27 +126,60 @@ export class ColumnsController {
   }
 
   public columnOption<TProp extends keyof ColumnSettings>(
-    { name }: Column,
+    column: Column,
     // TODO: Fix type -> option may be path with dots in runtime
     //  E.g: 'columnOption('A', 'headerFilter.search.enabled', true)
     option: TProp,
     value: ColumnSettings[TProp],
   ): void {
+    const { name } = column;
     const settings = this.columnsSettings.peek();
     const columnIdx = getColumnIndexByName(settings, name);
+    const prevValue = getTreeNodeByPath(column, getPathParts(option));
 
     this.columnsSettings.value = columnOptionUpdate(
       settings,
       columnIdx,
       option,
       value,
     );
+
+    this.fireOptionChanged(columnIdx, option, value, prevValue);
   }
 
   public updateColumns(func: (columns: PreNormalizedColumn[]) => PreNormalizedColumn[]): void {
+    const prevColumns = this.columns.peek();
+
     let newColumnSettings = func(this.columnsSettings.peek());
     newColumnSettings = normalizeColumnsVisibleIndexes(newColumnSettings);
     this.columnsSettings.value = newColumnSettings;
+
+    const newColumns = this.columns.peek();
+    newColumns.forEach((newColumn, columnIdx) => {
+      const prevColumn = getColumnByIndexOrName(prevColumns, newColumn.name);
+      if (prevColumn) {
+        const options = new Set([...Object.keys(prevColumn), ...Object.keys(newColumn)]);
+        for (const option of options) {
+          if (!IGNORE_COLUMN_OPTION_NAMES[option]) {
+            const prevValue = prevColumn[option];
+            const newValue = newColumn[option];
+            this.fireOptionChanged(columnIdx, option, newValue, prevValue);
+          }
+        }
+      }
+    });
+  }
+
+  private fireOptionChanged(
+    columnIndex: number,
+    optionName: string,
+    newValue: unknown,
+    prevValue: unknown,
+  ): void {
+    if (!equalByValue(prevValue, newValue, { maxDepth: 5 })) {
+      const fullOptionPath = `columns[${columnIndex}].${optionName}`;
+      this.options.notifyColumnOptionChanged(fullOptionPath, newValue, prevValue);
+    }
   }
 
   public setColumnOptionsFromDataItem(

packages/devextreme/js/__internal/grids/new/grid_core/columns_controller/const.ts

@@ -0,0 +1,3 @@
+export const IGNORE_COLUMN_OPTION_NAMES = {
+  selector: true,
+};

packages/devextreme/js/__internal/grids/new/grid_core/options_controller/options_controller_base.test.ts

@@ -828,3 +828,21 @@ describe('oneWayWithChanges', () => {
     });
   });
 });
+
+describe('notifyColumnOptionChanged', () => {
+  it('should not update the internal state', () => {
+    const publicOptions = { columns: [{ visible: true }] };
+    const { optionsController } = setup<{ columns?: { visible?: boolean }[] }>(publicOptions, {});
+
+    const columnVisible = optionsController.oneWay('columns[0].visible');
+    const columnVisibleWithChanges = optionsController.oneWayWithChanges('columns[0].visible');
+
+    optionsController.notifyColumnOptionChanged('columns[0].visible', false, true);
+
+    expect(columnVisible.peek()).toBe(true);
+    expect(columnVisibleWithChanges.peek()).toStrictEqual({
+      changes: null,
+      value: true,
+    });
+  });
+});

packages/devextreme/js/__internal/grids/new/grid_core/options_controller/options_controller_base.ts

@@ -47,6 +47,8 @@ export class OptionsController<
 
   private isControlledMode = false;
 
+  private _skipProcessingColumnsChange: string | false = false;
+
   private readonly internalOptions: Signal<InternalOptionsState<TProps>>;
 
   // @ts-expect-error Component type doesn't have fields from widget.ts
@@ -76,6 +78,10 @@ export class OptionsController<
   private onOptionChangedHandler(optionChanges: ChangedOptionInfo): void {
     const { fullName } = optionChanges;
 
+    if (this._skipProcessingColumnsChange === fullName) {
+      return;
+    }
+
     this.updateIsControlledMode();
     this.updateInternalOptionsState(fullName, optionChanges);
   }
@@ -221,4 +227,15 @@ export class OptionsController<
       );
     });
   }
+
+  public notifyColumnOptionChanged(
+    fullOptionPath: string,
+    newValue: unknown,
+    prevValue: unknown,
+  ): void {
+    this._skipProcessingColumnsChange = fullOptionPath;
+    // @ts-expect-error
+    this.component._notifyOptionChanged(fullOptionPath, newValue, prevValue);
+    this._skipProcessingColumnsChange = false;
+  }
 }