Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
功能总览本 PR 为 NotificationList 组件引入了列表级别的样式钩子。通过扩展 NotificationClassNames 和 NotificationStyles 接口,添加 list 和 listContent 字段,使得列表容器和内容区域支持自定义类名和样式。渲染逻辑相应更新以应用这些新的样式配置。 变更列表级别样式扩展
相关 PR
预估审核工作量🎯 2 (Simple) | ⏱️ ~10-15 分钟 诗
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
Codecov Report✅ All modified and coverable lines are covered by tests. 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. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
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)
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,
Summary
listto notification semanticclassNamesandstylesclassNames.listandstyles.listto the notification list root nodelistandlistContentWhy
listContentwas supported throughuseNotification({ classNames, styles }), but the list root only accepted provider context class names and the placementclassName/styleprops. This makes the semantic API cover the list root consistently.Validation
npm test -- --run tests/index.test.tsxnpm testnpm run lint(passes with existing react-hooks warnings)Summary by CodeRabbit
发布说明
新特性
测试