Skip to content

fix: stop sticky group header jitter while scrolling#49

Open
aojunhao123 wants to merge 2 commits into
react-component:masterfrom
aojunhao123:fix/sticky-group-header-jitter
Open

fix: stop sticky group header jitter while scrolling#49
aojunhao123 wants to merge 2 commits into
react-component:masterfrom
aojunhao123:fix/sticky-group-header-jitter

Conversation

@aojunhao123

@aojunhao123 aojunhao123 commented Jun 27, 2026

Copy link
Copy Markdown
Collaborator

Summary

Fixes a subtle jitter of the sticky group header when scrolling up and down within a group (reproducible in the group demo).

Root cause

The sticky header overlay is rendered through rc-virtual-list's extraRender, which places it inside the natively-scrolled holder (the translateY(offsetY) container). To keep it visually pinned, listy recomputed its top as scrollTop - offsetY on every React render.

But the two positioning channels are not in sync:

  • The list content scrolls via the holder's native scrollTop — applied synchronously, takes effect the same frame.
  • The overlay's compensating top is applied through a React render/commit, which can paint one frame later.

On frames where the native scroll has already advanced but the re-render carrying the new top hasn't committed yet, the header is left under-compensated by exactly one scroll step and then snaps back the next frame. The list items, driven directly by native scroll, move correctly — so only the header appears to shake.

Per-frame measurement before the fix (fresh downward scroll) caught transient frames where the header's on-screen position was a full scroll step off its intended styleTop (e.g. rendered at -16px while it should be 0).

Fix

Render the overlay into the outer, non-scrolled list container (listRef.nativeElement) via Portal, and make top viewport-relative:

const top = nextHeader
  ? Math.min(0, getSize(nextHeader.groupKey).top - headerHeight - scrollTop)
  : 0;
  • Within a group the value is a static 0 — there is no per-frame scroll compensation, so nothing can lag → no jitter.
  • The next group header still pushes the current one up (top goes negative) near a boundary, so the push animation is preserved.
  • Because the overlay now lives in the non-clipping outer container, a pushed-up header could bleed above the list over surrounding content; overflow: hidden on the list root clips it back to the list viewport (the natural "slide out the top" behavior).

Verification

Per-frame sampling in a foreground browser at full frame rate:

  • Within-group aggressive up/down (160 frames): 0 jitter frames, 0 header/scroll desync.
  • After the fix the header's on-screen position equals its styleTop on every frame — the desync channel is eliminated by construction.
  • Boundary push: monotonic slide-up, clipped at the list top, no overflow over surrounding controls.
  • All unit tests pass (28). useStickyGroupHeader tests updated for the new viewport-relative contract.

Before / After

Before (header jitters while scrolling within a group)

Clipboard-20260627-185526-935

After (header stays pinned; push animation clipped to the list)

Clipboard-20260627-174305-770

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes
    • 优化虚拟列表中分组表头的固定展示位置,滚动时定位更准确。
    • 修复表头在部分场景下的内容溢出问题,避免显示异常。
    • 改进虚拟列表启用/禁用时的分组表头行为,确保显示结果一致。

…tter

The sticky group header overlay was rendered through rc-virtual-list's
extraRender, which places it inside the natively-scrolled holder. To stay
pinned it recomputed top = scrollTop - offsetY on every React render. The
content scrolls via the holder's native scrollTop (synchronous) while the
overlay's compensating top lands through a React commit that can paint one
frame later, so on transient frames the header lags the scroll by one step
and snaps back - the jitter.

Render the overlay into the outer, non-scrolled container via Portal and make
top viewport-relative (0 within a group, negative only when the next header
pushes it up). Within a group there is no per-frame compensation, so nothing
can lag. overflow: hidden on the list root clips the pushed-up header to the
viewport so it cannot bleed over content above the list.
@coderabbitai

coderabbitai Bot commented Jun 27, 2026

Copy link
Copy Markdown

Review Change Stack

Walkthrough

useStickyGroupHeader 新增 listRef 参数,将粘性分组表头的渲染方式从直接返回元素改为通过 Portal 挂载到虚拟列表的 nativeElement 容器中,同时将 top 偏移计算从基于 offsetY 的旧公式改为基于 getSize(...).top - headerHeight - scrollTop 的新公式,并为列表容器补充 overflow: hidden 样式,同步更新相关测试。

Changes

粘性表头 Portal 渲染重构

Layer / File(s) Summary
StickyHeaderParams 接口与依赖引入
src/VirtualList/useStickyGroupHeader.tsx, src/VirtualList/index.tsx
新增 PortalListRef 导入;StickyHeaderParams 增加 listRef: React.RefObject<RcVirtualListRef | null> 字段;VirtualList 调用 useStickyGroupHeader 时传入 listRef
extraRender 核心逻辑与 Portal 渲染
src/VirtualList/useStickyGroupHeader.tsx
extraRender 改用 scrollTop 入参,通过 listRef.current?.nativeElement 获取渲染容器;top 偏移改为 Math.min(0, nextHeader.getSize().top - headerHeight - scrollTop) 新公式;渲染改为 Portal 包裹 GroupHeader 挂载到该容器。
列表容器 overflow 样式
assets/index.less
.@{listy-prefix-cls} 新增 overflow: hidden,配合 Portal 挂载到容器内的裁剪行为。
测试用例更新
tests/hooks.test.tsx
新增 createListRef 工具函数;各用例改为显式创建并挂载 DOM 容器传入 listRef;断言改为从容器查询固定头节点;top 样式期望值按新公式(多数为 0px)更新;每个用例结束后清理容器。

估算代码审查工作量

🎯 3 (Moderate) | ⏱️ ~20 minutes

可能相关的 PR

  • react-component/listy#46: 该 PR 直接涉及 useStickyGroupHeader 的粘性表头偏移处理逻辑,与本次重构存在强关联。

Poem

🐰 小兔子跳进虚拟列表,
Portal 大门悄悄打开,
表头粘在容器里不乱跑,
overflow: hidden 把边界守牢,
scrollTop 一算,0px 对齐好~
✨ 咚咚咚,重构完成啦!

🚥 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%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
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.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
✨ Finishing Touches
🧪 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 Jun 27, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (077d6d8) to head (81d0c43).

Additional details and impacted files
@@            Coverage Diff            @@
##            master       #49   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            8         8           
  Lines          207       209    +2     
  Branches        61        63    +2     
=========================================
+ Hits           207       209    +2     

☔ 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.

@aojunhao123 aojunhao123 requested review from Copilot and zombieJ June 27, 2026 16:48

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request refactors the sticky group header rendering in the virtual list component to use a Portal targeted at the list container, simplifying the top offset calculation. The tests have been updated to support this portal-based rendering. The review feedback suggests several improvements: defensively handling potential undefined returns from getSize to prevent runtime crashes, wrapping the portaled header in a localized clipping container instead of applying overflow: hidden globally to avoid clipping list item children, and explicitly unmounting components in tests before removing custom DOM containers to prevent memory leaks.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread src/VirtualList/useStickyGroupHeader.tsx
Comment thread src/VirtualList/useStickyGroupHeader.tsx
Comment thread assets/index.less Outdated
Comment thread tests/hooks.test.tsx Outdated

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR eliminates sticky grouped header “jitter” during scroll by moving the sticky header overlay out of the natively scrolled container and into the outer list container via a portal, so the header can stay pinned without per-frame scroll compensation.

Changes:

  • Render the sticky group header overlay using @rc-component/portal into the list’s outer container and compute top viewport-relatively.
  • Extend useStickyGroupHeader to accept the virtual list ref (listRef) so it can target the correct portal container.
  • Add overflow: hidden on the list root to clip the pushed-up sticky header at the top boundary, and update unit tests to match the new positioning contract.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
tests/hooks.test.tsx Updates useStickyGroupHeader tests to validate the new portal-based rendering and the new top contract.
src/VirtualList/useStickyGroupHeader.tsx Ports the sticky header overlay into the outer container and switches to viewport-relative top math to avoid scroll/render desync.
src/VirtualList/index.tsx Passes the virtual list ref into useStickyGroupHeader so it can locate the portal container.
assets/index.less Clips pushed headers within the list viewport by applying overflow: hidden on the list root.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread tests/hooks.test.tsx
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.

2 participants