Skip to content

DataGrid - decompose _getNextCell method in keyboard navigation controller#32755

Open
dmirgaev wants to merge 3 commits intoDevExpress:26_1from
dmirgaev:26_1__keyboard_nav_tech_debt
Open

DataGrid - decompose _getNextCell method in keyboard navigation controller#32755
dmirgaev wants to merge 3 commits intoDevExpress:26_1from
dmirgaev:26_1__keyboard_nav_tech_debt

Conversation

@dmirgaev
Copy link
Contributor

@dmirgaev dmirgaev commented Mar 3, 2026

No description provided.

@dmirgaev dmirgaev self-assigned this Mar 3, 2026
@dmirgaev dmirgaev requested a review from a team as a code owner March 3, 2026 08:08
Copilot AI review requested due to automatic review settings March 3, 2026 08:08
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR refactors DataGrid keyboard navigation by decomposing the _getNextCell logic into smaller helper methods and tightening TypeScript typings for navigation-related parameters.

Changes:

  • Introduced extended keyboard navigation direction and element type aliases in types.ts.
  • Refactored _getNextCell into _adjustCellOnVerticalNav, _resolveInvalidCell, and _getNextCellSkippingHiddenRow helpers.
  • Minor TypeScript type style cleanup in keyboard navigation integration tests.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
packages/devextreme/js/__internal/grids/grid_core/keyboard_navigation/types.ts Adds shared type aliases for extended navigation directions and element types.
packages/devextreme/js/__internal/grids/grid_core/keyboard_navigation/m_keyboard_navigation.ts Decomposes _getNextCell and adds stricter typing around navigation params/return values.
packages/devextreme/js/__internal/grids/grid_core/keyboard_navigation/tests/keyboard_navigation.integration.test.ts Adjusts generic type syntax style in it.each cases (no behavior change).
Comments suppressed due to low confidence (1)

packages/devextreme/js/__internal/grids/grid_core/keyboard_navigation/m_keyboard_navigation.ts:1705

  • $cell is now typed as dxElementWrapper | null, but it’s used immediately via $cell.parent() without a null-guard. In the base controller _getFocusedCell() always returns a wrapper ($(...)), so the nullable type (and the subsequent $cell && check) is misleading. Either keep $cell as dxElementWrapper (preferred) or add an early return/guard before any $cell.* usage if null is actually possible.
      let $cell: dxElementWrapper | null = this._getFocusedCell();
      const isEditing = this._editingController.isEditing();

      if (!this.getMasterDetailCell($cell) || this._isRowEditMode()) {
        if (this._hasSkipRow($cell.parent())) {

@dmirgaev dmirgaev force-pushed the 26_1__keyboard_nav_tech_debt branch from bcc8492 to bedcddb Compare March 3, 2026 15:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants