Skip to content

fix(grid): do not group by a column when its move is cancelled with Escape - #17781

Open
ivanvpetrov wants to merge 2 commits into
ipetrov/drag-cancel-upon-escapefrom
ipetrov/fix-17745-grid-group-drag
Open

ivanvpetrov wants to merge 2 commits into
ipetrov/drag-cancel-upon-escapefrom
ipetrov/fix-17745-grid-group-drag

Conversation

@ivanvpetrov

Copy link
Copy Markdown
Contributor

Closes #17745

Description

Cancelling a column move, with Escape or by setting cancel in a columnMoving handler, now ends the drag through the new IgxDragDirective cancel API (cancelDrag() / cancelDragInternal()). No drop is sent anymore.

  • IgxColumnMovingDragDirective uses cancelDragInternal(event) on Escape and cancelDrag() when columnMoving is cancelled. The drop area under the pointer gets only a leave event, so its hover state is cleared. The existing cancelDragInternal override resets the moving state (cms.column, the Escape subscription).
  • Removed the internal cancelMove() method and the IgxColumnMovingService.cancelDrop flag (both @hidden @internal), along with its check in IgxColumnMovingDropDirective.onDragDrop.

Motivation / Context

cancelMove() set cancelDrop = true and then called onPointerUp(), which still sent a drop to the drop area under the pointer. Only the column header drop directive checked the flag:

  • Pressing Escape while dragging a column over the group-by area grouped the grid by that column.
  • The flag was reset only by a header drop. After a move was cancelled anywhere else, the next column drop on a header was ignored.

The tree grid's group-by area uses the same drop handler and had the same problem; it is fixed and covered by a test as well.

This builds on the drag cancel API from #17744 (the fix was postponed until that work was in, see the issue comment).

Type of Change (check all that apply):

  • Bug fix
  • New functionality
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Refactoring (no functional changes)
  • Documentation
  • Demos
  • CI/CD
  • Tests
  • Changelog
  • Skills/Agents

Component(s) / Area(s) Affected:

Grid, Tree Grid: column moving and the group-by area (IgxColumnMovingDragDirective, IgxColumnMovingDropDirective, IgxColumnMovingService)

How Has This Been Tested?

  • Unit tests
  • Manual testing
  • Automated e2e tests

New specs, each failing before the fix:

  • grid.groupby.spec.ts: Escape while dragging a column over the group-by area does not group by it, clears the drop area hover state, and a later pointer release does not group either.
  • column-moving.spec.ts: after a column move cancelled with Escape outside the headers, the next column move is dropped and reorders the columns.
  • tree-grid-grouping.spec.ts: Escape while dragging a column over the tree grid group-by area does not group by it.

Also ran column-moving, grid.groupby, tree-grid-grouping, hierarchical-grid.integration, tree-grid-integration, grid-keyBoardNav-headers, grid.multi-row-layout, column and row-drag.directive specs (all passing), and npm run lint:lib (no errors).

Note: the new specs dispatch Escape with bubbles: true, because column moving listens for keydown on window. A real key press bubbles there, but a KeyboardEvent created without bubbles does not.

Test Configuration:

  • Angular version: 22.2.0
  • Browser(s): Chrome Headless 155
  • OS: Windows 11

Screenshots / Recordings

N/A

Checklist:

  • All relevant tags have been applied to this PR
  • This PR includes unit tests covering all the new code (test guidelines)
  • This PR includes API docs for newly added methods/properties (api docs guidelines)
  • This PR includes feature/README.MD updates for the feature docs
  • This PR includes general feature table updates in the root README.MD
  • This PR includes CHANGELOG.MD updates for newly added functionality
  • This PR contains breaking changes
  • This PR includes ng update migrations for the breaking changes (migrations guidelines)
  • This PR includes behavioral changes and the feature specification has been updated with them
  • Accessibility (ARIA, keyboard navigation, focus management) has been verified

@ivanvpetrov
ivanvpetrov changed the base branch from master to ipetrov/drag-cancel-upon-escape October 16, 2026 08:02
@Marina-L-Stoyanova
Marina-L-Stoyanova requested a balanced review from Copilot October 16, 2026 08:03

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

Cancellation before movement exceeds the drag tolerance leaves the interaction active, allowing the same gesture to drag and drop later.

1 open finding
What changed in this PR

Updates grid column moving to use the drag-cancellation API, preventing cancelled moves from grouping columns or blocking later drops.

Changes:

  • Replaces simulated drops with explicit cancellation.
  • Removes the obsolete cancellation flag.
  • Adds grid and tree-grid regression tests and a changelog entry.
File Description
projects/​igniteui-angular/​grids/​tree-grid/​src/​tree-grid-grouping.spec.ts Tests Escape cancellation over tree-grid grouping.
projects/​igniteui-angular/​grids/​grid/​src/​grid.groupby.spec.ts Tests cancelled grouping and hover cleanup.
projects/​igniteui-angular/​grids/​grid/​src/​column-moving.spec.ts Tests reordering after cancellation.
projects/​igniteui-angular/​grids/​core/​src/​moving/​moving.service.ts Removes the cancellation flag.
projects/​igniteui-angular/​grids/​core/​src/​moving/​moving.drop.directive.ts Removes flag-based drop suppression.
projects/​igniteui-angular/​grids/​core/​src/​moving/​moving.drag.directive.ts Uses the drag-cancellation API.
CHANGELOG.md Documents the cancellation fixes.

🧠 Review effort: Balanced


💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.

Comment thread projects/igniteui-angular/grids/core/src/moving/moving.drag.directive.ts Outdated
@Marina-L-Stoyanova
Marina-L-Stoyanova requested a balanced review from Copilot October 16, 2026 08:42
@ivanvpetrov
ivanvpetrov added this pull request to stack #17782 October 16, 2026 08:42

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approval recommended

Static review identified no blocking issues, and focused regression tests cover the affected cancellation paths; tests were not executed during this review.

1 open finding

🧠 Review effort: Balanced

@ivanvpetrov
ivanvpetrov marked this pull request as ready for review October 16, 2026 09:06
Cancel the column move with cancelDrag() on Escape or a cancelled
columnMoving event instead of dispatching a drop guarded by a flag.
The group-by area no longer groups by the cancelled column, and a
cancelled move no longer blocks the next header drop.
Cancelling a column move before the pointer passes the drag tolerance
left the move pending, so the same gesture could still drag and drop
the column. End the pending interaction as a release does, and cancel
an active drag without dropping.
@kdinev
kdinev force-pushed the ipetrov/fix-17745-grid-group-drag branch from 42d5c51 to 5e66204 Compare October 19, 2026 07:59

This branch has not been deployed

No deployments
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.

Grid: pressing Escape while dragging a column over the group-by area groups by that column

2 participants