-
-
Notifications
You must be signed in to change notification settings - Fork 310
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
Fix UIElement onPointerDrag
only triggered once
#2577
Conversation
WalkthroughThis PR modifies the drag processing behavior by removing the reset of the Changes
Sequence Diagram(s)sequenceDiagram
participant P as Pointer Event
participant E as UIPointerEventEmitter
participant Eng as Engine
participant T as Test Suite
P->>E: Dispatch pointermove event
E->>Eng: Process drag (retaining draggedPath)
Eng->>T: Update event counts (e.g., dragCount)
T->>E: Assert expected event counts
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
tests/src/ui/UIEvent.test.ts (1)
132-133
: Remove extra blank lines to satisfy linter.The static analysis tool has flagged extra blank lines that should be removed.
target.dispatchEvent(generatePointerEvent("pointermove", 2, left + 5, top + 5)); engine.update(); - - expect(script1.enterCount).toBe(1); // Similar fix needed at lines 163-166Also applies to: 163-166
🧰 Tools
🪛 ESLint
[error] 132-133: Delete
⏎
(prettier/prettier)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/ui/src/input/UIPointerEventEmitter.ts
(0 hunks)tests/src/ui/UIEvent.test.ts
(4 hunks)
💤 Files with no reviewable changes (1)
- packages/ui/src/input/UIPointerEventEmitter.ts
🧰 Additional context used
🪛 ESLint
tests/src/ui/UIEvent.test.ts
[error] 132-133: Delete ⏎
(prettier/prettier)
[error] 163-164: Delete ⏎
(prettier/prettier)
[error] 165-166: Delete ⏎
(prettier/prettier)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: e2e (22.x)
- GitHub Check: codecov
🔇 Additional comments (4)
tests/src/ui/UIEvent.test.ts (4)
129-162
: Well-structured test coverage for multiple pointer drag events.This addition properly tests that the
onPointerDrag
event triggers correctly on the first pointer move. The test verifies both the drag count increment on the target element (script1
) and event bubbling to parent elements (script2
). This validates the fix for the issue where drag events were only triggered once.🧰 Tools
🪛 ESLint
[error] 132-133: Delete
⏎
(prettier/prettier)
164-197
: LGTM: Correct validation of multiple drag events.This second pointer move test case is crucial for verifying that multiple
onPointerDrag
events are triggered, rather than just the first one. The incrementeddragCount
(from 1 to 2) properly validates that the fix is working as expected.🧰 Tools
🪛 ESLint
[error] 165-166: Delete
⏎
(prettier/prettier)
206-206
: LGTM: Correct update of expected drag count.The expected drag count has been appropriately updated to reflect the behavior change fixed in this PR. Previously, the test expected
dragCount
to be reset to 0, but now it correctly expects the cumulative value of 2.
239-239
: Consistently updated drag count expectations.These updates to the expected drag count are consistent with the fix for the issue where UI element
onPointerDrag
was only triggered once. The test now correctly expects the drag count to remain at 2 even after subsequent events.Also applies to: 271-271
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2577 +/- ##
=======================================
Coverage 68.95% 68.96%
=======================================
Files 961 961
Lines 100298 100297 -1
Branches 8667 8668 +1
=======================================
+ Hits 69163 69165 +2
+ Misses 30875 30872 -3
Partials 260 260
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Please check if the PR fulfills these requirements
What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
What is the current behavior? (You can also link to an open issue here)
What is the new behavior (if this is a feature change)?
Does this PR introduce a breaking change? (What changes might users need to make in their application due to this PR?)
Other information:
Summary by CodeRabbit