feat: integrate new component Tippy with demo
#5355
Conversation
|
|
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
apps/web-antd/src/bootstrap.tsOops! Something went wrong! :( ESLint: 9.17.0 Error [ERR_MODULE_NOT_FOUND]: Cannot find module '/node_modules/@vben/eslint-config/dist/index.mjs' imported from /eslint.config.mjs WalkthroughThis pull request introduces Tippy.js tooltip functionality across multiple applications and the playground. The changes involve adding the Changes
Possibly related PRs
Suggested Labels
Suggested Reviewers
Poem
Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (5)
packages/effects/common-ui/src/components/tippy/index.ts (1)
34-39: Consider adjusting default delay values.The current delay settings (500ms show, 200ms hide) might feel slow for power users. Consider reducing the show delay or making it configurable.
setDefaultProps({ allowHTML: true, - delay: [500, 200], + delay: [200, 200], theme: isDark.value ? '' : 'light', ...options, });packages/effects/common-ui/src/components/tippy/directive.ts (1)
66-71: Ensure proper cleanup in unmounted hook.The current implementation correctly destroys the tippy instance but should also remove the reference.
unmounted(el) { if (el.$tippy) { el.$tippy.destroy(); + delete el.$tippy; } else if (el._tippy) { el._tippy.destroy(); + delete el._tippy; } },playground/src/views/examples/tippy/index.vue (3)
10-33: Consider adding TypeScript types and simplifying boolean conversion.The props and computed properties could be improved for better type safety and readability.
Consider these improvements:
-const props = reactive({ +interface TippyProps { + animation: string; + arrow: boolean; + content: string; + delay: number; + duration: number; + followCursor: string; + hideOnClick: string; + maxWidth: string; + placement: string; + theme: string; +} + +const props = reactive<TippyProps>({ animation: 'shift-away', arrow: true, content: '这是一个提示', // ... rest of the props }); const tippyProps = computed(() => ({ ...props, - followCursor: ['', 'true'].includes(props.followCursor) - ? !!props.followCursor - : props.followCursor, - hideOnClick: ['', 'true'].includes(props.hideOnClick) - ? !!props.hideOnClick - : props.hideOnClick, + followCursor: props.followCursor === '' || props.followCursor === 'true', + hideOnClick: props.hideOnClick === '' || props.hideOnClick === 'true', }));
35-159: Add form validation and consider i18n for labels.The form configuration is comprehensive but could benefit from validation rules and internationalization.
Consider adding validation rules and extracting labels to i18n:
const [Form] = useVbenForm({ handleValuesChange(values) { Object.assign(props, { ...values }); }, schema: [ { component: 'Select', + rules: [{ required: true, message: t('validation.required') }], componentProps: { class: 'w-full', options: [ { label: 'shift-away', value: 'shift-away' }, // ... options ], }, defaultValue: props.animation, fieldName: 'animation', - label: '动画', + label: t('tippy.animation'), }, // ... rest of the schema ], });
165-226: Enhance accessibility and extract constants.The template could benefit from improved accessibility and constant extraction.
Consider these improvements:
+const TIPPY_DOC_URL = 'https://atomiks.github.io/tippyjs/v6/all-props/'; function goDoc() { - window.open('https://atomiks.github.io/tippyjs/v6/all-props/'); + window.open(TIPPY_DOC_URL); } <template> <Page title="Tippy"> <!-- ... --> <Button + aria-label="Open Tippy documentation" type="link" size="small" @click="goDoc" > 查看文档 </Button> <!-- ... --> <Button + aria-describedby="tooltip-content" v-tippy="'这是一个提示，使用了默认的配置'" > 默认配置 </Button> <!-- ... --> </Page> </template>
📜 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 (11)
apps/web-antd/src/bootstrap.ts(2 hunks)apps/web-ele/src/bootstrap.ts(2 hunks)apps/web-naive/src/bootstrap.ts(2 hunks)packages/effects/common-ui/package.json(2 hunks)packages/effects/common-ui/src/components/index.ts(1 hunks)packages/effects/common-ui/src/components/tippy/directive.ts(1 hunks)packages/effects/common-ui/src/components/tippy/index.ts(1 hunks)playground/src/bootstrap.ts(2 hunks)playground/src/router/routes/modules/examples.ts(1 hunks)playground/src/views/examples/tippy/index.vue(1 hunks)pnpm-workspace.yaml(2 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
packages/effects/common-ui/src/components/tippy/directive.ts
[error] 25-25: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 31-31: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 37-37: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 43-43: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 49-49: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
⏰ Context from checks skipped due to timeout of 90000ms (9)
- GitHub Check: Check (windows-latest)
- GitHub Check: Check (ubuntu-latest)
- GitHub Check: Test (windows-latest)
- GitHub Check: Test (ubuntu-latest)
- GitHub Check: Lint (windows-latest)
- GitHub Check: post-update (windows-latest)
- GitHub Check: Lint (ubuntu-latest)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: post-update (ubuntu-latest)
🔇 Additional comments (11)
packages/effects/common-ui/src/components/index.ts (1)
8-8: LGTM! Export follows the established pattern.The new Tippy export follows the consistent barrel file pattern used for other component exports.
apps/web-naive/src/bootstrap.ts (2)
4-4: LGTM! Import follows the established pattern.The import is correctly placed with other @VBen imports.
32-34: Verify consistent Tippy initialization across apps.The initialization timing looks correct, placed after access directive registration. However, let's verify the consistency across all apps.
✅ Verification successful
Tippy initialization is consistent across all apps
All apps (web-antd, web-ele, web-naive, and playground) initialize Tippy in the same way:
- Imported from
@vben/common-ui- Initialized after access directive registration
- Initialized before router configuration
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify consistent Tippy initialization across all apps # Expected: All apps should initialize Tippy in the same order # Check initialization order in all bootstrap files echo "Checking bootstrap files for Tippy initialization:" rg -B 2 -A 2 "initTippy\(app\)" "apps/*/src/bootstrap.ts" "playground/src/bootstrap.ts"Length of output: 452
Script:
#!/bin/bash # Find all bootstrap.ts files and check Tippy initialization echo "Locating bootstrap.ts files:" fd bootstrap.ts echo -e "\nChecking Tippy initialization in these files:" fd bootstrap.ts --exec rg -B 5 -A 5 "initTippy"Length of output: 2063
apps/web-antd/src/bootstrap.ts (1)
4-4: LGTM! Changes maintain consistency with other apps.The import and initialization of Tippy follow the same pattern as in other apps, maintaining consistency across the codebase.
Also applies to: 33-35
playground/src/bootstrap.ts (1)
4-4: LGTM! Consistent implementation across all apps confirmed.The import and initialization of Tippy maintain consistency with other apps, completing a uniform implementation across the codebase.
Also applies to: 34-36
apps/web-ele/src/bootstrap.ts (1)
4-4: LGTM! Proper initialization of Tippy.The initialization is correctly placed after core functionality setup and before mounting the app.
Also applies to: 36-37
packages/effects/common-ui/src/components/tippy/index.ts (1)
40-44: Potential reactivity issue with theme handling.The theme watchEffect is only set up if theme is not provided in options. This might cause issues if options.theme is initially set but later changes to 'auto'.
Consider moving the watchEffect outside the condition:
- if (!options || !Reflect.has(options, 'theme') || options.theme === 'auto') { watchEffect(() => { - setDefaultProps({ theme: isDark.value ? '' : 'light' }); + if (!options?.theme || options.theme === 'auto') { + setDefaultProps({ theme: isDark.value ? '' : 'light' }); + } }); - }playground/src/router/routes/modules/examples.ts (1)
251-259: LGTM! Route configuration follows existing patterns.The Tippy demo route is properly configured with appropriate icon and title.
packages/effects/common-ui/package.json (1)
25-25: LGTM! Dependencies added for Tippy integration.The new dependencies are correctly added and align with the PR objectives for implementing tooltip functionality.
Also applies to: 36-36, 39-39
pnpm-workspace.yaml (1)
159-159: Verify Tippy.js dependency versions for security and compatibility.Let's verify that the specified versions are up-to-date and secure:
Also applies to: 179-179
playground/src/views/examples/tippy/index.vue (1)
1-9: LGTM! Well-structured imports and setup.The component setup and imports are well-organized and follow Vue 3 best practices.
|
Caution An unexpected error occurred while opening a pull request: Not Found - https://docs.github.com/rest/git/refs#get-a-reference |
Description
添加工具提示组件 Tippy
Type of change
Please delete options that are not relevant.
pnpm-lock.yamlunless you introduce a new test example.Checklist
pnpm run docs:devcommand.pnpm test.feat:,fix:,perf:,docs:, orchore:.Summary by CodeRabbit
New Features
Dependencies
tippy.jsandvue-tippylibraries to the projectDocumentation