DataGrid - Native horizontal scrollbar is temporarily shown when clicking cells in batch/cell mode if focused row and column fixing features are enabled (T1310336)#33374
Conversation
There was a problem hiding this comment.
Pull request overview
Fixes a DataGrid UI regression where a native horizontal scrollbar briefly appears when interacting with cells in batch/cell editing mode while focused row and column fixing (sticky columns) are enabled.
Changes:
- Adjusts focus overlay sizing under sticky columns to avoid the 1px width expansion for group rows.
- Hides the focus overlay when the focused element is a data row (
tr.dx-data-row) to prevent transient overflow that can trigger a native horizontal scrollbar.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| packages/devextreme/js/__internal/grids/grid_core/sticky_columns/m_sticky_columns.ts | Tweaks focus overlay width calculation in sticky columns mode to avoid an extra pixel for group rows. |
| packages/devextreme/js/__internal/grids/grid_core/editor_factory/m_editor_factory.ts | Updates focus overlay rendering to keep it hidden when focus is on a data row element, reducing overflow/scrollbar flicker risk. |
16565fa to
4a689ee
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
packages/devextreme/js/__internal/grids/grid_core/editor_factory/m_editor_factory.ts:250
- When
isDataRowElementis true, the focus overlay is being hidden, but the method still appends the overlay to the DOM, resizes it, and runs positioning calculations. This extra work is unnecessary and can cause avoidable layout/paint work; consider early-returning (or skipping size/position setup) when the overlay should remain hidden for data rows, similar to the existingisHideBorderbranch.
const align = browser.mozilla ? 'right bottom' : 'left top';
const isDataRowElement = isDataRow($element);
const isFocusedCellInvalid = $element.hasClass(this.addWidgetPrefix(CELL_INVALID_CLASS));
const isFocusedCellModified = $element.hasClass(CELL_MODIFIED_CLASS) && !isFocusedCellInvalid;
const $content = this.getFocusOverlayContainer($element);
const focusOverlaySize = this.getFocusOverlaySize($element);
this._$focusOverlay
.toggleClass(DX_HIDDEN, isDataRowElement)
.toggleClass(FOCUSED_CELL_INVALID_CLASS, isFocusedCellInvalid)
.toggleClass(FOCUSED_CELL_MODIFIED_CLASS, isFocusedCellModified)
.appendTo($content);
setOuterHeight(this._$focusOverlay, focusOverlaySize.height);
setOuterWidth(this._$focusOverlay, focusOverlaySize.width);
| @@ -233,13 +234,14 @@ export class EditorFactory extends ViewControllerWithMixin { | |||
| } else if ($element.length) { | |||
There was a problem hiding this comment.
Can’t the isDataRow check be moved into this condition?
| $element, | ||
| this.addWidgetPrefix.bind(this), | ||
| ); | ||
| const isGroupRow = isGroupRowElement($element); |
There was a problem hiding this comment.
You also need to add a check for the group footer row.
For master/adaptive detail rows, I’ve checked — everything seems fine there. Please make sure to verify this as well.
| const firstGroupRow = dataGrid.getGroupRow(0); | ||
| const firstDataRow = dataGrid.getDataRow(1); | ||
| const secondDataCell = firstDataRow.getDataCell(2); | ||
| const hasHorizontalScroll = async () => { |
There was a problem hiding this comment.
Minor comment: Could we move this into the DataGrid helpers? For example, into domUtils?
No description provided.