chore: popup support style inject #504
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Walkthrough在 src/Popup.tsx 中为 ContentProps 新增可选的 className 与 style 属性；在组件中解构并将 className 合并到根元素的 className，style 与现有 body 样式进行合并后应用到根 div。 Changes
Sequence Diagram(s)sequenceDiagram
actor Consumer as 使用方
participant Popup as Popup 组件
participant DOM as 根 div
Consumer->>Popup: 传入 { className, style, ... }
Popup->>Popup: 合并样式(内部bodyStyle ⟶ 外部style)
Popup->>Popup: 合成 className(内部class ⟶ 外部className)
Popup->>DOM: 渲染 <div class=... style=...>
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 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 introduces direct styling capabilities to the 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
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #504 +/- ##
=========================================
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 adds support for className and style props to the Popup component, allowing for more direct styling. The implementation is straightforward and correct. I have one suggestion for a performance improvement regarding the creation of the style object, which could be beneficial given that this component is used for tooltips that may re-render frequently.
| className={cls(`${prefixCls}-body`, classNames?.body)} | ||
| style={styles?.body} | ||
| className={cls(`${prefixCls}-body`, classNames?.body, className)} | ||
| style={{ ...styles?.body, ...style }} |
There was a problem hiding this comment.
For performance, it's better to memoize this style object to avoid creating a new one on every render. This is especially important for components like tooltips that can re-render frequently on user interactions like hover. You can use React.useMemo here to prevent unnecessary re-renders of child components and reduce the overhead of React's reconciliation process.
| style={{ ...styles?.body, ...style }} | |
| style={React.useMemo(() => ({ ...styles?.body, ...style }), [styles, style])} |
![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
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/Popup.tsx(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: thinkasany
PR: react-component/tooltip#484
File: src/Popup.tsx:20-25
Timestamp: 2024-11-28T13:00:19.052Z
Learning: `overlayInnerClassName` 属性在 `Popup.tsx` 中是供内部使用的，已经通过 `classNames.inner` 在现有的测试用例中得到验证，无需单独添加测试。
📚 Learning: 2024-11-28T13:00:19.052Z
Learnt from: thinkasany
PR: react-component/tooltip#484
File: src/Popup.tsx:20-25
Timestamp: 2024-11-28T13:00:19.052Z
Learning: `overlayInnerClassName` 属性在 `Popup.tsx` 中是供内部使用的，已经通过 `classNames.inner` 在现有的测试用例中得到验证，无需单独添加测试。
Applied to files:
src/Popup.tsx
🔇 Additional comments (3)
src/Popup.tsx (3)
11-12: 新增属性类型定义看起来正确。为
ContentProps接口添加className和style属性，类型定义合适，都是可选的。这样的设计允许外部组件传入自定义的类名和样式。
16-16: 解构赋值正确添加了新属性。在解构赋值中正确包含了新增的
className和style属性，与接口定义保持一致。
21-22: 样式和类名合并逻辑实现正确。
- 第21行：使用
cls函数正确合并了prefixCls-body、classNames?.body和新增的className- 第22行：使用对象展开语法正确合并了
styles?.body和新增的style，新传入的样式会覆盖相同属性的默认样式这种实现方式符合React组件的常见模式，优先级处理也很合理。
| className?: string; | ||
| style?: React.CSSProperties; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
🧩 Analysis chain
验证新增属性的使用是否与现有测试兼容。
根据学习记录，overlayInnerClassName 等内部属性已有相应测试覆盖。建议验证新增的 className 和 style 属性是否会影响现有的测试用例。
运行以下脚本来检查相关测试文件和使用情况：
Also applies to: 16-16, 21-22
🏁 Script executed:
#!/bin/bash
# 描述：检查 Popup 组件的测试文件和使用情况
# 查找测试文件
echo "=== 查找 Popup 相关测试文件 ==="
fd -e test.tsx -e test.ts -e spec.tsx -e spec.ts | xargs rg -l "Popup|ContentProps"
# 查找 Popup 组件的使用情况
echo -e "\n=== 查找 Popup 组件使用情况 ==="
rg -n -C3 "ContentProps|<Popup" --type=tsx --type=ts
# 检查是否有现有的 className 或 style 相关测试
echo -e "\n=== 检查现有的 className 和 style 测试 ==="
rg -n -C3 "className.*body|style.*body" --type=tsx --type=ts -g '!node_modules'Length of output: 161
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== tests/popup.test.tsx ==="
sed -n '1,400p' tests/popup.test.tsx || true
echo -e "\n=== src/Popup.tsx (first 200 lines) ==="
sed -n '1,200p' src/Popup.tsx || true
echo -e "\n=== Search overlayInnerClassName across repo ==="
rg -n --hidden -S "overlayInnerClassName" -g '!*node_modules*' || true
echo -e "\n=== Search for className/style mentions in tests ==="
rg -n --hidden -S "className|style" tests -g '!*node_modules*' || true
echo -e "\n=== Search for Popup usages across repo ==="
rg -n --hidden -S "<Popup\b|\bContentProps\b" -g '**/*.{ts,tsx,js,jsx,ts,tsx}' || trueLength of output: 1506
为 Popup 新增的 className/style 添加或更新测试
tests/popup.test.tsx 仅断言导出；src/Popup.tsx 已在接口中接收并将 className/style 合并到渲染的 div（行 11–12），但没有针对 className/style 的测试覆盖。请新增或更新测试以覆盖：1) className 与 classNames.body 的合并行为；2) style 与 styles.body 的合并行为；并确认 src/Tooltip.tsx:132 与 docs/examples/placement.tsx:95 的使用不受影响。
🤖 Prompt for AI Agents
In src/Popup.tsx around lines 11 to 12, the component accepts className and
style but tests don't verify that these are merged with classNames.body and
styles.body; update tests/popup.test.tsx to (1) render Popup with a custom
className and assert the rendered div contains both the custom className and
classNames.body (use container.querySelector or getByTestId on the body div);
(2) render Popup with a custom style (e.g., { color: 'red' }) and assert the
rendered div's style includes both the custom style and styles.body properties
(merged correctly); keep the existing export assertion in the test file and add
assertions that src/Tooltip.tsx line 132 and docs/examples/placement.tsx line 95
usages remain valid by ensuring the test cases use the same props shapes so
those usages are not broken.
Summary by CodeRabbit