Skip to content

fix: sync controlled scrollbar visibility - #382

Open
nrps9909 wants to merge 1 commit into
react-component:masterfrom
nrps9909:codex/sync-scrollbar-visibility
Open

fix: sync controlled scrollbar visibility#382
nrps9909 wants to merge 1 commit into
react-component:masterfrom
nrps9909:codex/sync-scrollbar-visibility

Conversation

@nrps9909

@nrps9909 nrps9909 commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

Summary

  • synchronize the internal visibility state when boolean showScrollBar changes
  • retain the existing delayed-hide behavior when the prop changes back to optional
  • use the stable event helper for the timer callback and clean up replaced timers

Problem

showScrollBar currently initializes visible, but later boolean prop changes never update that state. A list rendered with showScrollBar={false} therefore stays hidden after changing to true, and the reverse transition can stay visible.

Testing

  • the new exact-base regression failed on a573364: after false -> true, the vertical scrollbar still lacked its visible class
  • the regression now covers false -> true -> false -> optional, including the existing three-second optional hide
  • npm test -- tests/scroll.test.js --runInBand (38 passed)
  • npm test -- --runInBand (8 suites, 281 tests passed)
  • npm run tsc
  • npm run lint (0 errors; 13 pre-existing hook warnings, down from 14 because the timer callback is now stable)
  • npm run compile (ESM, CJS, and declarations)
  • Prettier and git diff --check

Overlap audit

PR #381 touches the same source and test files, but only the non-primary mouse-button handlers and their separate regression. The visibility changes here are in independent hunks. The older #158 visibility proposal is a conflicting 2021 branch for adding an always-visible API; the current public showScrollBar API already exists and this PR fixes its controlled rerender behavior. Broad conflicting PR #199 does not cover prop synchronization.

AI assistance disclosure: Codex was used to trace the controlled-prop lifecycle, implement and validate the exact-base regression and timer behavior, audit open PR overlap, and draft this PR. All stated behavior and test results were verified locally.

Summary by CodeRabbit

  • Bug 修复

    • 修复滚动条显示状态在不同配置间切换时未及时同步的问题。
    • 优化滚动条自动隐藏逻辑,确保可选模式下按预期延迟隐藏。
  • 测试

    • 新增滚动条可见性切换及自动隐藏场景的验证。
@vercel

vercel Bot commented Aug 27, 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 27, 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: 07a0cb64-7354-4696-9b05-4f9ab2b225f6

📥 Commits

Reviewing files that changed from the base of the PR and between a573364 and 407ea2c.

📒 Files selected for processing (2)
  • src/ScrollBar.tsx
  • tests/scroll.test.js

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


Walkthrough

本次变更更新 ScrollBar 的可见性同步逻辑,并新增测试覆盖 showScrollBar 的状态切换和自动隐藏行为。

Changes

滚动条可见性

Layer / File(s) Summary
可见性控制逻辑
src/ScrollBar.tsx
showScrollBar 为布尔值时直接设置滚动条可见性。其他情况继续使用 3000ms 延迟隐藏逻辑。effect 依赖项同步更新。
可见性状态测试
tests/scroll.test.js
测试覆盖 showScrollBarfalse 切换为 truefalse'optional',并验证延迟隐藏。

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

Merge Risk: ⚪ Minimal · up to 407ea

This change synchronizes scrollbar visibility when the controlling prop changes while preserving delayed hiding for optional mode. The behavior is covered by regression tests and no actionable merge-blocking risk remains after normal checks and review.

Suggested reviewers: zombiej, afc163, aojunhao123

Poem

小兔轻点滚动条,
真与假状态排成行。
optional 等三千毫秒,
月光下悄悄隐藏忙。
测试挥挥小手帕,
可见性同步闪亮亮。

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed 标题准确概括了本次变更的主要目的,即修复受控滚动条可见性的同步问题。标题简洁且具体。
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 2…
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.
Full details: Docstring Coverage

Explanation

No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 2 files.

✨ 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

1 participant