Skip to content

Commit ee2d232

Browse files
Alyar666Alyar666
authored
DataGrid: Fix updating of the focused row index after a row update when the repaintChangesOnly option is enabled (T1292991) (#30994)
1 parent b99bbb7 commit ee2d232

File tree

3 files changed

+156
-52
lines changed

3 files changed

+156
-52
lines changed

‎packages/devextreme/js/__internal/grids/grid_core/focus/m_focus.integration.test.ts‎

Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,103 @@
1+
import {
2+
afterEach, describe, expect, it, jest,
3+
} from '@jest/globals';
4+
import type { dxElementWrapper } from '@js/core/renderer';
5+
import $ from '@js/core/renderer';
6+
import type { Properties as DataGridProperties } from '@js/ui/data_grid';
7+
import DataGrid from '@js/ui/data_grid';
8+
9+
const SELECTORS = {
10+
gridContainer: '#gridContainer',
11+
};
12+
13+
const GRID_CONTAINER_ID = 'gridContainer';
14+
15+
const createDataGrid = async (
16+
options: DataGridProperties = {},
17+
): Promise<{ $container: dxElementWrapper; instance: DataGrid }> => new Promise((resolve) => {
18+
const $container = $('<div>')
19+
.attr('id', GRID_CONTAINER_ID)
20+
.appendTo(document.body);
21+
22+
const instance = new DataGrid($container.get(0) as HTMLDivElement, {
23+
// @ts-ignore
24+
loadingTimeout: null,
25+
...options,
26+
});
27+
28+
resolve({ $container, instance });
29+
});
30+
31+
describe('GridCore focus', () => {
32+
afterEach(() => {
33+
const $container = $(SELECTORS.gridContainer);
34+
const dataGrid = ($container as any).dxDataGrid('instance') as DataGrid;
35+
36+
dataGrid.dispose();
37+
$container.remove();
38+
});
39+
40+
const testCases: [boolean, 'insert' | 'remove' | 'update', number][] = [
41+
[true, 'insert', 2],
42+
[true, 'remove', 0],
43+
[true, 'update', 2],
44+
[false, 'insert', 2],
45+
[false, 'remove', 0],
46+
[false, 'update', 2],
47+
];
48+
49+
// T1292991
50+
describe.each(testCases)(
51+
'when repaintChangesOnly=%s and performing %s operation',
52+
(repaintChangesOnly, operation, expectedFocusedRowIndex) => {
53+
it('should updates the focused row index correctly', async () => {
54+
const onFocusedRowChanged = jest.fn();
55+
const { instance } = await createDataGrid({
56+
dataSource: {
57+
store: {
58+
type: 'array',
59+
data: [
60+
{ id: 1, name: 'Item 1' },
61+
{ id: 2, name: 'Item 2' },
62+
{ id: 3, name: 'Item 3' },
63+
],
64+
key: 'id',
65+
},
66+
reshapeOnPush: true,
67+
pushAggregationTimeout: 0,
68+
},
69+
showBorders: true,
70+
focusedRowEnabled: true,
71+
focusedRowKey: 2,
72+
onFocusedRowChanged,
73+
repaintChangesOnly,
74+
columns: [
75+
{ dataField: 'id', width: 80 },
76+
{ dataField: 'name', caption: 'Name', sortOrder: 'asc' },
77+
],
78+
});
79+
const store = instance.getDataSource().store();
80+
81+
onFocusedRowChanged.mockClear();
82+
83+
switch (operation) {
84+
case 'insert':
85+
store.push([{ type: 'insert', index: 0, data: { name: 'Item 0' } }]);
86+
break;
87+
case 'remove':
88+
store.push([{ type: 'remove', key: 1 }]);
89+
break;
90+
case 'update':
91+
store.push([{ type: 'update', key: 3, data: { id: 3, name: 'A Item 3' } }]);
92+
break;
93+
default:
94+
break;
95+
}
96+
97+
expect(onFocusedRowChanged.mock.calls.length).toBe(1);
98+
expect(instance.option('focusedRowKey')).toEqual(2);
99+
expect(instance.option('focusedRowIndex')).toEqual(expectedFocusedRowIndex);
100+
});
101+
},
102+
);
103+
});

‎packages/devextreme/js/__internal/grids/grid_core/focus/m_focus.ts‎

Lines changed: 52 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -84,30 +84,18 @@ export class FocusController extends core.ViewController {
8484
if (!this.option('focusedRowEnabled')) {
8585
return;
8686
}
87-
const isEmptyData = this.getDataController().isEmpty();
88-
const currentIndex = this._getCurrentFocusRowIndex(isEmptyData, index);
87+
88+
const currentIndex = index !== undefined ? index : this.option('focusedRowIndex');
8989

9090
if (currentIndex < 0) {
91-
if (isEmptyData || this.isAutoNavigateToFocusedRow()) {
91+
if (this.isAutoNavigateToFocusedRow()) {
9292
this._resetFocusedRow();
9393
}
9494
} else {
9595
this._focusRowByIndexCore(currentIndex, operationTypes);
9696
}
9797
}
9898

99-
private _getCurrentFocusRowIndex(isEmptyData, index?): number {
100-
let currentIndex = index;
101-
if (currentIndex === undefined) {
102-
if (isEmptyData) {
103-
currentIndex = -1;
104-
} else {
105-
currentIndex = this.option('focusedRowIndex');
106-
}
107-
}
108-
return currentIndex;
109-
}
110-
11199
private _focusRowByIndexCore(index, operationTypes) {
112100
const pageSize = this.getDataController().pageSize();
113101
const setKeyByIndex = () => {
@@ -560,7 +548,7 @@ const columns = (Base: ModuleType<ColumnsController>) => class FocusColumnsExten
560548
};
561549

562550
const data = (Base: ModuleType<DataController>) => class FocusDataControllerExtender extends Base {
563-
private _needToUpdateFocusedRowByIndex = false;
551+
private _isDataPushed = false;
564552

565553
protected _applyChange(change) {
566554
if (change && change.changeType === 'updateFocusedRow') return;
@@ -572,20 +560,23 @@ const data = (Base: ModuleType<DataController>) => class FocusDataControllerExte
572560
protected _fireChanged(e) {
573561
super._fireChanged(e);
574562

563+
const forceUpdateFocusedRow = this._isDataPushed;
564+
565+
this._isDataPushed = false;
566+
575567
if (this.option('focusedRowEnabled') && this._dataSource) {
576568
const isPartialUpdate = e.changeType === 'update' && e.repaintChangesOnly;
577569
const isPartialUpdateWithDeleting = isPartialUpdate && e.changeTypes && e.changeTypes.indexOf('remove') >= 0;
578570

579-
if (this._needToUpdateFocusedRowByIndex) {
580-
this._needToUpdateFocusedRowByIndex = false;
581-
this._focusController._focusRowByIndex();
571+
if (forceUpdateFocusedRow && this.isEmpty()) {
572+
this._focusController._resetFocusedRow();
582573
} else if (e.changeType === 'refresh' && e.items.length || isPartialUpdateWithDeleting) {
583574
this._updatePageIndexes();
584-
this._updateFocusedRow(e);
575+
this._updateFocusedRowIfNeeded(e, forceUpdateFocusedRow);
585576
} else if (e.changeType === 'append' || e.changeType === 'prepend') {
586577
this._updatePageIndexes();
587-
} else if (e.changeType === 'update' && e.repaintChangesOnly) {
588-
this._updateFocusedRow(e);
578+
} else if (isPartialUpdate) {
579+
this._updateFocusedRowIfNeeded(e, forceUpdateFocusedRow);
589580
}
590581
}
591582
}
@@ -595,7 +586,7 @@ const data = (Base: ModuleType<DataController>) => class FocusDataControllerExte
595586

596587
const focusedRowKey = this.option('focusedRowKey');
597588

598-
this._needToUpdateFocusedRowByIndex = changes?.some((change) => change.type === 'remove' && equalByValue(change.key, focusedRowKey));
589+
this._isDataPushed = isDefined(focusedRowKey) && !!changes.length;
599590
}
600591

601592
private _updatePageIndexes() {
@@ -610,7 +601,7 @@ const data = (Base: ModuleType<DataController>) => class FocusDataControllerExte
610601
return this._isPagingByRendering;
611602
}
612603

613-
private _updateFocusedRow(e) {
604+
private _updateFocusedRowIfNeeded(e, forceUpdate = false) {
614605
const operationTypes = e.operationTypes || {};
615606
const {
616607
reload, fullReload, pageIndex, paging,
@@ -620,30 +611,44 @@ const data = (Base: ModuleType<DataController>) => class FocusDataControllerExte
620611
const focusedRowKey = this.option('focusedRowKey');
621612
const isAutoNavigate = this._focusController.isAutoNavigateToFocusedRow();
622613
const isReload = reload && pageIndex === false;
623-
if (isReload && !fullReload && isDefined(focusedRowKey)) {
624-
this._focusController._navigateToRow(focusedRowKey, true)
625-
.done((focusedRowIndex) => {
626-
if (focusedRowIndex < 0) {
627-
this._focusController._focusRowByIndex(undefined, operationTypes);
628-
}
629-
});
630-
} else if (pagingWithoutVirtualScrolling && isAutoNavigate) {
631-
const rowIndexByKey = this.getRowIndexByKey(focusedRowKey);
632-
const focusedRowIndex = this.option('focusedRowIndex')!;
633-
const isValidRowIndexByKey = rowIndexByKey >= 0;
634-
const isValidFocusedRowIndex = focusedRowIndex >= 0;
635-
const isSameRowIndex = focusedRowIndex === rowIndexByKey;
636-
if (isValidFocusedRowIndex && (isSameRowIndex || !isValidRowIndexByKey)) {
637-
this._focusController._focusRowByIndex(focusedRowIndex, operationTypes);
614+
const rowIndexByKey = this.getRowIndexByKey(focusedRowKey);
615+
616+
switch (true) {
617+
case forceUpdate: {
618+
this._focusController._focusRowByKeyOrIndex();
619+
break;
620+
}
621+
case isReload && !fullReload && isDefined(focusedRowKey): {
622+
this._focusController._navigateToRow(focusedRowKey, true)
623+
.done((focusedRowIndex) => {
624+
if (focusedRowIndex < 0) {
625+
this._focusController._focusRowByIndex(undefined, operationTypes);
626+
}
627+
});
628+
break;
629+
}
630+
case pagingWithoutVirtualScrolling && isAutoNavigate: {
631+
const focusedRowIndex = this.option('focusedRowIndex')!;
632+
const isValidRowIndexByKey = rowIndexByKey >= 0;
633+
const isValidFocusedRowIndex = focusedRowIndex >= 0;
634+
const isSameRowIndex = focusedRowIndex === rowIndexByKey;
635+
636+
if (isValidFocusedRowIndex && (isSameRowIndex || !isValidRowIndexByKey)) {
637+
this._focusController._focusRowByIndex(focusedRowIndex, operationTypes);
638+
}
639+
break;
640+
}
641+
case pagingWithoutVirtualScrolling && !isAutoNavigate && (rowIndexByKey < 0): {
642+
this.option('focusedRowIndex', -1);
643+
break;
644+
}
645+
case operationTypes.fullReload: {
646+
this._focusController._focusRowByKeyOrIndex();
647+
break;
648+
}
649+
default: {
650+
break;
638651
}
639-
} else if (
640-
pagingWithoutVirtualScrolling
641-
&& !isAutoNavigate
642-
&& (this.getRowIndexByKey(focusedRowKey) < 0)
643-
) {
644-
this.option('focusedRowIndex', -1);
645-
} else if (operationTypes.fullReload) {
646-
this._focusController._focusRowByKeyOrIndex();
647652
}
648653
}
649654

‎packages/devextreme/js/__internal/grids/grid_core/keyboard_navigation/m_keyboard_navigation.ts‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2791,11 +2791,7 @@ const rowsView = (Base: ModuleType<RowsView>) => class RowsViewKeyboardExtender
27912791
const { operationTypes, repaintChangesOnly } = change ?? {};
27922792
const { fullReload, pageSize } = operationTypes ?? {};
27932793

2794-
const hasInsertsOrRemoves = !!change?.changeTypes?.find(
2795-
(changeType) => changeType === 'insert' || changeType === 'remove',
2796-
);
2797-
2798-
if (!change || !repaintChangesOnly || fullReload || pageSize || hasInsertsOrRemoves) {
2794+
if (!change || !repaintChangesOnly || fullReload || pageSize) {
27992795
const preventScroll = shouldPreventScroll(this);
28002796
this.renderFocusState({
28012797
preventScroll,

0 commit comments

Comments
 (0)