feat: adapt semantic close #83
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
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在 DefaultPanel 的关闭按钮上新增对 Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
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 @Arktomson, 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 options for the close button within tour steps. It introduces the ability to apply custom CSS classes and inline styles directly to the close button, and formally recognizes 'close' as a semantic element. This change provides developers with more control over the appearance and behavior of the tour step's close functionality. 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
|
![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 correctly adds support for semantic styling of the close button, allowing className and style customization. The implementation is consistent with existing patterns in the codebase. My main feedback is to add test coverage for this new feature to ensure its correctness and prevent future regressions.
| className={classNames(`${prefixCls}-close`, tourClassNames?.close)} | ||
| style={styles?.close} |
There was a problem hiding this comment.
While the implementation for applying custom className and style to the close button is correct, this pull request is missing corresponding tests to verify the new functionality. Please add a test case to ensure that tourClassNames.close and styles.close are applied as expected. You could extend the existing 'support custom styles' test in tests/index.test.tsx for this.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #83 +/- ##
=======================================
Coverage 99.07% 99.07%
=======================================
Files 10 10
Lines 217 217
Branches 96 98 +2
=======================================
Hits 215 215
Misses 2 2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| aria-label="Close" | ||
| {...ariaProps} | ||
| className={`${prefixCls}-close`} | ||
| className={classNames(`${prefixCls}-close`, tourClassNames?.close)} |
|
ok
…---Original---
From: ***@***.***>
Date: Sat, Sep 27, 2025 11:27 AM
To: ***@***.***>;
Cc: "sheng ***@***.******@***.***>;
Subject: Re: [react-component/tour] feat: adapt semantic close (PR #83)
@coding-ice commented on this pull request.
In src/TourStep/DefaultPanel.tsx:
> @@ -41,7 +41,8 @@ export default function DefaultPanel(props: DefaultPanelProps) { onClick={onClose} aria-label="Close" {...ariaProps} - className={`${prefixCls}-close`} + className={classNames(`${prefixCls}-close`, tourClassNames?.close)}
测试用例需要补一下
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you were mentioned.Message ID: ***@***.***>
|
|
同步一下上游分支 |
… arktomson/semantic-close
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
| className={classNames(`${prefixCls}-close`, tourClassNames?.close)} | ||
| style={styles?.close} |
There was a problem hiding this comment.
🧩 Analysis chain
验证测试覆盖是否已补充。
过往评审中 coding-ice 和 gemini-code-assist[bot] 都提到需要补充测试用例来验证 tourClassNames.close 和 styles.close 的应用。虽然 AI 摘要提到测试已扩展，但建议确认 tests/index.test.tsx 中是否包含了针对关闭按钮的类名和样式的断言。
运行以下脚本验证测试覆盖：
🏁 Script executed:
#!/bin/bash
# 描述：验证 tests/index.test.tsx 中是否包含对 close 按钮样式的测试
# 搜索测试文件中与 close 相关的断言
rg -n -A 5 -B 5 "tourClassNames.*close|styles.*close|\.close" tests/index.test.tsxLength of output: 361
补充关闭按钮类名和样式测试
在 tests/index.test.tsx 中缺少对 DefaultPanel 关闭按钮 className={classNames(..., tourClassNames?.close)} 和 style={styles?.close} 的断言，请添加测试以验证 tourClassNames.close 与 styles.close 的正确应用。
🤖 Prompt for AI Agents
In src/TourStep/DefaultPanel.tsx around lines 43-44, the close button receives
className={classNames(`${prefixCls}-close`, tourClassNames?.close)} and
style={styles?.close} but tests don't assert these; update tests/index.test.tsx
to render DefaultPanel with explicit tourClassNames={{ close: 'my-close' }} and
styles={{ close: { color: 'red' } }} (or similar), locate the close button (by
role/button, aria-label, or the generated class), and add assertions that the
element's classList contains the provided tourClassNames.close and that its
style matches the provided styles.close (use toContain/toHaveClass for class and
toHaveStyle or equivalent for inline styles).
| aria-label="Close" | ||
| {...ariaProps} | ||
| className={`${prefixCls}-close`} | ||
| className={classNames(`${prefixCls}-close`, tourClassNames?.close)} |
There was a problem hiding this comment.
关键错误：使用了未定义的 classNames 函数。
第 43 行使用了 classNames 函数，但文件顶部导入的是 clsx（第 3 行）。这会导致运行时错误 ReferenceError: classNames is not defined。文件中其他所有地方都正确使用了 clsx（第 32、34、50、54、61、70、86 行）。
应用此 diff 修复：
- className={classNames(`${prefixCls}-close`, tourClassNames?.close)}
+ className={clsx(`${prefixCls}-close`, tourClassNames?.close)}📝 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.
| className={classNames(`${prefixCls}-close`, tourClassNames?.close)} | |
| className={clsx(`${prefixCls}-close`, tourClassNames?.close)} |
🤖 Prompt for AI Agents
In src/TourStep/DefaultPanel.tsx around line 43, the code calls an undefined
classNames function (causing ReferenceError) while the module imports clsx;
replace the usage of classNames with clsx at line 43 so it matches the rest of
the file (or alternatively add an import that exposes classNames), ensuring the
close button className uses clsx(`${prefixCls}-close`, tourClassNames?.close).
Summary by CodeRabbit
新功能
测试