-
-
Notifications
You must be signed in to change notification settings - Fork 244
feat: adapt semantic close #915
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Summary of ChangesHello @li-jia-nan, 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 implements a new feature that provides enhanced control over the visual presentation of tab components. It refactors the styling approach by introducing semantic names for different parts of a tab, such as the main item and its close button, allowing developers to apply distinct class names and styles more precisely. This change involves updating component props to accept structured style objects rather than single string/object values. Additionally, a minor adjustment was made to the tab navigation logic to ensure the currently selected tab is always brought into view. 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 #915 +/- ##
==========================================
- Coverage 98.98% 98.98% -0.01%
==========================================
Files 18 18
Lines 789 788 -1
Branches 237 233 -4
==========================================
- Hits 781 780 -1
Misses 8 8 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces a 'semantic close' feature, allowing for more granular styling of the close button on tabs by adding 'close' to SemanticName
and plumbing through classNames.close
and styles.close
to the TabNode
component. The changes are well-structured and consistently applied across the affected files. I've included one suggestion to improve the maintainability of how styles are passed down to child components.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
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本次变更将 Tab 系列组件的样式接口改为语义化 Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Nav as TabNavList
participant Tab as TabNode
participant Op as OperationNode
participant DOM as DOM (scroll/overlay)
Note over Nav,Tab: 样式/类名以语义组传递
Nav->>Tab: props { classNames, styles, ... }
Nav->>Op: props { classNames, styles, ... }
Note over Op,DOM #D3F8E2: 选中项变化触发滚动(依赖 selectedItemId)
Op->>Op: compute selectedItemId (from selectedKey + popupId)
Op->>DOM: scrollIntoView(selectedItemId) (when selectedItemId or selectedKey changes)
Op->>DOM: open overlay with overlayStyle = styles?.popup
Op->>DOM: remove button uses class = classNames?.close, style = styles?.close
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used🧬 Code graph analysis (1)src/TabNavList/TabNode.tsx (1)
🔇 Additional comments (7)
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/TabNavList/OperationNode.tsx (1)
164-171
: 优化 Effect 依赖与空值保护仅依赖 selectedItemId 即可,且在其为空时应短路,避免 getElementById(null) 的无效查询与多余重渲染。
- useEffect(() => { - // We use query element here to avoid React strict warning - const ele = document.getElementById(selectedItemId); - if (ele && ele.scrollIntoView) { - ele.scrollIntoView(false); - } - }, [selectedItemId, selectedKey]); + useEffect(() => { + if (!selectedItemId) return; + // We use query element here to avoid React strict warning + const ele = document.getElementById(selectedItemId); + ele?.scrollIntoView?.(false); + }, [selectedItemId]);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/TabNavList/OperationNode.tsx
(1 hunks)src/TabNavList/TabNode.tsx
(5 hunks)src/TabNavList/index.tsx
(1 hunks)src/Tabs.tsx
(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/TabNavList/TabNode.tsx (1)
src/Tabs.tsx (1)
SemanticName
(38-38)
🔇 Additional comments (2)
src/TabNavList/index.tsx (1)
438-442
: 完整透传 styles,仅覆写 item,做法正确这样保持与 classNames 的一致性,并为未来扩展留余地;同时用户样式优先级覆盖内置间距,语义清晰。
src/TabNavList/TabNode.tsx (1)
84-94
: 语义化样式接入到位
- 根节点合并 classNames.item 与 styles.item
- 移除按钮使用 classNames.close 与 styles.close
实现与扩展后的 SemanticName 完整对齐,样式优先级与可定制性良好。
Also applies to: 128-136
Summary by CodeRabbit