Skip to content

fix: finish drag when mouseup propagation stops - #1089

Open
nrps9909 wants to merge 4 commits into
react-component:masterfrom
nrps9909:codex/capture-drag-end
Open

fix: finish drag when mouseup propagation stops#1089
nrps9909 wants to merge 4 commits into
react-component:masterfrom
nrps9909:codex/capture-drag-end

Conversation

@nrps9909

@nrps9909 nrps9909 commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

Summary

  • observe drag-ending mouseup in the capture phase;
  • remove the listener with the same capture option in both normal completion and unmount cleanup;
  • add a regression for a slider inside a parent that stops mouseup propagation.

Background

Fixes #1024.

The drag listener was attached to document in the bubble phase. If any ancestor handled mouseup and called stopPropagation(), the event never reached that listener. The slider therefore remained in its dragging transaction: onChangeComplete was not called, the document mousemove listener stayed attached, and later pointer movement continued changing the value after release.

Listening for the document-level end event in the capture phase lets the slider close its own drag transaction before an ancestor intentionally stops bubbling. The listener registration and both removal paths use the same capture flag.

Exact-base reproduction: the new regression left onChangeComplete at zero calls. With this patch it receives the final dragged value, and a subsequent document mousemove produces no onChange call.

Verification

  • npm test -- --runInBand: 5 suites, 120 tests, 5 snapshots passed
  • npm run tsc: passed
  • npm run lint: passed
  • npm run compile: ESM, CJS, declarations, and CSS passed
  • git diff --check: passed

AI assistance disclosure: Codex was used to trace event propagation and cleanup paths, audit duplicate PRs, draft the focused regression, and run the verification above. The final diff and results were reviewed locally before submission.

Summary by CodeRabbit

  • Bug 修复
    • 修复父级阻止鼠标抬起事件冒泡时,Slider 拖拽无法正常结束的问题。
    • 拖拽结束后将正确停止响应后续鼠标移动,并触发完成回调。
    • 优化鼠标与触摸操作的事件处理,避免不必要地阻止浏览器默认行为。

@vercel

vercel Bot commented Aug 26, 2026

Copy link
Copy Markdown

@nrps9909 is attempting to deploy a commit to the afc163's projects Team on Vercel.

A member of the Team first needs to authorize it.

@coderabbitai

coderabbitai Bot commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 7ef5d348-c765-49bf-969a-48fa1497b5e6

📥 Commits

Reviewing files that changed from the base of the PR and between 88b4752 and a536a73.

📒 Files selected for processing (1)
  • tests/Range.test.tsx

Included review availability: Your plan provides up to 8 included reviews per hour; 6 remain after this review.


Walkthrough

本次修改使 useDrag 在捕获阶段处理文档级 mouseup 事件,并限制 preventDefault 仅用于 touchend。新增鼠标和触摸拖拽释放测试。

Changes

拖拽释放事件修复

Layer / File(s) Summary
捕获阶段拖拽释放
src/hooks/useDrag.ts
useDrag 在捕获阶段注册和移除文档级 mouseup 监听器。preventDefault 仅在事件类型为 touchend 时调用。
鼠标与触摸释放验证
tests/Slider.test.js, tests/Range.test.tsx
测试验证父级停止 mouseup 冒泡时拖拽仍能完成,并验证 onChangeComplete、最终值和释放后的行为。多个 Range 测试仅调整 JSX 格式。

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to a536a

This localized fix makes drag completion reliable when mouseup propagation is stopped and cleans up the listener consistently; no actionable merge-blocking risk remains after normal checks and review.

Sequence Diagram(s)

sequenceDiagram
  participant Slider
  participant Document
  participant Parent
  Slider->>Document: 以捕获阶段注册 mouseup
  Parent->>Document: 处理 mouseup 并停止冒泡
  Document->>Slider: 触发拖拽释放处理
  Slider->>Document: 移除 mouseup 监听器
  Slider-->>Slider: 触发 onChangeComplete
Loading

Poem

小兔检查拖拽线,
捕获阶段先接收。
父级停止事件冒泡,
Slider 仍能完成释放。
触摸结束阻止默认。
松手之后不再移动。

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 3 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed 标题准确概括了主要修复:即使 mouseup 传播被阻止,拖拽仍可完成。
Linked Issues check ✅ Passed 变更符合问题 #1024 的目标。文档级 mouseup 监听器改为捕获阶段,使 Slider 可在父级停止传播时完成拖拽。回归测试验证了 onChangeComplete 调用和释放后的停止响应。
Out of Scope Changes check ✅ Passed 未发现与问题 #1024 无关的代码变更。触摸结束测试覆盖相关的 preventDefault 分支,JSX 格式化未改变测试语义。
  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@codecov

codecov Bot commented Aug 27, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.67%. Comparing base (e43d7da) to head (a536a73).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #1089   +/-   ##
=======================================
  Coverage   98.66%   98.67%           
=======================================
  Files          15       15           
  Lines         751      752    +1     
  Branches      234      235    +1     
=======================================
+ Hits          741      742    +1     
  Misses         10       10           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Comment thread src/hooks/useDrag.ts Outdated
@nrps9909

Copy link
Copy Markdown
Contributor Author

Follow-up signed commit a536a73 covers the touchend-only preventDefault branch that Codecov identified. The regression starts a real touch drag, dispatches touchend, verifies the native default is prevented exactly once, and confirms onChangeComplete receives the unchanged range. Focused: 51 Range tests pass. Full coverage run: 123 tests and 5 snapshots pass; useDrag.ts now reports the new line covered. TypeScript and lint pass, and the touched files are Prettier-clean.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

OnMouseUp never called if <Slider> is a child of a parent component with an "OnMouseUp"

2 participants