-
Notifications
You must be signed in to change notification settings - Fork 8
fix: Fixes a bug when pointer d&d transition crashes when pressing Escape #368
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #368 +/- ##
==========================================
+ Coverage 94.97% 95.39% +0.42%
==========================================
Files 67 67
Lines 3263 3282 +19
Branches 699 707 +8
==========================================
+ Hits 3099 3131 +32
+ Misses 164 151 -13 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@johannes-weber I also tested the board manually by mixing different d&d interactions, including pointer-based, UAP-based, keyboard-based. That is to ensure it is not possible to disengage the global listeners w/o submitting or cancelling the respective transition. If that is the case - the moved/resized item will be stuck in mid-air, unable to commit. Please do a round of manual testing too, just to be sure. |
| // isn't set in stone. | ||
| export const CLICK_DRAG_THRESHOLD = 3; | ||
|
|
||
| // We use singleton helper to avoid creating event listeners per each board item. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great improvement! It did not make sense having the global event listeners attached on every board item!
|
I did manual testing and was not able to crash it with a mix of D&D and keyboard interactions. Tested with a trackpad, I don't have a external mouse available. |
7cfd088 to
25497ab
Compare
Description
We have two drastically different types of d&d transition: pointer-based and keyboard-based. Due to async nature of events, there was a bug where the pointer-based transition crashed when user pressed Escape. It happened because:
Besides, pressing arrow keys while the pointer-based transition was in progress led to unexpected results, too. The solution is to disallow keyboard input while pointer-based d&d is engaged by immediately cancelling the transition in this case.
I additionally refactored the code a little so that the global handlers are injected in a better predictable manner, so instead of relying on whether the handlers are present - we rely on a ref that tells us whether a transition is engaged.
Rel: AWSUI-61155, similar to: #367
The change also fixes AWSUI-61157. Before, when the right-click (reproducible with a mouse, but not with trackpad) was made during a pointer-based d&d operation, the transition was stuck because the related pointer-up was removed. This is no longer the case.
Before:
Screen.Recording.2025-08-26.at.13.57.00.mov
After:
Screen.Recording.2025-08-26.at.13.57.40.mov
How has this been tested?
Review checklist
The following items are to be evaluated by the author(s) and the reviewer(s).
Correctness
CONTRIBUTING.md.CONTRIBUTING.md.Security
checkSafeUrlfunction.Testing
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.