feat: support classNames/styles semantic DOM API#51
Conversation
Add root/item/groupHeader semantic slots via `classNames` and `styles` props, wired through both RawList and VirtualList (including the sticky group header) so the two render modes stay at parity.
|
@aojunhao123 is attempting to deploy a commit to the React Component Team on Vercel. A member of the Team first needs to authorize it. |
概览为 变更语义化样式支持
估算代码审查工作量🎯 3 (Moderate) | ⏱️ ~25 分钟 可能相关的 PR
建议审查者
小诗
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces support for semantic styling in the Listy component by adding classNames and styles props, allowing customization of the root, item, and groupHeader elements across both native and virtual list implementations. It also includes comprehensive test coverage for these changes. Regarding the feedback, in useStickyGroupHeader.tsx, the dynamically calculated top style property should be placed after spreading headerStyle to ensure it is not overridden by custom styles, which would break the sticky header positioning.
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.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #51 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 8 8
Lines 211 211
Branches 63 62 -1
=========================================
Hits 211 211 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
tests/semantic.test.tsx (1)
186-215: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win改成走真实的
Listy/VirtualList渲染路径来测 sticky clone。这里直接调用
useStickyGroupHeader()并手动执行extraRender，绕过了src/VirtualList/index.tsx把classNames.groupHeader/styles.groupHeader透传给 hook 的那层集成。即使实际组件漏传了headerClassName或headerStyle，这个用例也还是会通过，覆盖不到这次 PR 最关键的接线。建议改成渲染开启 sticky 的<Listy />，再断言 portal 里的固定表头。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/semantic.test.tsx` around lines 186 - 215, The current sticky clone test bypasses the real Listy/VirtualList integration by calling useStickyGroupHeader directly, so it won’t catch missing headerClassName/headerStyle wiring from VirtualList. Update this test to render the actual Listy or VirtualList path with sticky enabled, using the real props flow through VirtualList/index.tsx and useStickyGroupHeader, then assert the pinned clone in the portal contains the expected groupHeader class and style.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/RawList/index.tsx`:
- Around line 62-69: `RawList` 的 `style` 合并顺序有问题，`styles?.item` 可能覆盖 sticky
分组下的内部 `scrollMarginTop`，导致滚动定位失效；请调整 `RawList/index.tsx` 中该对象展开顺序，让内部计算出的
`scrollMarginTop` 在 `sticky && groupKey !== undefined` 时保持最高优先级，并确保
`styles?.item` 仍可提供其他样式但不能覆盖这个偏移量。
In `@src/VirtualList/useStickyGroupHeader.tsx`:
- Around line 110-111: The cloned sticky group header is letting external
headerStyle.top override the internally computed sticky offset, which breaks the
push-off logic. Update useStickyGroupHeader so the calculated top value from the
hook stays authoritative in the header clone, and only merge the rest of
headerStyle’s custom styles without allowing its top to replace the internal
positioning.
---
Nitpick comments:
In `@tests/semantic.test.tsx`:
- Around line 186-215: The current sticky clone test bypasses the real
Listy/VirtualList integration by calling useStickyGroupHeader directly, so it
won’t catch missing headerClassName/headerStyle wiring from VirtualList. Update
this test to render the actual Listy or VirtualList path with sticky enabled,
using the real props flow through VirtualList/index.tsx and
useStickyGroupHeader, then assert the pinned clone in the portal contains the
expected groupHeader class and style.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 75b213c4-7fc8-463c-8589-f8ad4b0a0d3a
📒 Files selected for processing (9)
.gitignorejest.config.jssrc/GroupHeader.tsxsrc/List.tsxsrc/RawList/index.tsxsrc/VirtualList/index.tsxsrc/VirtualList/useStickyGroupHeader.tsxsrc/index.tstests/semantic.test.tsx
15bdd5b to
4f48026
Compare
Demonstrate the classNames/styles slots (root/item/groupHeader) with a virtual on/off toggle showing the same slots apply in both render modes.
In sticky grouped mode RawList sets scroll-margin-top on items so
scrollTo({ key }) lands them clear of the position:sticky group header
(the header height is measured live into the --*-item-scroll-margin-top
var). It's internal scrollTo machinery, not a style knob, so it must win
over styles.item — otherwise a user-supplied scrollMarginTop breaks the
scroll landing and the item ends up hidden behind the header.
4f48026 to
7fce56b
Compare
styles.item.scrollMarginTop must not override the sticky scrollTo offset, and styles.groupHeader.top must not override the computed sticky-clone top. The existing slot tests only asserted coexistence, so they stayed green even with the override bug present; these two assert the internal value wins.
2 participants
Summary
classNamesandstylesprops withroot/item/groupHeaderslots (ListySemanticName,ListyClassNames,ListyStylesexported from the entry).RawListandVirtualList, including the sticky group header inuseStickyGroupHeader— so the two modes stay at parity.tests/semantic.test.tsxcovering slot application across both modes..claudeworktrees in.gitignoreandjest.config.js. jest-haste-map crawls the filesystem (not.gitignore), so a nested.claudeworktree copy collided on the duplicate package name and crashed the whole suite withdupMap.get is not a function.Test plan
pnpm test— 4 suites / 38 tests passclassNames/stylesto<Listy>in virtual mode and confirm root/item/groupHeader (incl. sticky header) pick up the class/style🤖 Generated with Claude Code
Summary by CodeRabbit