refactor(Masonry): 🛠 use genCssVar hook to generate CSS variables#56547
refactor(Masonry): 🛠 use genCssVar hook to generate CSS variables#56547li-jia-nan merged 1 commit intomasterfrom
Conversation
|
|
👁 Visual Regression Report for PR #56547 Failed ❌
Important There are 2 diffs found in this PR: 🔄
|
📝 WalkthroughSummary by CodeRabbit版本发布说明
✏️ Tip: You can customize this high-level summary in your review settings. 总体说明此拉取请求实现了砌体组件的动态CSS变量处理机制，用genCssVar和getPrefixCls替代硬编码的CSS变量名，以支持主题化的样式计算。涉及组件逻辑和对应测试的更新。 变更内容
估计代码审查工作量🎯 2 (简单) | ⏱️ ~12 分钟 诗句
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
Summary of ChangesHello @li-jia-nan, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a refactoring effort within the Masonry component to adopt a more standardized approach for managing CSS variables. By integrating the Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
|
|
There was a problem hiding this comment.
Pull request overview
This PR refactors the Masonry component to use the genCssVar hook for generating CSS variable names, replacing the hardcoded --item-width variable with the properly prefixed --ant-masonry-item-width variable. This aligns the Masonry component with Ant Design's CSS variable naming conventions.
Key Changes:
- Adopted
genCssVarhook to generate consistent CSS variable names and references - Updated CSS variable from
--item-widthto--ant-masonry-item-width - Modified tests to use the new variable naming pattern
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| components/masonry/Masonry.tsx | Added genCssVar hook import and implementation; replaced hardcoded CSS variable names with generated ones using varName and varRef functions |
| components/masonry/tests/index.test.tsx | Updated test to use genCssVar hook for retrieving the correct CSS variable name; added optional chaining for safer null handling |
| components/masonry/tests/snapshots/demo-extend.test.ts.snap | Updated all snapshot occurrences of CSS variable from --item-width to --ant-masonry-item-width |
There was a problem hiding this comment.
Code Review
This pull request refactors the Masonry component to use the genCssVar hook for generating CSS variables, which is a great improvement for consistency and avoiding potential CSS variable name conflicts across the application. The changes in the component and the updated snapshots look correct. I have one minor suggestion in a test file to make test failures more explicit and easier to debug.
size-limit report 📦
|
More templates
commit: |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
components/masonry/__tests__/index.test.tsx (1)
203-210: 避免对match()结果使用非空断言，提升失败时可诊断性Line 206-210：
horizontalGutter!/verticalGutter!在解析失败时会抛出不直观的异常（或产出NaN），建议显式断言并给出更清晰的错误。建议修改（更清晰的断言与报错）
const getGutter = () => { const itemElements = document.body.querySelectorAll<HTMLElement>('.ant-masonry-item'); - const horizontalGutter = itemElements[0].style + const horizontalGutter = itemElements[0].style .getPropertyValue(varName('item-width')) .match(/\d+px/)?.[0]; - const verticalGutter = itemElements[2].style.top.match(/\d+px/)?.[0]; - return [Number.parseInt(horizontalGutter!, 10), Number.parseInt(verticalGutter!, 10)]; + const verticalGutter = itemElements[2].style.top.match(/\d+px/)?.[0]; + + if (!horizontalGutter || !verticalGutter) { + throw new Error( + `Failed to parse gutter. horizontal=${String(horizontalGutter)} vertical=${String(verticalGutter)}`, + ); + } + + return [Number.parseInt(horizontalGutter, 10), Number.parseInt(verticalGutter, 10)]; };components/masonry/Masonry.tsx (1)
265-267: CSS 变量替换到varName/varRef一致性 OK；可选地把 itemWidth 计算提到外层避免每个 item 重复拼接Line 265-267：用
[varName('item-width')]+varRef('item-width')替代硬编码/手写var(...)是一致且更可维护的做法。
可选优化：calc((100% + ${horizontalGutter}px) / ${columnCount})对所有 item 相同，可以在外层预先计算（或 memo）再复用，减少 render 时每个 item 的字符串拼接开销。
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
components/masonry/__tests__/__snapshots__/demo-extend.test.ts.snapis excluded by!**/*.snap
📒 Files selected for processing (2)
components/masonry/Masonry.tsxcomponents/masonry/__tests__/index.test.tsx
🧰 Additional context used
📓 Path-based instructions (8)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Always use TypeScript with strict type checking
Never useanytype - define precise types instead
Use interfaces (not type aliases) for object structures
Export all public interface types
Prefer union types over enums, useas constfor constants
Use early returns to improve readability
Support tree shaking
Follow import order: React → dependencies → antd components → custom components → types → styles
Prefer antd built-in components over external dependencies
Pass all ESLint and TypeScript checks
No console errors or warnings
**/*.{ts,tsx}: Use TypeScript and React to develop, with function-based components and hooks instead of class components
Use early returns to improve code readability
Component names should use PascalCase
Property names should use camelCase
Use React.memo, useMemo, and useCallback appropriately to optimize performance
All components and functions must provide accurate type definitions; avoid usinganytype and use interfaces instead of type aliases for object structures
Component props should use interface definition with naming conventionComponentNameProps; define separate interfaces for component state likeComponentNameState
UseReact.ForwardRefRenderFunctionfor component ref types and explicitly define all callback function parameters and return values
Use generics appropriately to enhance type flexibility; use union types andas constinstead ofenum; rely on TypeScript's type inference and minimize type assertions
Internationalization: obtain locale configuration viauseLocalehook fromcomponents/locale/index.tsx; ensure new locale configurations have corresponding type definitions
Files:
components/masonry/__tests__/index.test.tsxcomponents/masonry/Masonry.tsx
components/**/*.tsx
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
components/**/*.tsx: Component props interfaces should be namedComponentNameProps
Component ref types should useReact.ForwardRefRenderFunction
Use functional components with hooks exclusively (no class components)
Apply performance optimizations with React.memo, useMemo, useCallback appropriately
Support server-side rendering
Components must support ref forwarding with structure including nativeElement, focus, and other methods
Use PascalCase for component names
Use camelCase for props with specific patterns:default+PropNamefor defaults,forceRenderfor force rendering,openinstead ofvisiblefor panel state,show+PropNamefor display toggles,PropName+ablefor capabilities,dataSourcefor data source,disabledfor disabled state,extrafor additional content,iconfor icons,triggerfor triggers,classNamefor CSS classes
Useon+EventNamepattern for event handlers (e.g.,onClick,onChange)
Useon+SubComponentName+EventNamepattern for sub-component events
Use complete names, never abbreviations in prop naming
Optimize for minimal re-renders
UseuseLocalehook fromcomponents/locale/index.tsx
Support accessibility (WCAG 2.1 AA)
Files:
components/masonry/__tests__/index.test.tsxcomponents/masonry/Masonry.tsx
components/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Never hardcode colors, sizes, or spacing values
Files:
components/masonry/__tests__/index.test.tsxcomponents/masonry/Masonry.tsx
**/__tests__/**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/__tests__/**/*.test.{ts,tsx}: Write comprehensive tests using Jest and React Testing Library
Include snapshot tests for UI components
Files:
components/masonry/__tests__/index.test.tsx
**/*.{ts,tsx,md}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use 2-space indentation
Files:
components/masonry/__tests__/index.test.tsxcomponents/masonry/Masonry.tsx
**/*.{ts,tsx,css}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Maintain cross-browser compatibility
Files:
components/masonry/__tests__/index.test.tsxcomponents/masonry/Masonry.tsx
**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
**/*.tsx: Use @ant-design/cssinjs as the styling solution; place component styles instyle/directory with filenames followinggen[ComponentName]Stylenaming convention
Use CSS-in-JS and avoid hardcoded colors, sizes, and spacing values; base component styles on global and component-level design tokens
Components must support dark mode, different screen sizes, and RTL (right-to-left) reading direction; use CSS logical properties likemargin-inline-startinstead of directional properties
Use CSS transitions for simple animations and @rc-component/motion for complex animations; respect prefers-reduced-motion user preference
Follow WCAG 2.1 AA accessibility standards: ensure clear focus states, sufficient color contrast, avoid color-only information conveyance, support 200% page zoom, and avoid flickering animations
Files:
components/masonry/__tests__/index.test.tsxcomponents/masonry/Masonry.tsx
**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use Jest and React Testing Library for unit tests; place test files in tests directory with naming format
index.test.tsxorxxx.test.tsx; achieve 100% test coverage
Files:
components/masonry/__tests__/index.test.tsx
🧠 Learnings (9)
📚 Learning: 2025-11-24T16:30:28.374Z
Learnt from: CR
Repo: ant-design/ant-design PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T16:30:28.374Z
Learning: Applies to components/**/*.{ts,tsx} : Never hardcode colors, sizes, or spacing values
Applied to files:
components/masonry/__tests__/index.test.tsxcomponents/masonry/Masonry.tsx
📚 Learning: 2025-11-24T16:30:28.374Z
Learnt from: CR
Repo: ant-design/ant-design PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T16:30:28.374Z
Learning: Applies to **/*.{ts,tsx,css} : Maintain cross-browser compatibility
Applied to files:
components/masonry/__tests__/index.test.tsx
📚 Learning: 2026-01-04T15:16:45.886Z
Learnt from: CR
Repo: ant-design/ant-design PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-04T15:16:45.886Z
Learning: Applies to **/*.tsx : Use CSS-in-JS and avoid hardcoded colors, sizes, and spacing values; base component styles on global and component-level design tokens
Applied to files:
components/masonry/__tests__/index.test.tsxcomponents/masonry/Masonry.tsx
📚 Learning: 2025-11-24T16:30:28.374Z
Learnt from: CR
Repo: ant-design/ant-design PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T16:30:28.374Z
Learning: Applies to components/**/style/*.ts : Use CSS logical properties for RTL support (e.g., `margin-inline-start` instead of `margin-left`)
Applied to files:
components/masonry/__tests__/index.test.tsxcomponents/masonry/Masonry.tsx
📚 Learning: 2025-11-24T16:30:28.374Z
Learnt from: CR
Repo: ant-design/ant-design PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T16:30:28.374Z
Learning: Applies to **/__tests__/**/*.test.{ts,tsx} : Include snapshot tests for UI components
Applied to files:
components/masonry/__tests__/index.test.tsx
📚 Learning: 2025-11-24T16:30:28.374Z
Learnt from: CR
Repo: ant-design/ant-design PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T16:30:28.374Z
Learning: Applies to **/__tests__/**/*.test.{ts,tsx} : Write comprehensive tests using Jest and React Testing Library
Applied to files:
components/masonry/__tests__/index.test.tsx
📚 Learning: 2025-11-24T16:30:28.374Z
Learnt from: CR
Repo: ant-design/ant-design PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T16:30:28.374Z
Learning: Applies to components/**/style/*.ts : Generate styles with functions named `gen[ComponentName]Style`
Applied to files:
components/masonry/__tests__/index.test.tsx
📚 Learning: 2025-11-24T16:30:28.374Z
Learnt from: CR
Repo: ant-design/ant-design PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T16:30:28.374Z
Learning: Applies to components/**/style/*.ts : Use `ant-design/cssinjs` for all styling
Applied to files:
components/masonry/__tests__/index.test.tsxcomponents/masonry/Masonry.tsx
📚 Learning: 2025-11-13T02:52:08.942Z
Learnt from: meet-student
Repo: ant-design/ant-design PR: 55697
File: components/drawer/index.tsx:86-88
Timestamp: 2025-11-13T02:52:08.942Z
Learning: In Ant Design components (components/**/*.{ts,tsx}), always use `import useId from 'rc-util/lib/hooks/useId'` instead of `React.useId()` for generating unique IDs, to ensure compatibility with React 16-19. The rc-util hook automatically uses the native implementation in React 18+ and provides a fallback for older versions.
Applied to files:
components/masonry/Masonry.tsx
🧬 Code graph analysis (1)
components/masonry/Masonry.tsx (1)
components/theme/util/genStyleUtils.ts (1)
genCssVar(48-57)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (19)
- GitHub Check: Agent
- GitHub Check: CodeQL analysis (javascript-typescript)
- GitHub Check: visual-diff snapshot (2/2)
- GitHub Check: build preview
- GitHub Check: visual-diff snapshot (1/2)
- GitHub Check: test lib/es module (es, 1/2)
- GitHub Check: test lib/es module (es, 2/2)
- GitHub Check: test-node
- GitHub Check: test lib/es module (lib, 2/2)
- GitHub Check: test-react-legacy (18, 2/2)
- GitHub Check: test-react-legacy (18, 1/2)
- GitHub Check: test-react-latest (dom, 2/2)
- GitHub Check: build
- GitHub Check: test-react-latest (dom, 1/2)
- GitHub Check: lint
- GitHub Check: size
- GitHub Check: build
- GitHub Check: Analyze (javascript)
- GitHub Check: build
🔇 Additional comments (3)
components/masonry/__tests__/index.test.tsx (1)
7-8: 使用genCssVar/defaultPrefixCls替代硬编码 CSS 变量名是正确方向Line 7-8：测试侧用
genCssVar(defaultPrefixCls, 'masonry')来推导变量名，能避免变量名前缀变更导致测试脆弱，和本次重构目标一致。
如果后续要覆盖“自定义 ConfigProvider prefixCls”的场景，这里可能需要从渲染上下文而非defaultPrefixCls推导（否则测试只验证默认前缀路径）。components/masonry/Masonry.tsx (2)
96-100:rootPrefixCls + genCssVar的引入合理，可随全局前缀变化而稳定生成变量名Line 96-100：用
getPrefixCls()拿到根前缀再genCssVar(rootPrefixCls, 'masonry')，能让 CSS 变量命名跟随 ConfigProvider 的全局前缀配置（避免把组件级prefixCls误当作 CSS 变量前缀）。
238-238: 确认是否允许外部style.height覆盖内部计算的totalHeightLine 238：当前合并顺序是
height: totalHeight在最前，随后...style可能覆盖高度；如果 Masonry 期望“始终用内部计算高度”，建议把height放到最后。可选修改（让 totalHeight 始终生效）
- style={{ height: totalHeight, ...mergedStyles.root, ...contextStyle, ...style }} + style={{ ...mergedStyles.root, ...contextStyle, ...style, height: totalHeight }}
Deploying ant-design with
|
| Latest commit: |
b07fa22
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://9909167f.ant-design.pages.dev |
| Branch Preview URL: | https://gencssvar-masonry.ant-design.pages.dev |
Deploying ant-design-next with
|
| Latest commit: |
b07fa22
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://dccaa467.ant-design-next.pages.dev |
| Branch Preview URL: | https://gencssvar-masonry.ant-design-next.pages.dev |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #56547 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 805 805
Lines 14862 14864 +2
Branches 3919 3919
=========================================
+ Hits 14862 14864 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Bundle ReportChanges will increase total bundle size by 78 bytes (0.0%) ⬆️. This is within the configured threshold ✅ Detailed changes
Affected Assets, Files, and Routes:view changes for bundle: antd.min-array-pushAssets Changed:
|
中文版模板 / Chinese template
🤔 This is a ...
🔗 Related Issues
💡 Background and Solution
📝 Change Log