refactor: rename to container #505
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Rate limit exceeded@zombieJ has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 11 minutes and 0 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (3)
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. Walkthrough将组件样式语义键从 Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
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 @zombieJ, 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 focuses on a refactoring effort to enhance the semantic clarity and consistency of the tooltip component's internal naming. By renaming the 'body' concept to 'container', the changes aim to provide a more intuitive and accurate representation of the component's structure, impacting prop definitions, internal logic, documentation, and test coverage. Highlights
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
|
|
All alerts resolved. Learn more about Socket for GitHub. This PR previously contained dependency changes with security issues that have been resolved, removed, or ignored. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #505 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 3 3
Lines 37 37
Branches 15 15
=========================================
Hits 37 37 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![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 refactors the component by renaming the body property to container across the codebase, including props, types, and class names. The changes are consistent and correctly applied. I've added a couple of suggestions to improve documentation and test clarity following this refactoring.
| | classNames | classNames?: { root?: string; container?: string;}; | | Semantic DOM class | | ||
| | styles | styles?: {root?: React.CSSProperties;container?: React.CSSProperties;}; | | Semantic DOM styles | |
There was a problem hiding this comment.
The documentation for classNames and styles is incomplete. It should also include arrow and uniqueBody as possible keys, according to the SemanticName type in src/Tooltip.tsx.
| | classNames | classNames?: { root?: string; container?: string;}; | | Semantic DOM class | | |
| | styles | styles?: {root?: React.CSSProperties;container?: React.CSSProperties;}; | | Semantic DOM styles | | |
| | classNames | classNames?: { root?: string; container?: string; arrow?: string; uniqueBody?: string; }; | | Semantic DOM class | | |
| | styles | styles?: { root?: React.CSSProperties; container?: React.CSSProperties; arrow?: React.CSSProperties; uniqueBody?: React.CSSProperties; }; | | Semantic DOM styles | |
tests/index.test.tsx
Outdated
| const customClassNames = { | ||
| root: 'custom-root', | ||
| body: 'custom-body', | ||
| container: 'custom-body', |
There was a problem hiding this comment.
The class name 'custom-body' is a remnant of the pre-refactor naming and is now misleading. For consistency and clarity, please rename it to 'custom-container'. This applies to other tests in this file as well. You'll also need to update the corresponding assertions to check for this new class name.
| container: 'custom-body', | |
| container: 'custom-container', |
|
|
||
| const tooltipElement = container.querySelector('.rc-tooltip') as HTMLElement; | ||
| const tooltipBodyElement = container.querySelector('.rc-tooltip-body') as HTMLElement; | ||
| const tooltipBodyElement = container.querySelector('.rc-tooltip-container') as HTMLElement; |
There was a problem hiding this comment.
The variable name tooltipBodyElement is now misleading since it queries for .rc-tooltip-container. For better code clarity and maintainability, consider renaming it to tooltipContainerElement and updating its usages accordingly. This applies to all tests where this variable is used in this file.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
tests/index.test.tsx (1)
442-462: 命名一致性：unique-body-class → unique-container-class既然已从 uniqueBody 迁移为 uniqueContainer，测试类名也建议同步，避免混淆。
- classNames={{ uniqueContainer: 'unique-body-class' }} + classNames={{ uniqueContainer: 'unique-container-class' }}src/Tooltip.tsx (1)
157-160: 确认 Trigger 支持 uniqueBg 属性，并可微调书写以降噪*当前通过 as any 透传 uniqueBgClassName/uniqueBgStyle OK，但请确认依赖版本已支持；另外可去掉多余括号，稍降可读性噪音。
建议微调为：
- {...(({ - uniqueBgClassName: classNames?.uniqueContainer, - uniqueBgStyle: styles?.uniqueContainer, - } as any))} + {...({ + uniqueBgClassName: classNames?.uniqueContainer, + uniqueBgStyle: styles?.uniqueContainer, + } as any)}并参考 package.json 评论确认 @rc-component/trigger 版本具备该能力。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
package.json(1 hunks)src/Tooltip.tsx(2 hunks)tests/index.test.tsx(10 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
tests/index.test.tsx (1)
docs/examples/simple.tsx (1)
render(112-227)
🔇 Additional comments (10)
tests/index.test.tsx (9)
322-322: 将自定义类名从 custom-body 统一重命名为 custom-container与“container”语义保持一致，避免遗留“body”命名混淆。
建议改成：
- container: 'custom-body', + container: 'custom-container',
338-345: 变量名与查询选择器不一致：tooltipBodyElement → tooltipContainerElement当前选择器为
.rc-tooltip-container，变量名建议同步为 container，提升可读性。- const tooltipBodyElement = container.querySelector('.rc-tooltip-container') as HTMLElement; + const tooltipContainerElement = container.querySelector('.rc-tooltip-container') as HTMLElement; @@ - expect(tooltipBodyElement).toHaveClass('custom-body'); + expect(tooltipContainerElement).toHaveClass('custom-container');
361-367: 同上：重命名变量并同步断言与 container 语义保持一致。
- const tooltipBodyElement = container.querySelector('.rc-tooltip-container') as HTMLElement; + const tooltipContainerElement = container.querySelector('.rc-tooltip-container') as HTMLElement; @@ - expect(tooltipBodyElement).toHaveStyle({ color: 'red', fontSize: '14px' }); + expect(tooltipContainerElement).toHaveStyle({ color: 'red', fontSize: '14px' });
373-373: 统一类名：custom-body → custom-container保持与“container”语义一致。
- container: 'custom-body', + container: 'custom-container',
396-405: 重命名变量并同步断言内容与类名与 container 语义保持一致。
- const tooltipBodyElement = container.querySelector('.rc-tooltip-container') as HTMLElement; + const tooltipContainerElement = container.querySelector('.rc-tooltip-container') as HTMLElement; @@ - expect(tooltipBodyElement).toHaveClass('custom-body'); - expect(tooltipBodyElement).toHaveStyle({ color: 'red' }); + expect(tooltipContainerElement).toHaveClass('custom-container'); + expect(tooltipContainerElement).toHaveStyle({ color: 'red' });
410-410: 统一类名：custom-body → custom-container保持与“container”语义一致。
- container: 'custom-body', + container: 'custom-container',
430-436: 重命名变量并同步断言类名与 container 语义保持一致。
- const tooltipBodyElement = container.querySelector('.rc-tooltip-container') as HTMLElement; + const tooltipContainerElement = container.querySelector('.rc-tooltip-container') as HTMLElement; @@ - expect(tooltipBodyElement).toHaveClass('custom-body'); + expect(tooltipContainerElement).toHaveClass('custom-container');
468-468: 统一类名：custom-body → custom-container保持与“container”语义一致。
- container: 'custom-body', + container: 'custom-container',
491-501: 重命名变量并同步断言类名与 container 语义保持一致。
- const tooltipBodyElement = container.querySelector('.rc-tooltip-container') as HTMLElement; + const tooltipContainerElement = container.querySelector('.rc-tooltip-container') as HTMLElement; @@ - expect(tooltipBodyElement).toHaveClass('custom-body'); - expect(tooltipBodyElement).toHaveStyle({ color: 'red' }); + expect(tooltipContainerElement).toHaveClass('custom-container'); + expect(tooltipContainerElement).toHaveStyle({ color: 'red' });src/Tooltip.tsx (1)
11-11: 语义键重命名对齐：container/uniqueContainer类型层面的改名清晰到位，便于外部 API 一致性维护。
| }, | ||
| "dependencies": { | ||
| "@rc-component/trigger": "^3.6.7", | ||
| "@rc-component/trigger": "^3.6.15", |
There was a problem hiding this comment.
🧩 Analysis chain
依赖升级合理，但请确认 Trigger 新增属性可用性
升级到 @rc-component/trigger@^3.6.15 看起来是为支持 uniqueBgClassName/uniqueBgStyle 映射。为避免运行期无效属性导致样式不生效，请确认该版本已正式暴露这两个属性。
请用以下 Web 查询确认属性支持与变更日志：
🌐 Web query:
Does @rc-component/trigger v3.6.15 support props `uniqueBgClassName` and `uniqueBgStyle` on Trigger? Provide release notes or code references.
💡 Result:
Short answer: No — @rc-component/trigger (v3.x) does not expose props named uniqueBgClassName or uniqueBgStyle. The component's documented props (README / API) do not list those props. [1]
References:
- Repository README / API props list for @rc-component/trigger (does not include uniqueBgClassName / uniqueBgStyle). [1]
- Releases / tags for react-component/trigger (v3.x series). [2]
Sources:
[1] react-component/trigger — README API (props list). (github.com)
[2] react-component/trigger — Releases / tags. (github.com)
If you want, I can search specific release (3.6.15) source files (index.tsx / prop types) and paste the exact code lines from that tag.
Citations:
- 1: https://github.com/react-component/trigger
- 2: https://github.com/react-component/trigger/releases?utm_source=openai
移除或替换 uniqueBgClassName/uniqueBgStyle，Trigger v3.6.15 未暴露该 props
经核实，v3.x 系列 API 文档和 Release Notes 均未列出这两个属性，运行时会无效。请移除或替换为受支持的属性。
🤖 Prompt for AI Agents
package.json lines 44–44: the dependency "@rc-component/trigger": "^3.6.15"
points to a version that does not expose uniqueBgClassName/uniqueBgStyle; locate
any usages of uniqueBgClassName and uniqueBgStyle in the codebase and remove
them (or replace with supported alternatives such as applying className/style to
the trigger's child, overlay/popup container, or the library's documented
overlayClassName/overlayStyle prop), then ensure tests/build pass; do not try to
rely on those props for v3.x — if you truly need those props instead of
refactoring, change the dependency to a verified version that documents and
supports them and update package.json accordingly.
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/Tooltip.tsx (1)
11-11: 公开类型重命名可能带来破坏性变更；建议提供平滑迁移（兼容旧键）将 'body'/'uniqueBody' 重命名为 'container'/'uniqueContainer' 会立即破坏依赖老键的使用者（运行时不会自动映射）。建议在一个小版本周期内同时兼容旧键，并做运行时规范化映射。
最小改动建议：
- 类型暂时同时接受新旧键（便于用户升级且不阻断编译）：
-export type SemanticName = 'root' | 'arrow' | 'container' | 'uniqueContainer'; +export type SemanticName = + | 'root' + | 'arrow' + | 'container' + | 'uniqueContainer' + | 'body' // legacy + | 'uniqueBody'; // legacy
- 在运行时归一化 classNames/styles（放在 props 解构后），以确保旧键仍能生效（支持迁移期）：
const normalizedClassNames = React.useMemo(() => ({ ...classNames, container: classNames?.container ?? (classNames as any)?.body, uniqueContainer: classNames?.uniqueContainer ?? (classNames as any)?.uniqueBody, }), [classNames]); const normalizedStyles = React.useMemo(() => ({ ...styles, container: styles?.container ?? (styles as any)?.body, uniqueContainer: styles?.uniqueContainer ?? (styles as any)?.uniqueBody, }), [styles]);并在 JSX 中改为：
<Popup ... classNames={normalizedClassNames} styles={normalizedStyles} >可选：开发环境下对使用旧键打印一次性 deprecate 警告，便于用户发现和迁移。
请同时确认本 PR 的发版语义（若不做兼容，应按 semver 主版本发布并在变更日志/README 明示）。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/Tooltip.tsx(2 hunks)tests/index.test.tsx(10 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
tests/index.test.tsx (1)
docs/examples/simple.tsx (1)
render(112-227)
🔇 Additional comments (2)
tests/index.test.tsx (2)
320-324: 统一命名以匹配 container 语义当前几处测试仍然把 container 的自定义 class 命名为
custom-body，与新版语义不符，容易误导后续维护者。建议顺手替换为custom-container（以及相应断言），并同步更新测试数据，保证用例语义与实现一致。Also applies to: 372-374, 409-412, 466-474
338-340: 变量名应反映容器语义这些变量依旧叫
tooltipBodyElement，但现在查询的是.rc-tooltip-container。沿用旧名会造成理解偏差，建议改成tooltipContainerElement并更新引用，以便测试代码更准确地表达新的 DOM 结构。Also applies to: 361-362, 396-397, 430-431, 490-491
src/Tooltip.tsx
Outdated
| {...(({ | ||
| uniqueContainerClassName: classNames?.uniqueContainer, | ||
| uniqueContainerStyle: styles?.uniqueContainer, | ||
| } as any))} |
There was a problem hiding this comment.
🧩 Analysis chain
uniqueContainer 透传依赖 rc-trigger 新接口；建议临时桥接与类型增强，避免回归*
这里通过 as any 向 Trigger 透传 uniqueContainerClassName/Style。若下游 rc-trigger 尚未发布相应属性，功能会静默失效（此前的 uniqueBg* 也会随之失效）。建议：
- 迁移期同时透传新旧两套 prop（新优先），保障双向兼容；
- 用模块声明合并为 TriggerProps 增补类型，避免 as any。
兼容性透传最小 diff（仅替换该块）：
- {...(({
- uniqueContainerClassName: classNames?.uniqueContainer,
- uniqueContainerStyle: styles?.uniqueContainer,
- } as any))}
+ {...(({
+ // New API (prefer)
+ uniqueContainerClassName: classNames?.uniqueContainer ?? (classNames as any)?.uniqueBody,
+ uniqueContainerStyle: styles?.uniqueContainer ?? (styles as any)?.uniqueBody,
+ // Back-compat for older rc-trigger
+ uniqueBgClassName: classNames?.uniqueContainer ?? (classNames as any)?.uniqueBody,
+ uniqueBgStyle: styles?.uniqueContainer ?? (styles as any)?.uniqueBody,
+ } as any))}类型增强（新增 ambient 声明文件 types/rc-trigger-aug.d.ts）：
declare module '@rc-component/trigger/lib/interface' {
interface TriggerProps {
uniqueContainerClassName?: string;
uniqueContainerStyle?: React.CSSProperties;
uniqueBgClassName?: string;
uniqueBgStyle?: React.CSSProperties;
}
}核验脚本：确认仓库使用的 @rc-component/trigger 版本，并检索仓库内对相关键的使用是否一致。
如需对下游库进行在线确认，请搜索：
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Package version of @rc-component/trigger:"
if [ -f package.json ]; then
rg -nP '"@rc-component/trigger"\s*:\s*".*"' package.json || true
fi
echo
echo "Scan code for uniqueContainer*/uniqueBg* and legacy keys to ensure consistent rename:"
rg -nP -C2 --type=ts 'unique(Container|Bg)(ClassName|Style)|classNames\?\.(unique(Container|Body))|styles\?\.(unique(Container|Body))'Length of output: 846
🌐 Web query:
Does @rc-component/trigger support props `uniqueContainerClassName` and `uniqueContainerStyle` (and were they formerly `uniqueBgClassName` / `uniqueBgStyle`)? Provide the first version that introduced the new props.
💡 Result:
I can't find evidence that @rc-component/trigger exposes props named uniqueContainerClassName / uniqueContainerStyle (or formerly uniqueBgClassName / uniqueBgStyle). The package docs/readme and published API lists do not show those props. [1][2]
If you want, I can continue searching the package source/commit history (or check specific versions) to try to find the exact PR/first version that added them. Which would you prefer?
Sources:
- @rc-component/trigger npm README / props (package page). [1]
- GitHub repo code search (no matches for "uniqueContainerClassName" / "uniqueBgClassName"). [2]
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Display the test around uniqueContainer mapping
sed -n '430,470p' tests/index.test.tsxLength of output: 1646
同时透传新旧 uniqueContainer*/uniqueBg* Props，增强类型声明
- 当前依赖的
@rc-component/trigger@^3.6.15尚未支持uniqueContainerClassName/uniqueContainerStyle，会被忽略 - 建议按下述最小 diff 在 spread 中同时传递旧 API (
uniqueBgClassName/uniqueBgStyle) 和新 API，优先使用新属性：
- {...(({
- uniqueContainerClassName: classNames?.uniqueContainer,
- uniqueContainerStyle: styles?.uniqueContainer,
- } as any))}
+ {...(({
+ uniqueContainerClassName: classNames?.uniqueContainer ?? (classNames as any)?.uniqueBody,
+ uniqueContainerStyle: styles?.uniqueContainer ?? (styles as any)?.uniqueBody,
+ uniqueBgClassName: classNames?.uniqueContainer ?? (classNames as any)?.uniqueBody,
+ uniqueBgStyle: styles?.uniqueContainer ?? (styles as any)?.uniqueBody,
+ } as any))}- 使用 module declaration 为
TriggerProps补充四个字段，移除as any
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| {...(({ | |
| uniqueContainerClassName: classNames?.uniqueContainer, | |
| uniqueContainerStyle: styles?.uniqueContainer, | |
| } as any))} | |
| {...(({ | |
| uniqueContainerClassName: classNames?.uniqueContainer ?? (classNames as any)?.uniqueBody, | |
| uniqueContainerStyle: styles?.uniqueContainer ?? (styles as any)?.uniqueBody, | |
| uniqueBgClassName: classNames?.uniqueContainer ?? (classNames as any)?.uniqueBody, | |
| uniqueBgStyle: styles?.uniqueContainer ?? (styles as any)?.uniqueBody, | |
| } as any))} |
🤖 Prompt for AI Agents
In src/Tooltip.tsx around lines 157 to 160, the spread currently casts to any
and only passes new uniqueContainer* props which are ignored by older
@rc-component/trigger versions; update the spread to include both new
(uniqueContainerClassName/uniqueContainerStyle) and legacy
(uniqueBgClassName/uniqueBgStyle) props, preferring the new values when present,
and remove the "as any" cast. Also add a module declaration to augment
TriggerProps with all four fields (uniqueContainerClassName,
uniqueContainerStyle, uniqueBgClassName, uniqueBgStyle) so types are correct and
you can drop the any cast.
Summary by CodeRabbit