From 61857b79fe8e5a0ca7600e95e264edc19f2a2af9 Mon Sep 17 00:00:00 2001 From: Eldar Iusupzhanov Date: Fri, 10 Jan 2025 14:39:56 +0800 Subject: [PATCH 1/6] fix --- .../grid_core/keyboard_navigation/m_keyboard_navigation.ts | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/packages/devextreme/js/__internal/grids/grid_core/keyboard_navigation/m_keyboard_navigation.ts b/packages/devextreme/js/__internal/grids/grid_core/keyboard_navigation/m_keyboard_navigation.ts index aca95231433f..3e625bd0e6d0 100644 --- a/packages/devextreme/js/__internal/grids/grid_core/keyboard_navigation/m_keyboard_navigation.ts +++ b/packages/devextreme/js/__internal/grids/grid_core/keyboard_navigation/m_keyboard_navigation.ts @@ -331,6 +331,13 @@ export class KeyboardNavigationController extends modules.ViewController { this._documentClickHandler = this._documentClickHandler || this.createAction((e) => { const $target = $(e.event.target); + + // if target is no more presented in the DOM, then prevent unfocusing the focused view + if (!$target.get(0).isConnected) { + e.event.preventDefault(); + return; + } + const isCurrentRowsViewClick = this._isEventInCurrentGrid(e.event) && $target.closest(`.${this.addWidgetPrefix(ROWS_VIEW_CLASS)}`).length; const isEditorOverlay = $target.closest( From 9f180d36de43d9b848230c807c911926208aa061 Mon Sep 17 00:00:00 2001 From: Eldar Iusupzhanov Date: Fri, 10 Jan 2025 15:43:15 +0800 Subject: [PATCH 2/6] preventDefault only if click was inside the datagrid --- .../grid_core/keyboard_navigation/m_keyboard_navigation.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/devextreme/js/__internal/grids/grid_core/keyboard_navigation/m_keyboard_navigation.ts b/packages/devextreme/js/__internal/grids/grid_core/keyboard_navigation/m_keyboard_navigation.ts index 3e625bd0e6d0..b44c929fac2c 100644 --- a/packages/devextreme/js/__internal/grids/grid_core/keyboard_navigation/m_keyboard_navigation.ts +++ b/packages/devextreme/js/__internal/grids/grid_core/keyboard_navigation/m_keyboard_navigation.ts @@ -332,8 +332,9 @@ export class KeyboardNavigationController extends modules.ViewController { this._documentClickHandler = this._documentClickHandler || this.createAction((e) => { const $target = $(e.event.target); - // if target is no more presented in the DOM, then prevent unfocusing the focused view - if (!$target.get(0).isConnected) { + // if click was on the datagrid, but the target is no more presented in the DOM + if (!$target.get(0).isConnected && $target.closest('.dx-datagrid-table').length) { + // then prevent unfocusing the focused view e.event.preventDefault(); return; } From 6cc021230ba716893dac48debbcb62e279cf1075 Mon Sep 17 00:00:00 2001 From: Eldar Iusupzhanov Date: Fri, 10 Jan 2025 17:48:47 +0800 Subject: [PATCH 3/6] add test --- .../focus.integration.tests.js | 37 +++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/packages/devextreme/testing/tests/DevExpress.ui.widgets.dataGrid/focus.integration.tests.js b/packages/devextreme/testing/tests/DevExpress.ui.widgets.dataGrid/focus.integration.tests.js index eae2d8691466..130f0f769f74 100644 --- a/packages/devextreme/testing/tests/DevExpress.ui.widgets.dataGrid/focus.integration.tests.js +++ b/packages/devextreme/testing/tests/DevExpress.ui.widgets.dataGrid/focus.integration.tests.js @@ -4384,6 +4384,43 @@ QUnit.module('View\'s focus', { assert.equal(this.dataGrid.option('focusedRowKey'), 2, 'row key is changed'); assert.ok($(this.dataGrid.getRowElement(1)).hasClass('dx-row-focused'), 'second row is focused'); }); + + // T1224663 + ['onFocusedRowChanged', 'onFocusedRowChanging'].forEach(event => { + QUnit.testInActiveWindow(`Focus should be preserved on datagrid when rowsview repaints in ${event} event`, function(assert) { + // arrange + this.dataGrid.option({ + dataSource: [ + { id: 1, name: 'name 1' }, + { id: 2, name: 'name 2' }, + { id: 3, name: 'name 3' } + ], + keyExpr: 'id', + focusedRowEnabled: true, + [event]: (e) => { + this.dataGrid.repaint(); + } + }); + this.clock.tick(300); + + // act + $(this.dataGrid.getCellElement(0, 0)).trigger(CLICK_EVENT); + this.clock.tick(300); + + // assert + const firstRow = $(this.dataGrid.getRowElement(0)); + assert.ok(firstRow.hasClass('dx-row-focused'), 'first row is focused'); + + // act + const keyboard = keyboardMock(firstRow); + keyboard.keyDown('down'); + this.clock.tick(300); + + // assert + const secondRow = $(this.dataGrid.getRowElement(1)); + assert.ok(secondRow.hasClass('dx-row-focused'), 'second row is focused'); + }); + }); }); QUnit.module('API methods', baseModuleConfig, () => { From 9540ba0e79d829e0c709108fa6b7903b1718fc0d Mon Sep 17 00:00:00 2001 From: Eldar Iusupzhanov Date: Fri, 10 Jan 2025 18:15:18 +0800 Subject: [PATCH 4/6] refactoring --- .../grid_core/keyboard_navigation/const.ts | 1 + .../m_keyboard_navigation.ts | 35 +++++++++++-------- 2 files changed, 22 insertions(+), 14 deletions(-) diff --git a/packages/devextreme/js/__internal/grids/grid_core/keyboard_navigation/const.ts b/packages/devextreme/js/__internal/grids/grid_core/keyboard_navigation/const.ts index aa70eac76c9b..f4c3cf0901c6 100644 --- a/packages/devextreme/js/__internal/grids/grid_core/keyboard_navigation/const.ts +++ b/packages/devextreme/js/__internal/grids/grid_core/keyboard_navigation/const.ts @@ -4,6 +4,7 @@ export const ATTRIBUTES = { }; export const ROWS_VIEW_CLASS = 'rowsview'; +export const TABLE_CLASS = 'table'; export const EDIT_FORM_CLASS = 'edit-form'; export const GROUP_FOOTER_CLASS = 'group-footer'; export const ROW_CLASS = 'dx-row'; diff --git a/packages/devextreme/js/__internal/grids/grid_core/keyboard_navigation/m_keyboard_navigation.ts b/packages/devextreme/js/__internal/grids/grid_core/keyboard_navigation/m_keyboard_navigation.ts index b44c929fac2c..9e3161ceba90 100644 --- a/packages/devextreme/js/__internal/grids/grid_core/keyboard_navigation/m_keyboard_navigation.ts +++ b/packages/devextreme/js/__internal/grids/grid_core/keyboard_navigation/m_keyboard_navigation.ts @@ -71,6 +71,7 @@ import { REVERT_BUTTON_CLASS, ROWS_VIEW, ROWS_VIEW_CLASS, + TABLE_CLASS, WIDGET_CLASS, } from './const'; import { GridCoreKeyboardNavigationDom } from './dom'; @@ -332,25 +333,31 @@ export class KeyboardNavigationController extends modules.ViewController { this._documentClickHandler = this._documentClickHandler || this.createAction((e) => { const $target = $(e.event.target); - // if click was on the datagrid, but the target is no more presented in the DOM - if (!$target.get(0).isConnected && $target.closest('.dx-datagrid-table').length) { - // then prevent unfocusing the focused view + const tableSelector = `.${this.addWidgetPrefix(TABLE_CLASS)}`; + const rowsViewSelector = `.${this.addWidgetPrefix(ROWS_VIEW_CLASS)}`; + const editorOverlaySelector = `.${DROPDOWN_EDITOR_OVERLAY_CLASS}`; + + // if click was on the datagrid table, but the target element is no more presented in the DOM + const keepFocus = !!$target.closest(tableSelector).length && !$target.get(0).isConnected; + + if (keepFocus) { e.event.preventDefault(); return; } - const isCurrentRowsViewClick = this._isEventInCurrentGrid(e.event) - && $target.closest(`.${this.addWidgetPrefix(ROWS_VIEW_CLASS)}`).length; - const isEditorOverlay = $target.closest( - `.${DROPDOWN_EDITOR_OVERLAY_CLASS}`, - ).length; - const isColumnResizing = !!this._columnResizerController && this._columnResizerController.isResizing(); - if (!isCurrentRowsViewClick && !isEditorOverlay && !isColumnResizing) { - const targetInsideFocusedView = this._focusedView - ? $target.parents().filter(this._focusedView.element()).length > 0 - : false; + const isRowsViewClick = this._isEventInCurrentGrid(e.event) && !!$target.closest(rowsViewSelector).length; + const isEditorOverlayClick = !!$target.closest(editorOverlaySelector).length; + const isColumnResizing = !!this._columnResizerController?.isResizing(); + + if (!isRowsViewClick && !isEditorOverlayClick && !isColumnResizing) { + const outsideFocusedView = this._focusedView + ? $target.closest(this._focusedView.element()).length === 0 + : true; + + if (outsideFocusedView) { + this._resetFocusedCell(true); + } - !targetInsideFocusedView && this._resetFocusedCell(true); this._resetFocusedView(); } }); From 7442113ebd956e82e7344aa1b80ac58d4350d705 Mon Sep 17 00:00:00 2001 From: Eldar Iusupzhanov Date: Tue, 14 Jan 2025 15:25:03 +0800 Subject: [PATCH 5/6] apply review --- .../keyboard_navigation/m_keyboard_navigation.ts | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/packages/devextreme/js/__internal/grids/grid_core/keyboard_navigation/m_keyboard_navigation.ts b/packages/devextreme/js/__internal/grids/grid_core/keyboard_navigation/m_keyboard_navigation.ts index 9e3161ceba90..63f7602f3192 100644 --- a/packages/devextreme/js/__internal/grids/grid_core/keyboard_navigation/m_keyboard_navigation.ts +++ b/packages/devextreme/js/__internal/grids/grid_core/keyboard_navigation/m_keyboard_navigation.ts @@ -24,6 +24,7 @@ import { import { isDeferred, isDefined, isEmptyObject } from '@js/core/utils/type'; import * as accessibility from '@js/ui/shared/accessibility'; import { focused } from '@js/ui/widget/selectors'; +import { isElementInDom } from '@ts/core/utils/m_dom'; import type { AdaptiveColumnsController } from '@ts/grids/grid_core/adaptivity/m_adaptivity'; import type { DataController } from '@ts/grids/grid_core/data_controller/m_data_controller'; import type { EditingController } from '@ts/grids/grid_core/editing/m_editing'; @@ -338,9 +339,9 @@ export class KeyboardNavigationController extends modules.ViewController { const editorOverlaySelector = `.${DROPDOWN_EDITOR_OVERLAY_CLASS}`; // if click was on the datagrid table, but the target element is no more presented in the DOM - const keepFocus = !!$target.closest(tableSelector).length && !$target.get(0).isConnected; + const needKeepFocus = !!$target.closest(tableSelector).length && !isElementInDom($target); - if (keepFocus) { + if (needKeepFocus) { e.event.preventDefault(); return; } @@ -350,11 +351,11 @@ export class KeyboardNavigationController extends modules.ViewController { const isColumnResizing = !!this._columnResizerController?.isResizing(); if (!isRowsViewClick && !isEditorOverlayClick && !isColumnResizing) { - const outsideFocusedView = this._focusedView + const isClickOutsideFocusedView = this._focusedView ? $target.closest(this._focusedView.element()).length === 0 : true; - if (outsideFocusedView) { + if (isClickOutsideFocusedView) { this._resetFocusedCell(true); } From 212cf2ea2d14457627399859d8ac7ee5b328bc2e Mon Sep 17 00:00:00 2001 From: Eldar Iusupzhanov Date: Thu, 16 Jan 2025 14:44:53 +0800 Subject: [PATCH 6/6] rewrite test in testcafe --- .../tests/dataGrid/focus/focus.ts | 27 ++++++++++++++ .../focus.integration.tests.js | 37 ------------------- 2 files changed, 27 insertions(+), 37 deletions(-) diff --git a/e2e/testcafe-devextreme/tests/dataGrid/focus/focus.ts b/e2e/testcafe-devextreme/tests/dataGrid/focus/focus.ts index ec66d94824bb..b3e056b76ab7 100644 --- a/e2e/testcafe-devextreme/tests/dataGrid/focus/focus.ts +++ b/e2e/testcafe-devextreme/tests/dataGrid/focus/focus.ts @@ -240,3 +240,30 @@ test('DataGrid - FilterRow cell loses focus when focusedRowEnabled is true and e }); }); }); + +['onFocusedRowChanged', 'onFocusedRowChanging'].forEach((event) => { + test(`Focus should be preserved on datagrid when rowsview repaints in ${event} event (T1224663)`, async (t) => { + const dataGrid = new DataGrid('#container'); + + await t + .click(dataGrid.getDataCell(0, 0).element) + .expect(dataGrid.getDataRow(0).isFocusedRow) + .ok(); + + await t + .pressKey('down') + .expect(dataGrid.getDataRow(1).isFocusedRow) + .ok(); + }).before(async () => createWidget('dxDataGrid', { + dataSource: [ + { id: 1, name: 'name 1' }, + { id: 2, name: 'name 2' }, + { id: 3, name: 'name 3' }, + ], + keyExpr: 'id', + focusedRowEnabled: true, + [event]: (e) => { + e.component.repaint(); + }, + })); +}); diff --git a/packages/devextreme/testing/tests/DevExpress.ui.widgets.dataGrid/focus.integration.tests.js b/packages/devextreme/testing/tests/DevExpress.ui.widgets.dataGrid/focus.integration.tests.js index 130f0f769f74..eae2d8691466 100644 --- a/packages/devextreme/testing/tests/DevExpress.ui.widgets.dataGrid/focus.integration.tests.js +++ b/packages/devextreme/testing/tests/DevExpress.ui.widgets.dataGrid/focus.integration.tests.js @@ -4384,43 +4384,6 @@ QUnit.module('View\'s focus', { assert.equal(this.dataGrid.option('focusedRowKey'), 2, 'row key is changed'); assert.ok($(this.dataGrid.getRowElement(1)).hasClass('dx-row-focused'), 'second row is focused'); }); - - // T1224663 - ['onFocusedRowChanged', 'onFocusedRowChanging'].forEach(event => { - QUnit.testInActiveWindow(`Focus should be preserved on datagrid when rowsview repaints in ${event} event`, function(assert) { - // arrange - this.dataGrid.option({ - dataSource: [ - { id: 1, name: 'name 1' }, - { id: 2, name: 'name 2' }, - { id: 3, name: 'name 3' } - ], - keyExpr: 'id', - focusedRowEnabled: true, - [event]: (e) => { - this.dataGrid.repaint(); - } - }); - this.clock.tick(300); - - // act - $(this.dataGrid.getCellElement(0, 0)).trigger(CLICK_EVENT); - this.clock.tick(300); - - // assert - const firstRow = $(this.dataGrid.getRowElement(0)); - assert.ok(firstRow.hasClass('dx-row-focused'), 'first row is focused'); - - // act - const keyboard = keyboardMock(firstRow); - keyboard.keyDown('down'); - this.clock.tick(300); - - // assert - const secondRow = $(this.dataGrid.getRowElement(1)); - assert.ok(secondRow.hasClass('dx-row-focused'), 'second row is focused'); - }); - }); }); QUnit.module('API methods', baseModuleConfig, () => {