Skip to content

[codex] Support semantic list styles#389

Merged
zombieJ merged 1 commit intomasterfrom
codex/support-list-semantic-styles
May 6, 2026
Merged

[codex] Support semantic list styles#389
zombieJ merged 1 commit intomasterfrom
codex/support-list-semantic-styles

Conversation

@zombieJ
Copy link
Copy Markdown
Member

@zombieJ zombieJ commented May 6, 2026

Summary

  • add list to notification semantic classNames and styles
  • apply classNames.list and styles.list to the notification list root node
  • extend the list semantic test to cover both list and listContent

Why

listContent was supported through useNotification({ classNames, styles }), but the list root only accepted provider context class names and the placement className/style props. This makes the semantic API cover the list root consistently.

Validation

  • npm test -- --run tests/index.test.tsx
  • npm test
  • npm run lint (passes with existing react-hooks warnings)

Summary by CodeRabbit

发布说明

  • 新特性

    • 通知列表组件现支持列表级别样式和类名自定义,允许对列表容器和内容区域进行更灵活的样式配置。
  • 测试

    • 新增测试用例验证列表级别样式和类名的正确应用。

@vercel
Copy link
Copy Markdown

vercel Bot commented May 6, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
notification Error Error May 6, 2026 3:39pm

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 6, 2026

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: bb1ca9c3-3e62-4684-bbf2-a61170a9ed09

📥 Commits

Reviewing files that changed from the base of the PR and between 2384559 and cc41ac7.

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

功能总览

本 PR 为 NotificationList 组件引入了列表级别的样式钩子。通过扩展 NotificationClassNames 和 NotificationStyles 接口,添加 list 和 listContent 字段,使得列表容器和内容区域支持自定义类名和样式。渲染逻辑相应更新以应用这些新的样式配置。

变更

列表级别样式扩展

Layer / File(s) Summary
数据形状
src/NotificationList/index.tsx
NotificationClassNames 接口新增 listlistContent 字符串字段;NotificationStyles 接口新增 listlistContent 的 CSSProperties 字段。
渲染逻辑
src/NotificationList/index.tsx
列表容器通过 clsx 应用 classNames?.list;外层容器样式合并 styles?.list;listContent 样式作为对象传递给 Content 包装器。
测试验证
tests/index.test.tsx
新增测试用例验证列表和内容区域的样式与类名应用;移除不相关的 bamboo-topRight 断言;扩展现有测试以覆盖列表级别的样式验证。

相关 PR

  • react-component/notification#388:直接修改 NotificationList 的导出接口和渲染逻辑,与本 PR 改动的接口定义相关联。
  • react-component/notification#387:两个 PR 均修改 NotificationList → Content 的交互;本 PR 将 listContent 从类名字符串改为样式对象并暴露列表级钩子,该 PR 则添加 topNoticeHeight/topNoticeWidth 属性转发。
  • react-component/notification#385:均扩展 NotificationList 的 classNames/styles API 以添加新的槽位字段,实现方向一致。

预估审核工作量

🎯 2 (Simple) | ⏱️ ~10-15 分钟

🐰✨ 列表风格新增添,
list 和 listContent 闪闪亮,
样式钩子随意挂,
通知列表展新妆!

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/support-list-semantic-styles

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 and usage tips.

@codecov
Copy link
Copy Markdown

codecov Bot commented May 6, 2026

Codecov Report

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

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #389   +/-   ##
=======================================
  Coverage   99.19%   99.19%           
=======================================
  Files          13       13           
  Lines        1366     1369    +3     
  Branches      180      182    +2     
=======================================
+ Hits         1355     1358    +3     
  Misses         11       11           

☔ View full report in Codecov by Sentry.
📢 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.

Copy link
Copy Markdown

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

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 introduces the list property to both classNames and styles props in the NotificationList component, allowing for more granular styling of the notification list container. Corresponding tests were added to ensure these properties are correctly applied to the DOM. The review feedback suggests reordering the class name application so that the generic className prop takes precedence over the semantic classNames.list, ensuring consistency with the style merging logic where the style prop overrides styles.list.

I am having trouble creating individual review comments. Click here to see my feedback.

src/NotificationList/index.tsx (243-244)

medium

For consistency with the style prop merging (where the generic style prop overrides semantic styles.list) and to ensure the className prop acts as the final override for static classes, consider placing className after classNames?.list in the clsx call. Currently, classNames?.list is placed after className, which creates an inconsistency in the override hierarchy between classes and styles within the same component.

        classNames?.list,
        className,

@zombieJ zombieJ marked this pull request as ready for review May 6, 2026 15:46
@zombieJ zombieJ merged commit ab7e558 into master May 6, 2026
8 of 10 checks passed
@zombieJ zombieJ deleted the codex/support-list-semantic-styles branch May 6, 2026 15:46
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