feat: Support uniqueBgXXX for additional passing #574
Conversation
|
Caution Review failedThe pull request is closed. 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
Sequence Diagram(s)sequenceDiagram
autonumber
participant U as 用户
participant T as Trigger
participant G as generateTrigger / getUniqueOptions
participant P as UniqueProvider
participant F as FloatBg
Note over T,G: Trigger 构建 uniqueOptions（含 uniqueBgClassName/uniqueBgStyle）
U->>T: render({ unique, uniqueBgClassName, uniqueBgStyle })
T->>G: build UniqueShowOptions
G-->>T: UniqueShowOptions
T->>P: show(uniqueOptions)
P->>F: render FloatBg(props + uniqueBgClassName/uniqueBgStyle)
F-->>U: DOM: `.rc-trigger-popup-float-bg`（含合并后的 className 与 inline style）
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary of Changes
Hello @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 enhances the customization capabilities of the UniqueProvider's FloatBg component by introducing new properties that allow developers to apply custom CSS classes and inline styles to its background. This provides greater flexibility for styling unique popup elements without modifying core component logic.
Highlights
- New Styling Props for FloatBg: Introduced
uniqueBgClassNameanduniqueBgStyleprops to theFloatBgcomponent, allowing for custom CSS classes and inline styles to be applied directly to the background element of unique popups. - Propagate Styling Options: The new
uniqueBgClassNameanduniqueBgStyleprops are now propagated throughUniqueShowOptionsandTriggerProps, making them accessible from the mainTriggercomponent. - Comprehensive Test Coverage: Added new test cases to
unique.test.tsxto ensure thatuniqueBgClassNameanduniqueBgStyleare correctly applied and function as expected on theFloatBgcomponent. - Build Artifact Exclusion: The
.gitignorefile has been updated to includedist/, preventing build output directories from being committed to the repository.
Using Gemini Code Assist
The 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 /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in pull request comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
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 .gemini/ folder in the base of the repository. Detailed instructions can be found here.
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
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
This pull request introduces support for uniqueBgClassName and uniqueBgStyle props to allow custom styling of the FloatBg component when using a UniqueProvider. The changes are well-implemented, passing the new props through the component hierarchy from Trigger to FloatBg. The feature is also accompanied by comprehensive tests that validate the new functionality. My feedback focuses on improving the robustness of one of the new test assertions.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #574 +/- ##
=======================================
Coverage 95.72% 95.72%
=======================================
Files 17 17
Lines 936 936
Branches 265 265
=======================================
Hits 896 896
Misses 40 40 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@gemini-code-assist CR again |
| it('should apply uniqueBgClassName to FloatBg component', async () => { | ||
| const { container } = render( | ||
| <UniqueProvider> | ||
| <Trigger | ||
| action={['click']} | ||
| popup={<strong className="x-content">tooltip</strong>} | ||
| unique | ||
| uniqueBgClassName="custom-bg-class" | ||
| > | ||
| <div className="target">click me</div> | ||
| </Trigger> | ||
| </UniqueProvider>, | ||
| ); | ||
|
|
||
| // Initially no popup should be visible | ||
| expect(document.querySelector('.rc-trigger-popup')).toBeFalsy(); | ||
|
|
||
| // Click trigger to show popup | ||
| fireEvent.click(container.querySelector('.target')); | ||
| await awaitFakeTimer(); | ||
|
|
||
| // Check that popup exists | ||
| const popup = document.querySelector('.rc-trigger-popup'); | ||
| expect(popup).toBeTruthy(); | ||
|
|
||
| // Check that FloatBg has the custom background className | ||
| const floatBg = document.querySelector('.rc-trigger-popup-float-bg'); | ||
| expect(floatBg).toBeTruthy(); | ||
| expect(floatBg.className).toContain('custom-bg-class'); | ||
| }); | ||
|
|
||
| it('should apply uniqueBgStyle to FloatBg component', async () => { | ||
| const { container } = render( | ||
| <UniqueProvider> | ||
| <Trigger | ||
| action={['click']} | ||
| popup={<strong className="x-content">tooltip</strong>} | ||
| unique | ||
| uniqueBgStyle={{ backgroundColor: 'red', border: '1px solid blue' }} | ||
| > | ||
| <div className="target">click me</div> | ||
| </Trigger> | ||
| </UniqueProvider>, | ||
| ); | ||
|
|
||
| // Initially no popup should be visible | ||
| expect(document.querySelector('.rc-trigger-popup')).toBeFalsy(); | ||
|
|
||
| // Click trigger to show popup | ||
| fireEvent.click(container.querySelector('.target')); | ||
| await awaitFakeTimer(); | ||
|
|
||
| // Check that popup exists | ||
| const popup = document.querySelector('.rc-trigger-popup'); | ||
| expect(popup).toBeTruthy(); | ||
|
|
||
| // Check that FloatBg has the custom background style | ||
| const floatBg = document.querySelector('.rc-trigger-popup-float-bg'); | ||
| expect(floatBg).toBeTruthy(); | ||
| expect(floatBg).toHaveStyle({ | ||
| backgroundColor: 'red', | ||
| border: '1px solid blue', | ||
| }); | ||
| }); | ||
|
|
||
| it('should not apply any additional className to FloatBg when uniqueBgClassName is not provided', async () => { | ||
| const { container } = render( | ||
| <UniqueProvider> | ||
| <Trigger | ||
| action={['click']} | ||
| popup={<strong className="x-content">tooltip</strong>} | ||
| unique | ||
| > | ||
| <div className="target">click me</div> | ||
| </Trigger> | ||
| </UniqueProvider>, | ||
| ); | ||
|
|
||
| // Initially no popup should be visible | ||
| expect(document.querySelector('.rc-trigger-popup')).toBeFalsy(); | ||
|
|
||
| // Click trigger to show popup | ||
| fireEvent.click(container.querySelector('.target')); | ||
| await awaitFakeTimer(); | ||
|
|
||
| // Check that popup exists | ||
| const popup = document.querySelector('.rc-trigger-popup'); | ||
| expect(popup).toBeTruthy(); | ||
|
|
||
| // Check that FloatBg exists but does not have custom background className | ||
| const floatBg = document.querySelector('.rc-trigger-popup-float-bg'); | ||
| expect(floatBg).toBeTruthy(); | ||
| expect(floatBg.className).not.toContain('undefined'); | ||
| }); |
There was a problem hiding this comment.
这三个新的测试用例 (should apply uniqueBgClassName..., should apply uniqueBgStyle..., should not apply any additional className...) 中存在较多重复的设置代码，例如渲染组件、触发弹窗和检查弹窗是否可见的逻辑。
为了提高测试代码的可读性和可维护性，建议将这部分通用的设置逻辑提取到一个辅助函数中。这样可以让每个测试用例更专注于其核心的断言逻辑，使代码更简洁。
例如，可以创建一个这样的辅助函数：
async function setupAndOpenPopup(triggerProps: Partial<TriggerProps> = {}) {
const { container } = render(
<UniqueProvider>
<Trigger
action={['click']}
popup={<strong className="x-content">tooltip</strong>}
unique
{...triggerProps}
>
<div className="target">click me</div>
</Trigger>
</UniqueProvider>,
);
// Initially no popup should be visible
expect(document.querySelector('.rc-trigger-popup')).toBeFalsy();
// Click trigger to show popup
fireEvent.click(container.querySelector('.target'));
await awaitFakeTimer();
// Check that popup exists
const popup = document.querySelector('.rc-trigger-popup');
expect(popup).toBeTruthy();
return { container, popup };
}然后测试用例可以简化为：
it('should apply uniqueBgClassName to FloatBg component', async () => {
await setupAndOpenPopup({ uniqueBgClassName: 'custom-bg-class' });
const floatBg = document.querySelector('.rc-trigger-popup-float-bg');
expect(floatBg).toBeTruthy();
expect(floatBg.className).toContain('custom-bg-class');
});
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
src/UniqueProvider/FloatBg.tsx (1)
86-91: 用户样式可能覆盖动画内联样式（transform/opacity/transition）uniqueBgStyle 放在最后会覆盖 motionStyle 与定位样式，若用户传入 transform/opacity/transition，可能破坏入场/离场动画或定位。建议至少在文档中提示“避免设置这些关键属性”，或在此处做白名单/黑名单保护。
可选思路（其一：仅文档告警）：
- 在 TriggerProps 的 JSDoc 处补充：避免在 uniqueBgStyle 中设置 transform/opacity/transition。
可选思路（其二：运行时保护，避免覆盖关键属性）：
- style={{ - ...offsetStyle, - ...sizeStyle, - ...motionStyle, - ...uniqueBgStyle, - }} + style={{ + ...offsetStyle, + ...sizeStyle, + ...motionStyle, + // 仅当未提供这些关键属性时再合并（示例） + ...(uniqueBgStyle?.transform !== undefined || + uniqueBgStyle?.opacity !== undefined || + uniqueBgStyle?.transition !== undefined + ? {} + : uniqueBgStyle), + }}src/UniqueProvider/index.tsx (1)
158-158: prefixCls 建议加兜底，避免外部误用导致 undefinedUniqueProvider 主要由 Trigger 驱动，但接口是导出的。为稳妥起见可在此增加默认值，避免外部直接调用 show 时未提供 prefixCls 导致类名异常。
- const prefixCls = options?.prefixCls; + const prefixCls = options?.prefixCls ?? 'rc-trigger-popup';src/index.tsx (1)
101-104: 新增 props 与透传链路完整
- TriggerProps 新增 uniqueBgClassName/uniqueBgStyle，命名与语义清晰；
- generateTrigger 中解构并在 getUniqueOptions 中传递，配合 UniqueProvider/FloatBg 完成闭环。
建议在这两处 JSDoc 补充提示：uniqueBgStyle 中请避免设置 transform/opacity/transition 以免影响动画。
如需，我可以顺手提交一版 JSDoc 与 README 片段更新。
Also applies to: 176-178, 332-334
tests/unique.test.tsx (1)
187-219: 用 computedStyle 断言在 JSDOM 下可能不稳定，建议改为读取行内样式JSDOM 对样式计算存在差异，backgroundColor 可能变为 rgb(...)；改为断言元素的行内样式更稳。
应用此 diff 改进断言稳定性：
- const floatBg = document.querySelector('.rc-trigger-popup-float-bg'); - expect(floatBg).toBeTruthy(); - const computedStyle = window.getComputedStyle(floatBg); - expect(computedStyle.backgroundColor).toBe('red'); - expect(computedStyle.border).toContain('1px solid blue'); + const floatBg = document.querySelector('.rc-trigger-popup-float-bg') as HTMLElement; + expect(floatBg).toBeTruthy(); + const style = floatBg.style; + expect(style.backgroundColor).toBe('red'); + expect(style.border).toBe('1px solid blue');
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
.gitignore(1 hunks)src/UniqueProvider/FloatBg.tsx(4 hunks)src/UniqueProvider/index.tsx(1 hunks)src/context.ts(1 hunks)src/index.tsx(3 hunks)tests/unique.test.tsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
tests/unique.test.tsx (1)
tests/util.tsx (1)
awaitFakeTimer(97-104)
🔇 Additional comments (6)
src/context.ts (1)
23-24: 为 UniqueShowOptions 新增 uniqueBgClassName/uniqueBgStyle 定义合理类型与后续透传一致，命名与 popupClassName/popupStyle 对齐，易理解。
src/UniqueProvider/FloatBg.tsx (1)
20-21: className 透传实现到位将 uniqueBgClassName 直接并入 classNames，undefined/空字符串会被忽略，不会出现 'undefined' 类名污染（测试已覆盖）。
Also applies to: 37-39, 79-81
src/UniqueProvider/index.tsx (1)
219-221: 透传到 FloatBg 的实现正确uniqueBgClassName / uniqueBgStyle 均从 options 透传，符合上下文设计。
tests/unique.test.tsx (2)
156-185: 验证 className 透传的用例到位覆盖了自定义类名命中 FloatBg 的主路径，能够有效防回归。
220-248: 避免 'undefined' 类名的用例很实用能直接捕捉 classNames 合并失误，保留该用例有价值。
.gitignore (1)
42-43: 结论：dist/ 不会随 npm 发布 — 需确认发版策略package.json 的 "files" 字段为 ["es","lib","assets//*.css","assets//*.less"]，且仓库无 .npmignore；npm publish 会以 files 为白名单（若无 .npmignore 则会参考 .gitignore），因此 dist/（已在 .gitignore 中）不会包含在发布包中。
建议（任选其一）：
- 若需发布 dist：在 package.json 的 files 中加入 "dist"；
- 或让 CI 把构建产物输出到已包含的目录（es 或 lib）；
- 不推荐：直接提交 dist 到仓库并移除 .gitignore 中的 dist/。
位置提示：.gitignore（Lines 42-43），package.json（仓库根目录）。
Summary by CodeRabbit