Skip to content

Commit da40488

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

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
@@ -83,30 +83,18 @@ export class FocusController extends core.ViewController {
8383
if (!this.option('focusedRowEnabled')) {
8484
return;
8585
}
86-
const isEmptyData = this.getDataController().isEmpty();
87-
const currentIndex = this._getCurrentFocusRowIndex(isEmptyData, index);
86+
87+
const currentIndex = index !== undefined ? index : this.option('focusedRowIndex');
8888

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

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

561549
const data = (Base: ModuleType<DataController>) => class FocusDataControllerExtender extends Base {
562-
private _needToUpdateFocusedRowByIndex = false;
550+
private _isDataPushed = false;
563551

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

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

578-
if (this._needToUpdateFocusedRowByIndex) {
579-
this._needToUpdateFocusedRowByIndex = false;
580-
this._focusController._focusRowByIndex();
570+
if (forceUpdateFocusedRow && this.isEmpty()) {
571+
this._focusController._resetFocusedRow();
581572
} else if (e.changeType === 'refresh' && e.items.length || isPartialUpdateWithDeleting) {
582573
this._updatePageIndexes();
583-
this._updateFocusedRow(e);
574+
this._updateFocusedRowIfNeeded(e, forceUpdateFocusedRow);
584575
} else if (e.changeType === 'append' || e.changeType === 'prepend') {
585576
this._updatePageIndexes();
586-
} else if (e.changeType === 'update' && e.repaintChangesOnly) {
587-
this._updateFocusedRow(e);
577+
} else if (isPartialUpdate) {
578+
this._updateFocusedRowIfNeeded(e, forceUpdateFocusedRow);
588579
}
589580
}
590581
}
@@ -594,7 +585,7 @@ const data = (Base: ModuleType<DataController>) => class FocusDataControllerExte
594585

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

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

600591
private _updatePageIndexes() {
@@ -609,7 +600,7 @@ const data = (Base: ModuleType<DataController>) => class FocusDataControllerExte
609600
return this._isPagingByRendering;
610601
}
611602

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

‎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
@@ -2820,11 +2820,7 @@ const rowsView = (Base: ModuleType<RowsView>) => class RowsViewKeyboardExtender
28202820
const { operationTypes, repaintChangesOnly } = change ?? {};
28212821
const { fullReload, pageSize } = operationTypes ?? {};
28222822

2823-
const hasInsertsOrRemoves = !!change?.changeTypes?.find(
2824-
(changeType) => changeType === 'insert' || changeType === 'remove',
2825-
);
2826-
2827-
if (!change || !repaintChangesOnly || fullReload || pageSize || hasInsertsOrRemoves) {
2823+
if (!change || !repaintChangesOnly || fullReload || pageSize) {
28282824
const preventScroll = shouldPreventScroll(this);
28292825
this.renderFocusState({
28302826
preventScroll,

0 commit comments

Comments
 (0)