DataGrid - Fix the browser scrolling back to the grid when data is updated (T1310557)#33168
DataGrid - Fix the browser scrolling back to the grid when data is updated (T1310557)#33168markallenramirez wants to merge 1 commit intoDevExpress:26_1from
Conversation
There was a problem hiding this comment.
Pull request overview
Fixes DataGrid keyboard-navigation focus restoration so that updating data doesn’t cause the browser to scroll the page back to the grid (by using native focus({ preventScroll }) during render completion).
Changes:
- Add a
preventScrollflag to_updateFocusand_focusand propagate it fromrenderCompleted. - Replace synthetic
eventsEngine.trigger(..., 'focus')with native DOMfocus({ preventScroll })for the focused cell/row.
| $focusedElementInsideCell, | ||
| ); | ||
| return; | ||
| } | ||
| !isFocusedElementDefined && this._focus($cell, false, skipFocusEvent); | ||
| } else if ( | ||
| !isFocusedElementDefined | ||
| && (this._isNeedFocus || this._isHiddenFocus) | ||
| ) { | ||
| this._focus($cell, this._isHiddenFocus, skipFocusEvent); | ||
| this._focus($cell, this._isHiddenFocus, skipFocusEvent, preventScroll); |
There was a problem hiding this comment.
_updateFocus(..., preventScroll) only forwards preventScroll to _focus(...) in the fallback branch. In the (isRenderView || !isCommandCell) && this._editorFactory.focus() branch, _focus($cell, false, skipFocusEvent) will still call focus({ preventScroll: false }), which can reintroduce the page jump this change is trying to prevent. Consider passing preventScroll through in this branch as well (or routing both branches through a single _focus call that receives the flag).
| public _focus($cell, disableFocus?, skipFocusEvent?, preventScroll = false) { | ||
| const $row = $cell && !$cell.hasClass(ROW_CLASS) | ||
| ? $cell.closest(`.${ROW_CLASS}`) |
There was a problem hiding this comment.
_focus now accepts preventScroll, but there are derived controllers overriding _focus (e.g. keyboard_navigation/scrollable_a11y.ts) that currently only accept 3 params and call super._focus($cell, disableFocus, skipFocusEvent). When preventScroll is passed from this controller, those overrides will ignore it and the scroll-prevention behavior will not apply in those modes. Please update the overrides to accept/forward the new parameter.
| if (needUpdateFocus) { | ||
| const isScrollEvent = !!e?.event?.type; | ||
| const skipFocusEvent = e?.virtualColumnsScrolling && isScrollEvent; | ||
| this._updateFocus(true, skipFocusEvent); | ||
| const preventScroll = !isFullUpdate; | ||
| this._updateFocus(true, skipFocusEvent, preventScroll); |
There was a problem hiding this comment.
This change introduces new behavior (calling element.focus({ preventScroll }) during renderCompleted when updating focus). There are existing DataGrid keyboard navigation QUnit tests (e.g. testing/tests/DevExpress.ui.widgets.dataGrid/keyboardNavigation.rowsView.tests.js) that cover renderCompleted focus restoration; consider adding a regression test that verifies preventScroll is propagated (e.g. by stubbing HTMLElement.prototype.focus and asserting the preventScroll option for non-refresh change types).
No description provided.