Skip to content

fix: use latest scroll close options - #647

Open
nrps9909 wants to merge 1 commit into
react-component:masterfrom
nrps9909:codex/latest-scroll-options
Open

fix: use latest scroll close options#647
nrps9909 wants to merge 1 commit into
react-component:masterfrom
nrps9909:codex/latest-scroll-options

Conversation

@nrps9909

@nrps9909 nrps9909 commented Aug 28, 2026

Copy link
Copy Markdown

Summary

  • make the scroll-close callback read the latest trigger props
  • preserve the existing long-lived scroll listener without rebinding it on every render
  • cover an alignPoint prop update while the popup remains open

Problem

useWatch installs its scroll listener while a popup is open. The onScroll callback passed to it was recreated on each render, but the effect intentionally does not depend on callbacks. If alignPoint changed from false to true while the popup stayed open, the installed listener kept the old closure and did not close a click-triggered popup on scroll.

Solution

Wrap the callback with the project's useEvent helper. The listener keeps a stable function identity while its implementation reads the current alignPoint, clickToHide, and trigger logic.

Tests

  • Before the source change, the new rerender regression test failed because the popup remained visible after scrolling
  • rc-test --runInBand — 17 suites passed, 134 tests passed, 1 skipped
  • tsc --noEmit — passed
  • ESLint — 0 errors; 11 pre-existing hook dependency warnings
  • father build plus Less compile — ESM, CJS, declarations, and CSS passed
  • changed-file Prettier and git diff --check — passed
Language Changelog
English Use the latest point-alignment settings when handling popup scroll events.
Chinese 修復彈出層開啟期間更新點對齊設定後,捲動事件仍使用舊設定的問題。

AI assistance disclosure: Codex was used to trace the listener lifecycle, audit open PRs and issues for overlap, implement the focused useEvent fix, and run the validation commands. The failing baseline and passing result were verified against upstream master.

Summary by CodeRabbit

  • Bug 修复

    • 修复上下文菜单滚动时未使用最新对齐配置的问题。
    • 当启用相应对齐设置并发生滚动时,弹层会按预期隐藏。
  • 测试

    • 新增测试,验证动态更新对齐配置后滚动关闭弹层的行为。

@vercel

vercel Bot commented Aug 28, 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 28, 2026

Copy link
Copy Markdown

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: a51e8c7a-806b-4afc-adc4-8dc0010d7868

📥 Commits

Reviewing files that changed from the base of the PR and between 3ff7d68 and 07fc30c.

📒 Files selected for processing (2)
  • src/index.tsx
  • tests/point.test.jsx

Included review availability: Your plan provides up to 2 included reviews per hour; 1 remains after this review.


Walkthrough

onScroll 现在通过 useEvent 保持函数引用稳定。新增测试验证 alignPoint 更新后,滚动事件会关闭弹层。

Changes

滚动关闭行为

Layer / File(s) Summary
稳定的滚动处理与回归测试
src/index.tsx, tests/point.test.jsx
onScroll 使用 useEvent 创建稳定引用,同时保留原有关闭条件。新增测试验证重新渲染启用 alignPoint 后,滚动事件会隐藏弹层。

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

Merge Risk: ⚪ Minimal · up to 07fc3

The change keeps the scroll listener stable while applying the latest popup trigger settings, with a regression test covering updates while the popup remains open. No actionable merge-blocking risk remains after normal checks and review.

Suggested reviewers: zombiej

Poem

小兔握住稳定的事件线
滚动来到,弹层轻轻隐
新的 alignPoint 已被看见
测试跳过,确认行为不变
耳朵一抖,代码更清晰

🚥 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 1 functions across 2 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 标题准确概括了主要变更:滚动关闭逻辑使用最新的配置选项。标题简洁、明确,并与代码修改和回归测试一致。
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
  • 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.

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.

1 participant