[ISSUE #1952] Actions support custom tooltip - #1973
Conversation
dosubot
Bot
added
documentation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughActions 新增 ChangesActions Tooltip 自定义
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant ActionConfig
participant ActionsItem
participant Item
participant renderWithTooltip
participant Tooltip
ActionConfig->>ActionsItem: 提供 tooltip 配置
ActionsItem->>renderWithTooltip: 传入节点、tooltip、label 和移动端状态
Item->>renderWithTooltip: 传入图标、item.tooltip、label 和移动端状态
renderWithTooltip->>Tooltip: 使用标题或 TooltipProps 包装节点
Tooltip-->>ActionsItem: 返回带 Tooltip 的操作项节点
Possibly related PRs
Suggested labels: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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.
Code Review
This pull request introduces support for a custom tooltip prop in the ActionsItem and Item components, allowing users to customize tooltips with a string, a TooltipProps object, or disable them entirely by passing false. It also includes corresponding updates to documentation, TypeScript interfaces, demos, and unit tests. The review feedback highlights a potential runtime issue where typeof tooltip === 'object' evaluates to true when tooltip is null, which could lead to spreading null into the Tooltip component. Additionally, the reviewer suggests avoiding wrapping elements in an empty Tooltip when both tooltip and label are undefined to prevent unnecessary DOM overhead.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| const tooltipProps = typeof tooltip === 'object' ? tooltip : { title: tooltip ?? label }; | ||
|
|
||
| return isMobile ? innerNode : <Tooltip title={label}>{innerNode}</Tooltip>; | ||
| return isMobile || tooltip === false ? ( | ||
| innerNode | ||
| ) : ( | ||
| <Tooltip {...tooltipProps}>{innerNode}</Tooltip> | ||
| ); |
There was a problem hiding this comment.
In JavaScript, typeof null is 'object'. If tooltip is passed as null, typeof tooltip === 'object' evaluates to true, and tooltipProps becomes null. Spreading null in <Tooltip {...tooltipProps}> can cause runtime errors or TypeScript issues.
Additionally, if both tooltip and label are undefined, wrapping the element in a <Tooltip> with an undefined title is unnecessary and adds extra virtual DOM overhead. We can refine this by checking if tooltip is a non-null object and ensuring we only render the Tooltip when there is a valid tooltip title or configuration.
const tooltipProps = typeof tooltip === 'object' && tooltip !== null ? tooltip : { title: tooltip ?? label };
const hasTooltip =
!isMobile &&
tooltip !== false &&
(typeof tooltip === 'object' && tooltip !== null ? true : !!(tooltip ?? label));
return hasTooltip ? (
<Tooltip {...tooltipProps}>{innerNode}</Tooltip>
) : (
innerNode
);
| const tooltipProps = | ||
| typeof item.tooltip === 'object' ? item.tooltip : { title: item.tooltip ?? item.label }; | ||
| const mergedIconElement = | ||
| isMobile || item.tooltip === false ? ( | ||
| iconElement | ||
| ) : ( | ||
| <Tooltip {...tooltipProps}>{iconElement}</Tooltip> | ||
| ); |
There was a problem hiding this comment.
Similar to ActionsItem.tsx, typeof item.tooltip === 'object' will evaluate to true if item.tooltip is null. Spreading null into <Tooltip> can cause issues. Also, if both item.tooltip and item.label are undefined, we should avoid wrapping the icon in an empty Tooltip to prevent unnecessary DOM and event listener overhead.
We can optimize this by checking for a non-null object and ensuring a tooltip is only rendered when there is a valid title or configuration.
const tooltipProps =
typeof item.tooltip === 'object' && item.tooltip !== null
? item.tooltip
: { title: item.tooltip ?? item.label };
const hasTooltip =
!isMobile &&
item.tooltip !== false &&
(typeof item.tooltip === 'object' && item.tooltip !== null ? true : !!(item.tooltip ?? item.label));
const mergedIconElement = hasTooltip ? (
<Tooltip {...tooltipProps}>{iconElement}</Tooltip>
) : (
iconElement
);
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
packages/x/components/actions/__tests__/action-item.test.tsx (1)
5-13: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win补充
TooltipProps对象类型的测试用例。当前测试覆盖了
string和false两种情况，但缺少tooltip为TooltipProps对象（如{ title: 'Edit', placement: 'bottom' }）的用例。Demo 中已展示此用法，建议补充测试确保对象透传行为正确。💚 建议补充的测试用例
+ it('supports tooltip with TooltipProps object', () => { + const { getByText } = render( + <ActionsItem + defaultIcon="default-icon" + label="Default Tooltip" + tooltip={{ title: 'Object Tooltip', placement: 'bottom' }} + />, + ); + + expect( + getByText('default-icon').closest('[data-tooltip-title="Object Tooltip"]'), + ).toBeInTheDocument(); + });Also applies to: 15-20, 22-30, 32-37
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/x/components/actions/__tests__/action-item.test.tsx` around lines 5 - 13, 在 action-item 测试中补充 tooltip 传入 TooltipProps 对象的用例，例如包含 title 和 placement 的配置，验证对象形式能正确透传并渲染标题，同时保留现有 string 与 false 场景覆盖。packages/x/components/actions/ActionsItem.tsx (1)
147-153: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTooltip 渲染逻辑在两个组件中重复。
ActionsItem.tsx和Item.tsx中tooltipProps的计算和条件渲染逻辑完全一致，共享同一根因。建议提取为共享辅助函数，避免后续维护时两处逻辑产生分歧。
packages/x/components/actions/ActionsItem.tsx#L147-L153: 将tooltipProps计算和条件渲染提取为共享函数。packages/x/components/actions/Item.tsx#L32-L39: 同样使用提取的共享函数替换内联逻辑。♻️ 建议提取共享辅助函数
+// packages/x/components/actions/utils.ts +import type { TooltipProps } from 'antd'; + +export function getTooltipProps( + tooltip: string | TooltipProps | false | undefined, + label?: string, +): TooltipProps { + return typeof tooltip === 'object' ? tooltip : { title: tooltip ?? label }; +} + +export function shouldRenderTooltip( + isMobile: boolean, + tooltip: string | TooltipProps | false | undefined, +): boolean { + return !isMobile && tooltip !== false; +}ActionsItem.tsx 复用：
- const tooltipProps = typeof tooltip === 'object' ? tooltip : { title: tooltip ?? label }; - - return isMobile || tooltip === false ? ( - innerNode - ) : ( - <Tooltip {...tooltipProps}>{innerNode}</Tooltip> - ); + const tooltipProps = getTooltipProps(tooltip, label); + return shouldRenderTooltip(isMobile, tooltip) ? ( + <Tooltip {...tooltipProps}>{innerNode}</Tooltip> + ) : ( + innerNode + );Item.tsx 复用：
- const tooltipProps = - typeof item.tooltip === 'object' ? item.tooltip : { title: item.tooltip ?? item.label }; - const mergedIconElement = - isMobile || item.tooltip === false ? ( - iconElement - ) : ( - <Tooltip {...tooltipProps}>{iconElement}</Tooltip> - ); + const tooltipProps = getTooltipProps(item.tooltip, item.label); + const mergedIconElement = shouldRenderTooltip(isMobile, item.tooltip) ? ( + <Tooltip {...tooltipProps}>{iconElement}</Tooltip> + ) : ( + iconElement + );🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/x/components/actions/ActionsItem.tsx` around lines 147 - 153, 提取共享辅助函数，统一封装 ActionsItem.tsx 中 tooltipProps 的计算及 isMobile 或 tooltip === false 时直接返回 innerNode、否则渲染 Tooltip 的逻辑；在 packages/x/components/actions/ActionsItem.tsx:147-153 和 packages/x/components/actions/Item.tsx:32-39 两处改为调用该辅助函数，保持现有行为一致。
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@packages/x/components/actions/__tests__/action-item.test.tsx`:
- Around line 5-13: 在 action-item 测试中补充 tooltip 传入 TooltipProps 对象的用例，例如包含 title
和 placement 的配置，验证对象形式能正确透传并渲染标题，同时保留现有 string 与 false 场景覆盖。
In `@packages/x/components/actions/ActionsItem.tsx`:
- Around line 147-153: 提取共享辅助函数，统一封装 ActionsItem.tsx 中 tooltipProps 的计算及
isMobile 或 tooltip === false 时直接返回 innerNode、否则渲染 Tooltip 的逻辑；在
packages/x/components/actions/ActionsItem.tsx:147-153 和
packages/x/components/actions/Item.tsx:32-39 两处改为调用该辅助函数，保持现有行为一致。
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 2af2238c-0606-4c08-a50b-6b3b6c256b86
📒 Files selected for processing (8)
packages/x/components/actions/ActionsItem.tsxpackages/x/components/actions/Item.tsxpackages/x/components/actions/__tests__/action-item.test.tsxpackages/x/components/actions/__tests__/index.test.tsxpackages/x/components/actions/demo/basic.tsxpackages/x/components/actions/index.en-US.mdpackages/x/components/actions/index.zh-CN.mdpackages/x/components/actions/interface.ts
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/x/components/actions/tooltip.tsx (1)
11-16: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value建议重命名参数以提升语义明确性
该参数名为
disabled，极易与组件或操作项自身的disabled（不可交互）状态混淆。而实际上，上游调用方（如Item.tsx和ActionsItem.tsx）传入的均是isMobile变量以在移动端屏蔽 Tooltip。建议将其重命名为isMobile或disableTooltip。💡 建议修改为 disableTooltip
export function renderWithTooltip( node: React.ReactElement, tooltip: ActionTooltip, label?: string, - disabled?: boolean, + disableTooltip?: boolean, ) {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/x/components/actions/tooltip.tsx` around lines 11 - 16, Rename the disabled parameter in renderWithTooltip to disableTooltip (or an equivalent tooltip-specific name), and update its internal references plus all callers such as Item.tsx and ActionsItem.tsx to use the new name while preserving the mobile Tooltip-suppression behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/x/components/actions/tooltip.tsx`:
- Around line 23-25: Update the guard in the tooltip rendering logic to treat
only absent or nullish tooltipProps.title values as missing, while preserving
rendering for valid falsy ReactNodes such as 0. Keep returning node for
genuinely missing titles and retain the existing Tooltip path for all provided
titles.
---
Nitpick comments:
In `@packages/x/components/actions/tooltip.tsx`:
- Around line 11-16: Rename the disabled parameter in renderWithTooltip to
disableTooltip (or an equivalent tooltip-specific name), and update its internal
references plus all callers such as Item.tsx and ActionsItem.tsx to use the new
name while preserving the mobile Tooltip-suppression behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 260401f0-9628-4cbb-9937-faa304909882
📒 Files selected for processing (5)
packages/x/components/actions/ActionsItem.tsxpackages/x/components/actions/Item.tsxpackages/x/components/actions/__tests__/action-item.test.tsxpackages/x/components/actions/__tests__/index.test.tsxpackages/x/components/actions/tooltip.tsx
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/x/components/actions/tests/index.test.tsx
- packages/x/components/actions/Item.tsx
- packages/x/components/actions/ActionsItem.tsx
Bundle ReportChanges will increase total bundle size by 8 bytes (0.0%) ⬆️. This is within the configured threshold ✅ Detailed changes
Affected Assets, Files, and Routes:view changes for bundle: antdx-array-pushAssets Changed:
|
kimteayon
left a comment
kimteayon
left a comment
There was a problem hiding this comment.
合法的 TooltipProps.overlay 会被静默忽略
tooltip.tsx:23 仅检查 title。但当前 antd 的 TooltipProps 仍支持 overlay，例如 tooltip={{ overlay: '说明' }} 类型合法且原生 Tooltip 可以展示，此处却直接返回未包装节点。建议同时检查 overlay，或让 antd 自行处理空内容，并补充对应测试。
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1973 +/- ##
=======================================
Coverage 97.06% 97.07%
=======================================
Files 159 160 +1
Lines 5766 5775 +9
Branches 1712 1707 -5
=======================================
+ Hits 5597 5606 +9
Misses 167 167
Partials 2 2 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
@kimteayon over thank you~ |
2 participants
🤔 This is a ...
🔗 Related Issues
Close #1952
💡 Background and Solution
Actions currently uses
labelas the tooltip content by default, but each action item cannot customize or disable its own tooltip.This PR adds a
tooltip?: string | TooltipProps | falseoption forActionsitems andActions.Item.string: use custom tooltip titleTooltipProps: pass custom Tooltip propsfalse: disable tooltipundefined: keep existing behavior and uselabel📝 Change Log
Summary by CodeRabbit
tooltip：支持string、完整 Tooltip 参数（含 placement），或通过tooltip: false禁用提示；未配置时默认使用label。ItemType/Actions.Item的tooltip类型、默认值与禁用说明。title: 0等。tooltip（含禁用）用于展示效果。