-
Notifications
You must be signed in to change notification settings - Fork 4
feat: tooltip #6
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
Conversation
WalkthroughA new Tooltip component is introduced to the Vue 3 UI library with full implementation: type definitions, a TypeScript+JSX component leveraging Floating UI for positioning, SCSS styling with theme variants and animations, comprehensive test coverage, and API documentation. Floating UI dependencies are added to support tooltip positioning logic. Changes
Sequence DiagramsequenceDiagram
actor User
participant Trigger as Trigger Element
participant Tooltip as Tooltip Component
participant FloatingUI as Floating UI
participant Popper as Popper/Content
User->>Trigger: Interact (hover/click/focus)
Trigger->>Tooltip: Emit trigger event
Tooltip->>Tooltip: Cancel hide timer
Tooltip->>Tooltip: Start show timer
Note over Tooltip: showAfter delay
Tooltip->>Tooltip: Emit before-show
Tooltip->>FloatingUI: Compute position
FloatingUI->>Tooltip: Return position & arrow
Tooltip->>Popper: Update position & styles
Tooltip->>Tooltip: Emit show
Popper->>Popper: Fade in animation
User->>Trigger: Leave trigger (hover/blur)
Trigger->>Tooltip: Emit leave event
Tooltip->>Tooltip: Start hide timer
Note over Tooltip: hideAfter delay
Tooltip->>Tooltip: Emit before-hide
Tooltip->>Tooltip: Emit hide
Popper->>Popper: Fade out animation
Tooltip->>Tooltip: Emit update:visible
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 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 |
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: 3
🧹 Nitpick comments (2)
packages/ccui/ui/tooltip/src/tooltip.tsx (2)
47-68: Unsafe non-null assertion on staticSide.Line 66 uses a non-null assertion (
staticSide!) when accessing the computedstaticSideas an object key. WhilestaticSideshould always be defined when the arrow exists, the type system doesn't guarantee this.Consider adding an explicit check:
const { x, y } = middlewareData.value.arrow const staticSide = { top: 'bottom', right: 'left', bottom: 'top', left: 'right', }[props.placement.split('-')[0]] + + if (!staticSide) return {} return { left: x != null ? `${x}px` : '', top: y != null ? `${y}px` : '', right: '', bottom: '', - [staticSide!]: '-4px', + [staticSide]: '-4px', }
120-126: Document why click trigger bypasses hideAfter delay.Line 120 specifically excludes the click trigger from the
hideAfterdelay, meaning click-triggered tooltips always hide immediately. This is a design decision that differs from hover/focus behavior.Consider adding a comment to explain this choice:
const hideTooltip = () => { emit('before-hide') if (!isControlled.value) { visible.value = false } emit('update:visible', false) emit('hide') } + // Click triggers hide immediately for better UX; hover/focus use hideAfter delay if (props.hideAfter > 0 && props.trigger !== 'click') {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (8)
.codeflicker/specs/tooltip-component-development.md(1 hunks)packages/ccui/package.json(1 hunks)packages/ccui/ui/tooltip/index.ts(1 hunks)packages/ccui/ui/tooltip/src/tooltip-types.ts(1 hunks)packages/ccui/ui/tooltip/src/tooltip.scss(1 hunks)packages/ccui/ui/tooltip/src/tooltip.tsx(1 hunks)packages/ccui/ui/tooltip/test/tooltip.test.ts(1 hunks)packages/docs/components/tooltip/index.md(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
packages/ccui/ui/tooltip/src/tooltip.tsx (2)
packages/ccui/ui/tooltip/src/tooltip-types.ts (2)
tooltipProps(27-95)TooltipProps(97-97)packages/ccui/ui/shared/hooks/use-namespace.ts (1)
useNamespace(30-44)
🔇 Additional comments (11)
packages/ccui/ui/tooltip/test/tooltip.test.ts (1)
1-487: Great coverage on tooltip interactions.Appreciate how these tests walk through every trigger mode, placement, delay, and a11y hook—it gives a lot of confidence in the new component.
packages/ccui/ui/tooltip/src/tooltip.scss (3)
38-46: Verify arrow z-index behavior in complex stacking contexts.The arrow uses
z-index: -1to position it behind the tooltip content. While this is a common pattern, it may cause the arrow to disappear behind other positioned elements or in certain stacking contexts.Consider testing the tooltip in scenarios with:
- Parent elements with
z-indexortransformproperties (which create new stacking contexts)- Overlapping tooltips or other floating elements
- Elements with backdrop filters or complex layering
If issues arise, an alternative approach is to use
overflow: visibleon the popper and position the arrow with careful clipping, or set the arrow's z-index relative to the content layer rather than -1.
60-68: Verify keyboard accessibility for disabled tooltips.The disabled state uses
pointer-events: noneon the trigger, which prevents mouse events but doesn't prevent keyboard focus. If the trigger element is naturally focusable (button, link, etc.), keyboard users could still focus it but not activate the tooltip.Consider adding
tabindex="-1"to disabled triggers in the component logic to fully prevent keyboard interaction, or ensure the component handles the disabled state in the trigger's tabindex attribute.
70-78: LGTM - Mobile-friendly responsive design.The responsive adjustments appropriately increase font size for better readability on mobile devices and prevent viewport overflow with a sensible max-width constraint.
packages/ccui/ui/tooltip/src/tooltip-types.ts (1)
27-97: LGTM - Well-structured props definition.The tooltip props are well-organized with logical grouping, sensible defaults, and proper TypeScript typing. The use of
as constensures accurate type inference for ExtractPropTypes.packages/ccui/ui/tooltip/src/tooltip.tsx (6)
1-13: LGTM - Clean component setup.The imports are well-organized, and the component is properly defined with typed props and comprehensive event emitters.
15-34: LGTM - Well-implemented state management.The controlled/uncontrolled visibility pattern is correctly implemented, and the dynamic class computation properly uses the BEM namespace utility.
128-174: LGTM - Event handlers correctly implement trigger modes.The event handlers properly gate behavior by trigger type and correctly implement the enterable feature for hover interactions.
176-211: LGTM - Proper lifecycle and positioning management.The component correctly manages autoUpdate lifecycle, cleaning up when the tooltip is hidden and recreating when shown. The watchers properly handle both controlled and uncontrolled visibility states.
257-266: Verify tabindex behavior with naturally focusable elements.Line 262 sets
tabindex={0}when the trigger is 'focus', which ensures the trigger can receive keyboard focus. However, if the trigger element is already naturally focusable (e.g., a<button>or<input>), this might be redundant.Consider testing the component with:
- Naturally focusable elements (button, input, link) as triggers
- Non-focusable elements (div, span) as triggers
If the trigger is already focusable, the explicit
tabindex="0"is harmless but redundant. If needed, you could check the trigger element's natural focusability, but the current approach is simpler and safer (erring on the side of always ensuring focusability).
268-288: LGTM - Accessible and well-structured tooltip rendering.The popper rendering correctly uses ARIA attributes (
role="tooltip",idlinked toaria-describedby), dynamic positioning styles from Floating UI, and properly handles the enterable behavior withpointerEvents.
| export interface TooltipEmits { | ||
| 'show': () => void | ||
| 'hide': () => void | ||
| 'update:visible': (visible: boolean) => void | ||
| } |
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.
Remove duplicate TooltipEmits interface declaration.
The TooltipEmits interface is declared twice in this file (lines 21-25 and lines 99-110), which will cause a TypeScript compilation error. The second declaration at lines 99-110 is more complete with additional events and JSDoc comments.
Apply this diff to remove the duplicate:
-export interface TooltipEmits {
- 'show': () => void
- 'hide': () => void
- 'update:visible': (visible: boolean) => void
-}
-
export const tooltipProps = {📝 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.
| export interface TooltipEmits { | |
| 'show': () => void | |
| 'hide': () => void | |
| 'update:visible': (visible: boolean) => void | |
| } | |
| export const tooltipProps = { |
🤖 Prompt for AI Agents
In packages/ccui/ui/tooltip/src/tooltip-types.ts around lines 21 to 25, there is
a duplicate, minimal TooltipEmits interface that conflicts with the more
complete declaration at lines 99 to 110; remove the earlier duplicate
declaration (lines 21-25) so only the full, documented TooltipEmits interface at
lines 99-110 remains, and run TypeScript build to confirm no missing references.
| // 使用 floating-ui 进行位置计算 | ||
| const { floatingStyles, middlewareData, update } = useFloating(triggerRef, popperRef, { | ||
| placement: props.placement as any, | ||
| middleware: [ | ||
| offset(props.offset), | ||
| flip(), | ||
| shift({ padding: 8 }), | ||
| ...(props.showArrow ? [arrow({ element: arrowRef })] : []), | ||
| ], | ||
| }) |
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.
🛠️ Refactor suggestion | 🟠 Major
Remove unsafe type assertion.
Line 38 uses as any to cast props.placement, which bypasses TypeScript's type checking. The TooltipPlacement type should be compatible with Floating UI's Placement type.
Apply this diff:
- const { floatingStyles, middlewareData, update } = useFloating(triggerRef, popperRef, {
- placement: props.placement as any,
+ const { floatingStyles, middlewareData, update } = useFloating(triggerRef, popperRef, {
+ placement: props.placement,
middleware: [If there's a type mismatch, consider importing and using Floating UI's Placement type in tooltip-types.ts or adding a proper type constraint.
📝 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.
| // 使用 floating-ui 进行位置计算 | |
| const { floatingStyles, middlewareData, update } = useFloating(triggerRef, popperRef, { | |
| placement: props.placement as any, | |
| middleware: [ | |
| offset(props.offset), | |
| flip(), | |
| shift({ padding: 8 }), | |
| ...(props.showArrow ? [arrow({ element: arrowRef })] : []), | |
| ], | |
| }) | |
| // 使用 floating-ui 进行位置计算 | |
| const { floatingStyles, middlewareData, update } = useFloating(triggerRef, popperRef, { | |
| placement: props.placement, | |
| middleware: [ | |
| offset(props.offset), | |
| flip(), | |
| shift({ padding: 8 }), | |
| ...(props.showArrow ? [arrow({ element: arrowRef })] : []), | |
| ], | |
| }) |
🤖 Prompt for AI Agents
In packages/ccui/ui/tooltip/src/tooltip.tsx around lines 36 to 45, remove the
unsafe `as any` cast on `props.placement` and instead supply a value typed as
Floating UI's Placement; to fix, import the Placement type from
@floating-ui/core (or @floating-ui/react) and update the TooltipPlacement
definition (or the prop type) in tooltip-types.ts to extend/alias Floating UI's
Placement, then use `props.placement` directly (no `as any`) when calling
useFloating so the compiler enforces compatibility.
| const renderContent = () => { | ||
| if (props.rawContent) { | ||
| return <div innerHTML={props.content}></div> | ||
| } | ||
|
|
||
| if (slots.content) { | ||
| return slots.content() | ||
| } | ||
|
|
||
| return props.content | ||
| } |
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.
Document security implications of rawContent prop.
Line 229 uses innerHTML to render raw HTML content when rawContent is true, which creates an XSS vulnerability if untrusted content is provided.
Recommendations:
- Add a clear JSDoc comment to the
rawContentprop intooltip-types.tswarning that it should only be used with trusted, sanitized content - Consider adding a development-mode warning when
rawContentis enabled - Document this security consideration in the component's API documentation
Example JSDoc addition:
// In tooltip-types.ts
rawContent: {
type: Boolean,
default: false,
// Add JSDoc:
/**
* Whether to render content as raw HTML.
* ⚠️ WARNING: Only use with trusted, sanitized content to prevent XSS attacks.
*/
},🤖 Prompt for AI Agents
In packages/ccui/ui/tooltip/src/tooltip.tsx around lines 227-237 the component
uses innerHTML when props.rawContent is true which creates an XSS risk for
untrusted content; update tooltip-types.ts to add a JSDoc on the rawContent prop
explicitly warning it must only be used with trusted/sanitized content, add a
runtime development-only warning (e.g., console.warn) in the tooltip component
to emit a clear message when rawContent is enabled, and document the security
caveat in the component API docs; ensure the JSDoc text matches the suggested
wording and the runtime warning only runs in non-production environments.
📋 概述
本 PR 完成了 Tooltip 组件的全面开发和优化,使用现代化的 floating-ui 库替代自定义位置计算,提供了更精确、稳定的浮层定位功能。
✨ 主要功能
🔧 核心特性
🎨 组件特性
🔄 技术改进
📦 依赖升级
{ "@floating-ui/vue": "^1.0.0", "@floating-ui/dom": "^1.0.0" }🏗️ 架构优化
autoUpdate自动监听位置变化,减少手动计算开销🎯 样式优化
useNamespacehook 确保一致的 CSS 类名🧪 测试覆盖
✅ 测试用例(22个)
📊 覆盖率
📚 文档完善
📖 API 文档
🎮 交互示例
🚀 性能优化
📈 性能提升
🔄 开发体验
📋 文件变更
🆕 新增文件
packages/ccui/ui/tooltip/- 完整的 Tooltip 组件实现packages/docs/components/tooltip/index.md- 组件文档和示例🔧 核心文件
src/tooltip.tsx- 组件主要实现(使用 floating-ui)src/tooltip-types.ts- TypeScript 类型定义src/tooltip.scss- 优化后的样式文件test/tooltip.test.ts- 全面的测试用例📦 依赖更新
packages/ccui/package.json- 添加 floating-ui 依赖Summary by CodeRabbit
New Features
Documentation