Skip to content

fix: drag & drop stability#29730

Merged
naltatis merged 2 commits intomasterfrom
fix/loadpoint-sort-drag-stability
May 8, 2026
Merged

fix: drag & drop stability#29730
naltatis merged 2 commits intomasterfrom
fix/loadpoint-sort-drag-stability

Conversation

@andig
Copy link
Copy Markdown
Member

@andig andig commented May 7, 2026

  • improve drag zones (start drag from indicator)
  • use drag cursor consistently thought the element (before icon/text split)
  • stabilize flaky drag&drop e2e test

@andig andig added the ux User experience/ interface label May 7, 2026
Copy link
Copy Markdown
Contributor

@sourcery-ai sourcery-ai Bot left a comment

Choose a reason for hiding this comment

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

Hey - I've left some high level feedback:

  • Consider also running an actionability check on the target (e.g. via await targetElement.hover({ trial: true }) or scrollIntoViewIfNeeded) before calling boundingBox() so the new hard failure on a null bbox is less likely to be triggered by transient layout/animation issues.
  • Since dragElement is now tightly coupled to the global page instance, it may be worth documenting or asserting that assumption (e.g. passing page in explicitly) to avoid confusing failures if this helper is reused in multi-page or multi-context tests.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- Consider also running an actionability check on the target (e.g. via `await targetElement.hover({ trial: true })` or `scrollIntoViewIfNeeded`) before calling `boundingBox()` so the new hard failure on a null bbox is less likely to be triggered by transient layout/animation issues.
- Since `dragElement` is now tightly coupled to the global `page` instance, it may be worth documenting or asserting that assumption (e.g. passing `page` in explicitly) to avoid confusing failures if this helper is reused in multi-page or multi-context tests.

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@andig andig changed the title test: stabilize dragElement against modal animation race chore: stabilize dragElement against modal animation race in test May 7, 2026
The reorder-loadpoints test in tests/loadpoint-sort.spec.ts intermittently
deadlocked: the drag never produced a reorder, every retry kept seeing the
original order. dragElement() read boundingBox() immediately after
expectModalVisible(), but Bootstrap modals continue to slide and fade for
~150ms after becoming "visible" to Playwright. On slower runners the bbox
landed mid-animation, mouse.down() then fired at stale coordinates and
missed the draggable source, so @formkit/drag-and-drop never registered
the gesture.

Use Locator.hover() for the source — it runs Playwright's actionability
checks, including the "stable" check that waits for animations to settle,
before positioning the cursor. Also throw on a null target bbox so future
regressions surface instead of silently no-oping.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@andig andig force-pushed the fix/loadpoint-sort-drag-stability branch from 5f8383e to b13f9c6 Compare May 7, 2026 18:28
@naltatis naltatis changed the title chore: stabilize dragElement against modal animation race in test fix: drag & drop stability May 8, 2026
@naltatis naltatis enabled auto-merge (squash) May 8, 2026 10:50
@naltatis naltatis merged commit 1e81696 into master May 8, 2026
19 checks passed
@naltatis naltatis deleted the fix/loadpoint-sort-drag-stability branch May 8, 2026 10:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ux User experience/ interface

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants