fix(drag-n-drop): send two mousemove events to target to make Angular CDK work #34882
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mxschmitt
force-pushed
the
angular-drag-n-drop
branch
from
38e4fb8 to
b930eba
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Skn0tt
reviewed
Feb 21, 2025
This comment has been minimized.
This comment has been minimized.
mxschmitt
force-pushed
the
angular-drag-n-drop
branch
from
8e183b6 to
45451ad
Compare
This comment has been minimized.
This comment has been minimized.
mxschmitt
force-pushed
the
angular-drag-n-drop
branch
from
45451ad to
1e6410b
Compare
Skn0tt
approved these changes
Feb 21, 2025
mxschmitt
changed the title
Test results for "tests others"1 failed 21704 passed, 504 skipped Merge workflow run. |
Test results for "tests 1"6 flaky38684 passed, 794 skipped Merge workflow run. |
Test results for "tests 2"1 fatal errors, not part of any test 130 flaky263254 passed, 10048 skipped Merge workflow run. |
yury-s
reviewed
Feb 21, 2025
| @@ -1210,6 +1210,9 @@ export class Frame extends SdkObject { | |||
| // Note: do not perform locator handlers checkpoint to avoid moving the mouse in the middle of a drag operation. | |||
| dom.assertDone(await this._retryWithProgressIfNotConnected(progress, target, options.strict, false /* performActionPreChecks */, async handle => { | |||
| return handle._retryPointerAction(progress, 'move and up', false, async point => { | |||
| // NOTE: Normal browsers emit usually a lot of dragover/mousemove events during drag'n | |||
| // drop operations. We want to emit minimal (2) to make Angular CDK work. | |||
| await this._page.mouse.move(point.x, point.y); | |||
There was a problem hiding this comment.
This is fishy. Feels like we are working around Angular implementation problem. We could extend the API to allow passing steps, similar to
mouse.move({steps}) and leave the users in control of whether they want intermediate move events, WDYT?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Background:
There are two types of Drag & Drop in the web:
Native HTML5 Drag and Drop (drag-n-drop.html)
draggable="true"attributedragstart,dragover,drop)mousemoveto drag events during operationdrag-n-drop.htmlUses raw mouse events (mousedown, mousemove, mouseup)
mousemoveevents throughout the operationIn this PR I had to add the latter with an expectation that there should be two
mousemoveevents emitted.Fixes #34688