fix: missing align style #576
Conversation
|
Warning Rate limit exceeded@zombieJ has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 10 minutes and 20 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 (1)
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本次变更在 UniqueProvider 中将对齐产生的类名与传入的 uniqueBgClassName 合并后再传给 UniqueBody；同时新增对应的单测覆盖该合并行为。对外导出/公共 API 无变更。 Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant U as UniqueProvider
participant A as getPopupClassNameFromAlign
participant B as UniqueBody
participant T as Trigger
T->>U: 打开弹层时渲染
U->>A: 计算 alignedClassName(基于 align)
A-->>U: 返回 alignedClassName
note over U: 新增：将 alignedClassName 与 options.uniqueBgClassName 合并
U->>B: 渲染，传入 merged uniqueBgClassName
B-->>T: 展示包含合并类名的背景节点
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 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! 此拉取请求解决了 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 #576 +/- ##
==========================================
+ Coverage 95.72% 96.04% +0.32%
==========================================
Files 17 17
Lines 936 936
Branches 265 265
==========================================
+ Hits 896 899 +3
+ Misses 40 37 -3 ☔ 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
这个 PR 修复了 alignedClassName 没有被应用到 UniqueBody 组件上的一个问题，该问题导致了在使用 unique 模式时，弹出框的 transition 动画会丢失。修复方案是使用 classnames 工具将 options.uniqueBgClassName 和 alignedClassName 合并，然后一起传递给 UniqueBody。PR 中还增加了一个新的测试用例来验证这一修复。代码变更清晰且正确，测试用例也很好地覆盖了修改的逻辑。整体来看，这是一个高质量的修复。
![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 (2)
src/UniqueProvider/index.tsx (1)
219-222: 如需保持“未传 uniqueBgClassName 不变更”的旧语义，可改为条件合并（可选）仅当传入 uniqueBgClassName 时才合并对齐类，其余情况保持 undefined，降低潜在选择器冲突风险。
- uniqueBgClassName={classNames( - options.uniqueBgClassName, - alignedClassName, - )} + uniqueBgClassName={ + options.uniqueBgClassName + ? classNames(options.uniqueBgClassName, alignedClassName) + : undefined + }tests/unique.test.tsx (1)
260-265: 断言更稳：使用 toHaveClass 并精确匹配 expected 对齐类名（小优化）避免 substring 误判，直接断言具体类名更稳（此处为 'custom-align-tl'）。
- expect(uniqueBody).toBeTruthy(); - expect(uniqueBody.className).toContain('custom-bg-class'); - expect(uniqueBody.className).toContain('custom-align'); + expect(uniqueBody).toBeTruthy(); + const expectedAlign = 'custom-align-tl'; + expect(uniqueBody).toHaveClass('custom-bg-class'); + expect(uniqueBody).toHaveClass(expectedAlign);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/UniqueProvider/index.tsx(1 hunks)tests/unique.test.tsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
tests/unique.test.tsx (2)
src/mock.tsx (1)
UniqueProvider(36-36)tests/util.tsx (1)
awaitFakeTimer(97-104)
🔇 Additional comments (2)
src/UniqueProvider/index.tsx (1)
219-222: 将对齐类合并到 UniqueBody：修复点正确，但请确认行为边界把 alignedClassName 传给 UniqueBody 能补齐过渡/对齐样式缺失的问题，方向对、实现简洁。
同时，这会在未提供 uniqueBgClassName 时“仍然”把 alignedClassName 下发到 UniqueBody（行为扩展）。请确认这是否为预期，避免与既有样式选择器产生意外叠加。tests/unique.test.tsx (1)
215-265: 新增用例覆盖“对齐类 + uniqueBgClassName 合并”行为：到位该用例有效验证了 UniqueBody 同时带上自定义底色类与对齐类，契合修复目标。
|
@gemini-code-assist CR again |
tests/unique.test.tsx
Outdated
| it('should combine alignedClassName with uniqueBgClassName', async () => { | ||
| const getPopupClassNameFromAlign = () => 'bamboo'; | ||
|
|
||
| render( | ||
| <UniqueProvider> | ||
| <Trigger | ||
| action={['click']} | ||
| popup={<strong className="x-content">tooltip</strong>} | ||
| unique | ||
| popupVisible | ||
| popupPlacement="bottomLeft" | ||
| getPopupClassNameFromAlign={getPopupClassNameFromAlign} | ||
| builtinPlacements={{ | ||
| bottomLeft: { | ||
| points: ['tl', 'bl'], | ||
| offset: [0, 4], | ||
| overflow: { | ||
| adjustX: 0, | ||
| adjustY: 1, | ||
| }, | ||
| }, | ||
| }} | ||
| > | ||
| <div className="target">click me</div> | ||
| </Trigger> | ||
| </UniqueProvider>, | ||
| ); | ||
|
|
||
| expect(document.querySelector('.rc-trigger-popup')).toHaveClass('bamboo'); | ||
| expect(document.querySelector('.rc-trigger-popup-unique-body')).toHaveClass( | ||
| 'bamboo', | ||
| ); | ||
| }); |
There was a problem hiding this comment.
这个测试用例的标题是 should combine alignedClassName with uniqueBgClassName，但它目前只验证了 alignedClassName (bamboo) 是否被应用，而没有验证与 uniqueBgClassName 的合并行为。为了让测试更完整并与标题完全对应，建议在 Trigger 组件上添加 uniqueBgClassName 属性，并断言 UniqueBody 同时包含这两个类名。
另外，由于此测试用例中没有使用 await，可以移除 async 关键字。
it('should combine alignedClassName with uniqueBgClassName', () => {
const getPopupClassNameFromAlign = () => 'bamboo';
render(
<UniqueProvider>
<Trigger
action={['click']}
popup={<strong className="x-content">tooltip</strong>}
unique
popupVisible
popupPlacement="bottomLeft"
getPopupClassNameFromAlign={getPopupClassNameFromAlign}
uniqueBgClassName="custom-bg"
builtinPlacements={{
bottomLeft: {
points: ['tl', 'bl'],
offset: [0, 4],
overflow: {
adjustX: 0,
adjustY: 1,
},
},
}}
>
<div className="target">click me</div>
</Trigger>
</UniqueProvider>,
);
const uniqueBody = document.querySelector('.rc-trigger-popup-unique-body');
expect(document.querySelector('.rc-trigger-popup')).toHaveClass('bamboo');
expect(uniqueBody).toHaveClass('bamboo');
expect(uniqueBody).toHaveClass('custom-bg');
});
添加 uniqueBody 丢失的 transition 动画问题
Summary by CodeRabbit