feat(tooltip): use the template syntax and rewrite the tooltip component. #3449
Conversation
WalkthroughThis update introduces a new, feature-rich Vue 3 tooltip component with extensive customization options, dynamic template selection, and renderless logic for tooltip state management. Supporting utilities like Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ReferenceElement
participant TooltipComponent
participant RenderlessLogic
participant Popper
participant useTimer
User->>ReferenceElement: mouseenter/mouseleave
ReferenceElement->>TooltipComponent: emit event
TooltipComponent->>RenderlessLogic: handleRefEvent(type)
RenderlessLogic->>useTimer: start/cancel show/hide timer
useTimer-->>RenderlessLogic: timer fires
RenderlessLogic->>TooltipComponent: update showPopper state
TooltipComponent->>Popper: show/hide tooltip (with transition)
User->>Popper: mouseenter/mouseleave
Popper->>TooltipComponent: emit event
TooltipComponent->>RenderlessLogic: handlePopEvent(type)
RenderlessLogic->>useTimer: manage hide timer
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
packages/renderless/src/tooltip/new-vue.tsOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the plugin "eslint-plugin-vue". (The package "eslint-plugin-vue" was not found when loaded as a Node module from the directory "".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following: The plugin "eslint-plugin-vue" was referenced from the config file in ".eslintrc.js » @antfu/eslint-config » @antfu/eslint-config-vue". If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team. packages/vue/src/tooltip/src/index.tsOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the plugin "eslint-plugin-vue". (The package "eslint-plugin-vue" was not found when loaded as a Node module from the directory "".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following: The plugin "eslint-plugin-vue" was referenced from the config file in ".eslintrc.js » @antfu/eslint-config » @antfu/eslint-config-vue". If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team. packages/vue/src/tooltip/src/tooltip.vueOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the plugin "eslint-plugin-vue". (The package "eslint-plugin-vue" was not found when loaded as a Node module from the directory "".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following: The plugin "eslint-plugin-vue" was referenced from the config file in ".eslintrc.js » @antfu/eslint-config » @antfu/eslint-config-vue". If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team.
Note ⚡️ AI Code Reviews for VS Code, Cursor, WindsurfCodeRabbit now has a plugin for VS Code, Cursor and Windsurf. This brings AI code reviews directly in the code editor. Each commit is reviewed immediately, finding bugs before the PR is raised. Seamless context handoff to your AI code agent ensures that you can easily incorporate review feedback. Note ⚡️ Faster reviews with cachingCodeRabbit now supports caching for code and dependencies, helping speed up reviews. This means quicker feedback, reduced wait times, and a smoother review experience overall. Cached data is encrypted and stored securely. This feature will be automatically enabled for all accounts on May 30th. To opt out, configure ✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Walkthrough此PR重写了tooltip组件，使用模板语法以提高可维护性和功能扩展性。引入了新的工具函数和组件，优化了事件处理逻辑，增强了组件的灵活性和性能。 Changes
|
![petercat-assistant[bot]](https://avatars.githubusercontent.com/in/875736?s=60&v=4){kind=small}
| (isShow: boolean) => { | ||
| // 智能识别模式 | ||
| if (props.visible === 'auto' && state.referenceElm) { | ||
| const { clientWidth, scrollWidth } = state.referenceElm.firstElementChild |
There was a problem hiding this comment.
Ensure state.referenceElm.firstElementChild is not null before accessing its properties to avoid potential runtime errors.
| } | ||
| }) | ||
|
|
||
| vm.$on('tooltip-update', updatePopper()) |
There was a problem hiding this comment.
Ensure updatePopper() is a function and is correctly invoked to avoid potential runtime errors.
WalkthroughThis PR rewrites the tooltip component, using template syntax for improved maintainability and functional scalability. New tool functions and components are introduced, event processing logic is optimized, and component flexibility and performance are enhanced. Changes
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (2)
packages/vue/src/tooltip/src/index.ts (1)
3-10: Dynamic template selection implementedThe code now supports dynamic selection between PC and mobile-first tooltip components, replacing the previous virtual template import mechanism. This provides more flexibility for different UI requirements.
Consider translating the Chinese comment "新PC, 老 MF" to English for better international team collaboration.
-const template = (mode) => (mode === 'pc' ? TooltipPc : TooltipMf) // 新PC, 老 MF +const template = (mode) => (mode === 'pc' ? TooltipPc : TooltipMf) // New PC, old Mobile-Firstpackages/renderless/src/tooltip/new-vue.ts (1)
73-74: Remove empty teardown hook
onUnmounted(() => {})is a no-op. Either calldoDestroy()to clean Popper
resources or delete the hook to avoid dead code.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
examples/sites/src/main.js(1 hunks)packages/renderless/src/tooltip/new-index.ts(1 hunks)packages/renderless/src/tooltip/new-vue.ts(1 hunks)packages/vue-hooks/index.ts(1 hunks)packages/vue-hooks/src/useTimer.ts(1 hunks)packages/vue/src/tooltip/src/index.ts(1 hunks)packages/vue/src/tooltip/src/tooltip.vue(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: PR E2E Test (pnpm test:e2e3)
🔇 Additional comments (6)
examples/sites/src/main.js (1)
15-15: CSS to LESS migration: ApproveThe import statement has been changed from 'custom-markdown.css' to 'custom-markdown.less', which is consistent with using LESS preprocessor for styling instead of plain CSS.
packages/vue-hooks/src/useTimer.ts (1)
1-36: Well-implemented timer utility with proper cleanupThis is a well-designed Vue hook that provides setTimeout/debounce functionality with automatic cleanup. The implementation:
- Prevents memory leaks by clearing timers on component unmount
- Handles reactive delay values
- Provides clear control methods with good examples
- Follows Vue Composition API best practices
packages/vue-hooks/index.ts (1)
24-24: Export added for new useTimer hookCorrectly exports the new useTimer hook, making it available for use across the project.
packages/vue/src/tooltip/src/index.ts (1)
104-104:❓ Verification inconclusive
Template function now passed to setup
The change enables dynamic template selection based on the mode parameter while maintaining the same component API.
Verify that the mode parameter is properly passed to this function in all tooltip usage locations. You might want to check how other components are determining which mode to use.
🏁 Script executed:
#!/bin/bash # Find where the tooltip component is being imported and used rg -A 3 "import.*Tooltip" --type ts --type vueLength of output: 76
🏁 Script executed:
#!/bin/bash # Search for Tooltip imports in TS and Vue files rg -A 3 "import .*Tooltip" -g '*.ts' -g '*.vue' # Search for Tooltip component usage in Vue templates rg -A 3 "<Tooltip" -g '*.vue'Length of output: 31792
🏁 Script executed:
#!/bin/bash # Inspect props definition, setup function, template selection, and mode usage rg -n -C3 "props:" packages/vue/src/tooltip/src/index.ts rg -n -C3 "setup(" packages/vue/src/tooltip/src/index.ts rg -n -C3 "return.*\\$setup" packages/vue/src/tooltip/src/index.ts rg -n -C3 "mode" packages/vue/src/tooltip/src/index.tsLength of output: 1012
🏁 Script executed:
#!/bin/bash # Show top of Vue Tooltip index.ts to inspect imports and $props usage sed -n '1,30p' packages/vue/src/tooltip/src/index.ts # Locate $props import and usage rg -n "\$props" -C3 packages/vue/src/tooltip/src/index.ts # Show top of renderless Tooltip index.ts to inspect exported props definition sed -n '1,200p' packages/renderless/src/tooltip/index.tsLength of output: 6378
Verify that
modeprop is actually passed through to the Tooltip templateEnsure that the common
$propsspread includes amodeproperty and that when calling$setup({ props, context, template }), themodevalue is forwarded into thetemplate(mode)function. In particular:
- packages/vue/src/tooltip/src/index.ts
const template = (mode) => …(around line 10)return $setup({ props, context, template })(line 104)- Review all
<Tooltip>usages in your .vue files to confirm whether:
- A
modeprop is set explicitly (e.g.<Tooltip mode="pc" …>), or- The default
modefrom$propscorrectly resolves (and doesn’t default to mobile unexpectedly).Please manually verify that every Tooltip consumer receives the intended mode.
packages/renderless/src/tooltip/new-vue.ts (1)
2-2: Suspicious typo:userPoppervs.usePopperMost Tiny-Vue code and the original Popper composable are named
usePopper.
If this really is a typo, both the import and the call on line 14 will throw at runtime.-import { userPopper, useTimer } from '@opentiny/vue-hooks' +import { usePopper, useTimer } from '@opentiny/vue-hooks'…and later:
-const { showPopper, updatePopper, ... } = userPopper({ +const { showPopper, updatePopper, ... } = usePopper({Please double-check the actual export name in
@opentiny/vue-hooks.
IfuserPopperis indeed intentional, feel free to ignore.packages/vue/src/tooltip/src/tooltip.vue (1)
15-24:disabledis not in component scopeTemplate uses
!disabledbutdisabledis a prop that is not returned by
setup(...). Make sure the helpersetup(...)proxies all props or return it
explicitly; otherwise Vue will warn “disabledis not defined”.
| export const api = ['state', 'handlePopEvent', 'handleRefEvent'] | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
toggleShow omitted from the public API list
tooltip.vue relies on the list in export const api to expose methods returned by
setup(...). toggleShow is created on line 52 but not exported in the array
declared on line 5, making it inaccessible to downstream consumers.
-export const api = ['state', 'handlePopEvent', 'handleRefEvent']
+export const api = [
+ 'state',
+ 'handlePopEvent',
+ 'handleRefEvent',
+ 'toggleShow'
+]Also applies to: 43-53
🤖 Prompt for AI Agents
In packages/renderless/src/tooltip/new-vue.ts around lines 5 to 6 and 43 to 53,
the method toggleShow is defined but not included in the exported api array,
which limits its accessibility to downstream consumers. To fix this, add
'toggleShow' to the api array declared on line 5 so it is properly exposed as
part of the public API.
| } | ||
| }) | ||
|
|
||
| vm.$on('tooltip-update', updatePopper()) |
There was a problem hiding this comment.
Event handler is executed immediately, not on event dispatch
vm.$on('tooltip-update', updatePopper()) invokes updatePopper right away and
registers its return value (likely undefined) as the listener.
-vm.$on('tooltip-update', updatePopper())
+vm.$on('tooltip-update', updatePopper)This prevents the popper from repositioning when the event is emitted.
📝 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.
| vm.$on('tooltip-update', updatePopper()) | |
| vm.$on('tooltip-update', updatePopper) |
🤖 Prompt for AI Agents
In packages/renderless/src/tooltip/new-vue.ts at line 71, the event handler for
'tooltip-update' is being called immediately instead of being registered as a
callback. Remove the parentheses after updatePopper so that the function
reference is passed to vm.$on, ensuring updatePopper is called only when the
event is emitted.
| if (props.visible === 'auto' && state.referenceElm) { | ||
| const { clientWidth, scrollWidth } = state.referenceElm.firstElementChild | ||
| if (scrollWidth <= clientWidth) { | ||
| return | ||
| } | ||
| } |
There was a problem hiding this comment.
Possible null dereference when computing scrollWidth
state.referenceElm.firstElementChild can be null
(e.g. when the slot renders text nodes only).
Accessing its properties will throw.
-const { clientWidth, scrollWidth } = state.referenceElm.firstElementChild
+const el = state.referenceElm?.firstElementChild as HTMLElement | null
+if (!el) {
+ state.showPopper = isShow // fallback – no auto mode check possible
+} else {
+ const { clientWidth, scrollWidth } = el
+ if (scrollWidth <= clientWidth) return
+}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In packages/renderless/src/tooltip/new-index.ts around lines 32 to 37, the code
accesses properties of state.referenceElm.firstElementChild without checking if
firstElementChild is null, which can cause a runtime error. Add a null check to
ensure firstElementChild exists before accessing clientWidth and scrollWidth,
and only proceed if it is not null.
| :tabindex="tabindex" | ||
| :aria-describeby="state.tooltipId" | ||
| @mouseenter="handleRefEvent('mouseenter')" | ||
| @mouseleave="handleRefEvent('mouseleave')" |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Accessibility attribute misspelled
aria-describeby should be aria-describedby; otherwise screen readers will
ignore the reference.
-:aria-describeby="state.tooltipId"
+:aria-describedby="state.tooltipId"📝 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.
| :tabindex="tabindex" | |
| :aria-describeby="state.tooltipId" | |
| @mouseenter="handleRefEvent('mouseenter')" | |
| @mouseleave="handleRefEvent('mouseleave')" | |
| :tabindex="tabindex" | |
| :aria-describedby="state.tooltipId" | |
| @mouseenter="handleRefEvent('mouseenter')" | |
| @mouseleave="handleRefEvent('mouseleave')" |
🤖 Prompt for AI Agents
In packages/vue/src/tooltip/src/tooltip.vue between lines 7 and 10, the
attribute aria-describeby is misspelled and should be corrected to
aria-describedby to ensure screen readers recognize and use the reference
properly. Update the attribute name from aria-describeby to aria-describedby in
the template.
| // 优先级 > effect | ||
| type: { | ||
| type: String, | ||
| validator: (value: string) => Boolean(~['normal', 'warning', 'error', 'info', 'success'].indexOf(value)) |
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
packages/vue/src/tooltip/src/tooltip.vue (1)
7-8:⚠️ Potential issueFix the accessibility attribute spelling
The attribute
:aria-describebyis misspelled and should be:aria-describedby. Screen readers won't recognize the current attribute name, which breaks accessibility.-:aria-describeby="state.tooltipId" +:aria-describedby="state.tooltipId"
🧹 Nitpick comments (7)
packages/vue/src/tooltip/src/tooltip.vue (7)
136-136: Improve array inclusion checkUsing the bitwise NOT operator with
indexOfis an outdated pattern. Theincludes()method provides a more readable and modern approach, as already used in line 62.-validator: (value: string) => Boolean(~['normal', 'warning', 'error', 'info', 'success'].indexOf(value)) +validator: (value: string) => ['normal', 'warning', 'error', 'info', 'success'].includes(value)
87-87: Add default value for content propThe
contentprop lacks a default value, which could lead to unexpected rendering issues. Consider adding a default empty string.-content: { type: [String, Object] }, +content: { + type: [String, Object], + default: () => '' +},
101-102: Add default values for boolean propsThe
manualandmodelValueprops don't have default values. It's a good practice to explicitly define defaults for boolean props.-manual: { type: Boolean }, -modelValue: { type: Boolean }, +manual: { + type: Boolean, + default: () => false +}, +modelValue: { + type: Boolean, + default: () => false +},
121-123: Define type and default for reference and popper propsThe
referenceandpopperprops are missing type definitions and default values, making their expected usage unclear.-reference: {}, -popper: {}, +reference: { + type: [Object, String], + default: () => null +}, +popper: { + type: [Object, String], + default: () => null +},
19-19: Extract complex class binding to a computed propertyThe class binding combines multiple conditions and is difficult to read. Consider extracting it to a computed property for better maintainability.
// In the template -:class="['is-' + (type || effect || 'dark'), popperClass, state.showContent ? 'tiny-tooltip__show-tips' : '']" +:class="popperClassNames" // In the script, add to the renderless function or setup return +computed: { + popperClassNames() { + return [ + `is-${this.type || this.effect || 'dark'}`, + this.popperClass, + this.state.showContent ? 'tiny-tooltip__show-tips' : '' + ] + } +}
63-67: Improve prop documentation with better commentsSeveral props have unclear purpose, indicated by Chinese comments or minimal documentation. Consider adding comprehensive JSDoc comments to explain each prop's purpose, expected values, and usage examples.
Example for adjustArrow:
-// 原来未暴露的属性, 自动传入vue-popper adjustArrow: { type: Boolean, default: () => false }, +/** + * @description Whether to automatically adjust the arrow position + * @default false + */ +adjustArrow: { + type: Boolean, + default: () => false +},Also applies to: 73-77, 78-82, 120-123, 142-146
20-20: Consider using computed properties for style bindingsDirect style bindings in templates are less maintainable and can make the template harder to read. Consider extracting these to computed properties.
// In the template -:style="{ ['max-width']: state.tipsMaxWidth }" +:style="popperStyle" // In the script, add to the renderless function or setup return +computed: { + popperStyle() { + return { 'max-width': this.state.tipsMaxWidth } + } +} // Same approach for the content wrapper styleAlso applies to: 29-29
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/renderless/src/tooltip/new-vue.ts(1 hunks)packages/vue-hooks/src/useTimer.ts(1 hunks)packages/vue/src/tooltip/src/index.ts(1 hunks)packages/vue/src/tooltip/src/tooltip.vue(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/vue/src/tooltip/src/index.ts
- packages/vue-hooks/src/useTimer.ts
- packages/renderless/src/tooltip/new-vue.ts
PR
使用模板的语法，重写tooltip组件
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Issue Number: N/A
What is the new behavior?
Does this PR introduce a breaking change?
Other information
Summary by CodeRabbit
New Features
Enhancements
Other