feat: support disabled prop#534
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
WalkthroughTooltip 现在支持 ChangesTooltip disabled 支持
Estimated code review effort: 2 (Simple) | ~10 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant DisabledDemo
participant Tooltip
participant Trigger
participant TooltipDOM
User->>DisabledDemo: 点击按钮切换 disabled
DisabledDemo->>Tooltip: 传入新的 disabled 状态
Tooltip->>Trigger: 传递 disabled 属性
Trigger->>TooltipDOM: 添加或移除 rc-tooltip-hidden
Possibly related PRs
Poem
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning Tools execution failed with the following error: Failed to run tools: 13 INTERNAL: Received RST_STREAM with code 2 (Internal server error) 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 a 'disabled' prop to the 'Tooltip' component, allowing it to be temporarily hidden without resetting its visibility state, and updates the '@rc-component/trigger' dependency. It also adds a demo, documentation, and a test case. The review feedback suggests improving accessibility compliance by ensuring that the 'aria-describedby' attribute is not applied to the trigger element when the tooltip is disabled, and recommends adding corresponding assertions to the test suite.
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.
| TriggerProps, | ||
| | 'onPopupAlign' | ||
| | 'builtinPlacements' | ||
| | 'disabled' |
There was a problem hiding this comment.
When the disabled prop is true, the tooltip is temporarily hidden, but its internal open state is not reset (as described in the PR description). This means open remains true, which causes aria-describedby to still be applied to the trigger element in getChildren.
To ensure proper accessibility (ARIA) compliance, we should prevent aria-describedby from being set when the tooltip is disabled.
We can achieve this by:
- Destructuring
disabledfrompropsinTooltip.tsx. - Explicitly passing
disabledto<Trigger>. - Updating
getChildrento check!disabledbefore settingaria-describedby.
Here is how the implementation would look:
// 1. Destructure `disabled` from props (around line 83)
const {
// ... other props
disabled,
...restProps
} = props;
// 2. Update getChildren (around line 117)
const getChildren: TriggerProps['children'] = ({ open }) => {
const child = React.Children.only(children);
const ariaProps: React.AriaAttributes = {
'aria-describedby': overlay && open && !disabled ? mergedId : undefined,
};
return React.cloneElement(child, ariaProps);
};
// 3. Pass disabled to Trigger (around line 158)
<Trigger
// ... other props
disabled={disabled}
{...extraProps}
>| fireEvent.mouseEnter(button); | ||
| expect(container.querySelector('.rc-tooltip')).not.toHaveClass('rc-tooltip-hidden'); | ||
|
|
||
| fireEvent.click(button); | ||
| expect(container.querySelector('.rc-tooltip')).toHaveClass('rc-tooltip-hidden'); | ||
|
|
||
| fireEvent.click(button); | ||
| expect(container.querySelector('.rc-tooltip')).not.toHaveClass('rc-tooltip-hidden'); |
There was a problem hiding this comment.
To verify that the accessibility attributes (aria-describedby) are correctly managed when the tooltip is disabled, we should add assertions for aria-describedby in this test case.
| fireEvent.mouseEnter(button); | |
| expect(container.querySelector('.rc-tooltip')).not.toHaveClass('rc-tooltip-hidden'); | |
| fireEvent.click(button); | |
| expect(container.querySelector('.rc-tooltip')).toHaveClass('rc-tooltip-hidden'); | |
| fireEvent.click(button); | |
| expect(container.querySelector('.rc-tooltip')).not.toHaveClass('rc-tooltip-hidden'); | |
| fireEvent.mouseEnter(button); | |
| expect(container.querySelector('.rc-tooltip')).not.toHaveClass('rc-tooltip-hidden'); | |
| expect(button).toHaveAttribute('aria-describedby'); | |
| fireEvent.click(button); | |
| expect(container.querySelector('.rc-tooltip')).toHaveClass('rc-tooltip-hidden'); | |
| expect(button).not.toHaveAttribute('aria-describedby'); | |
| fireEvent.click(button); | |
| expect(container.querySelector('.rc-tooltip')).not.toHaveClass('rc-tooltip-hidden'); | |
| expect(button).toHaveAttribute('aria-describedby'); |
zombieJ
force-pushed
the
codex/tooltip-disabled
branch
from
4fc8a63 to
2b32efb
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #534 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 3 3
Lines 36 36
Branches 14 14
=========================================
Hits 36 36 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
zombieJ
force-pushed
the
codex/tooltip-disabled
branch
from
2b32efb to
09f8f57
Compare
|
React Doctor found no new issues. 🎉 Reviewed by React Doctor for commit |
❌ Deploy failed
📋 Build log (last lines)🤖 Powered by surge-preview |
|||||||||
1 participant
Summary
disabledthroughTooltipPropsand pass it to Trigger@rc-component/trigger@^3.10.0Behavior
Hovering opens the Tooltip. Toggling
disabledhides it without resetting the current open state, so enabling it again restores the Tooltip without another mouse enter.Related: react-component/trigger#638
Validation
ut test --runInBand(2 suites, 28 tests)./node_modules/.bin/tsc --noEmitut lint(no errors; one existing warning)ut compileut docs:buildut x prettier@3.6.2 --check package.json src/Tooltip.tsx tests/index.test.tsx docs/examples/disabled.tsx docs/demo/disabled.mdSummary by CodeRabbit
disabled属性，可临时隐藏提示内容，并支持重新启用。disabled参数的说明，包括类型和默认值。