docs(auto-tip): [directives] updateauto-tip and highlight-query docs and demos #2406
Conversation
WalkthroughThe changes in this pull request primarily focus on enhancing the tooltip functionality within the Vue components, specifically in the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
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: 4
🧹 Outside diff range and nitpick comments (10)
examples/sites/demos/pc/app/directives/highlight-query/avoid.spec.js (1)
Line range hint
10-13: Add explicit wait for page load stabilityThe test might be flaky if the highlight nodes are rendered asynchronously. Consider adding an explicit wait for the initial nodes to be present.
await page.goto('directives-highlight-query#avoid') + await page.waitForSelector('.tiny-hl-query-node') const hlNode = page.locator('.pc-demo-container .tiny-hl-query-node') await expect(hlNode).toHaveCount(12)examples/sites/demos/pc/app/directives/auto-tip/always-show.spec.js (1)
Line range hint
3-12: Consider enhancing test clarity and coverage.The test could benefit from:
- More descriptive test title explaining the specific tooltip behavior being tested
- Comments explaining why certain tooltips should/shouldn't be visible
- Additional test cases for the new JSX element tooltip content
Consider updating the test like this:
-test('常显提示', async ({ page }) => { +test('should show/hide tooltips based on disabled state and reason', async ({ page }) => { page.on('pageerror', (exception) => expect(exception).toBeNull()) await page.goto('directives-auto-tip#auto-tip-always-show') + // Should not show tooltip for enabled items await page.getByText('去游泳馆游泳').hover() await expect(page.getByRole('tooltip', { name: '羽毛球拍坏了' })).not.toBeVisible() + // Should show tooltip with disabled reason for disabled items await page.getByText('去羽毛球馆打羽毛球').hover() await expect(page.getByRole('tooltip', { name: '羽毛球拍坏了' })).toBeVisible() + + // TODO: Add test case for JSX element tooltip content })examples/sites/demos/pc/app/directives/auto-tip/always-show-composition-api.vue (1)
29-36: Consider extracting styles from JSX.Inline styles in JSX make it harder to maintain consistent styling across components.
Consider moving the styles to the component's style section:
-<a href="javascript:void(0)" style="color:#f80"> +<a href="javascript:void(0)" class="tooltip-link">Add to style section:
.tooltip-link { color: #f80; }examples/sites/demos/pc/app/directives/webdoc/directives-highlight-query.js (1)
29-29: Clear explanation of technical limitationsThe updated Chinese description effectively communicates:
- The Vue compilation process's special handling of text nodes
- The directive's direct DOM manipulation behavior
- The potential failure scenarios
The technical accuracy and clarity are well maintained.
Consider adding line breaks in the Chinese description to match the English version's formatting, making it easier to read:
- '纯文字节点在<code>Vue</code> 编译时有特殊处理。自动高亮搜索字的指令是直接操作<code>Dom</code>节点的内容，所以要避免纯文本节点。以下2个场景会造成<code>Vue</code> 更新机制失败。', + '纯文字节点在<code>Vue</code> 编译时有特殊处理。\n' + + '自动高亮搜索字的指令是直接操作<code>Dom</code>节点的内容，所以要避免纯文本节点。\n' + + '以下2个场景会造成<code>Vue</code> 更新机制失败。',examples/sites/demos/pc/app/directives/auto-tip/always-show.vue (2)
11-11: LGTM! Consider adding JSDoc comments.The v-auto-tip directive usage is semantically clearer with
disabledReason. The dark effect enhances tooltip visibility.Consider adding a comment above the template to document the purpose of this demo:
<!-- Demo: Always Show Tooltip Demonstrates the v-auto-tip directive with always-visible tooltips for disabled items -->
26-39: Consider adding more demo cases.As this is a demo file, consider adding examples for:
- Dynamic tooltip content updates
- Error state tooltips
- Different tooltip placements
Would you like me to provide examples for these additional demo cases?
examples/sites/demos/pc/app/directives/highlight-query/avoid-composition-api.vue (1)
5-5: Consider adding tooltips to explain button purposesWhile the button text changes are more descriptive, consider adding tooltips to explain:
- Why the content is being changed (to demonstrate dynamic highlighting)
- Why buttons are mixed with text (to show component composition)
Example implementation:
-<tiny-button @click="changeList">修改诗的内容</tiny-button> +<tiny-button @click="changeList" title="点击切换内容以演示高亮的动态更新">修改诗的内容</tiny-button>Also applies to: 16-16, 27-27
examples/sites/demos/pc/app/directives/highlight-query/avoid.vue (1)
66-73: Consider standardizing spacing units for consistency.While the new styles improve the visual hierarchy and layout, consider using consistent spacing units throughout the component. The margin-left on .tiny-button could be aligned with the margin-bottom used on .tiny-input (20px vs 20px).
Consider updating the styles to maintain consistency:
.tiny-input { width: 280px; - margin-bottom: 20px; + margin-bottom: 1.25rem; /* or stick with 20px but document the spacing system */ } .tiny-button { - margin-left: 20px; + margin-left: 1.25rem; /* matching the spacing unit */ }examples/sites/demos/pc/app/directives/webdoc/directives-auto-tip.js (2)
15-26: Verify translation accuracy and consider adding style example.The documentation improvements clearly explain the directive's behavior. However, consider the following suggestions:
- The English translation of the tip block (line 24) could be more idiomatic: "Requires the user to add more than omitted styles" is unclear.
- Consider adding a minimal CSS example within the tip block to demonstrate the required overflow styles.
'en-US': ` With the <code>v-auto-tip</code> custom command, you can detect if the text is too long when the mouse moves over the <code>Dom</code> element, and automatically add a <code>tooltip</code> prompt when it is too long. <div class="tip custom-block"> - Requires the user to add more than omitted styles to the <code>Dom</code> element, see the example! + Note: The element must have text overflow styles applied (e.g., text-overflow: ellipsis) for this to work. See the example below! + <pre><code> + .element { + white-space: nowrap; + overflow: hidden; + text-overflow: ellipsis; + } + </code></pre> </div> `
38-46: Improve type declaration formatting and consistency.The Chinese documentation is well-structured, but the type declaration could be more readable:
-其类型声明为<code>{always:boolean; content:string | VNode | Vnode[]; effect: string; placement: string }</code> +其类型声明为： +<pre><code> +{ + always: boolean; // 是否始终显示 + content: string | VNode | VNode[]; // 提示内容 + effect: 'light' | 'dark'; // 主题 + placement: string; // 位置 +} +</code></pre>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (8)
- examples/sites/demos/pc/app/directives/auto-tip/always-show-composition-api.vue (2 hunks)
- examples/sites/demos/pc/app/directives/auto-tip/always-show.spec.js (1 hunks)
- examples/sites/demos/pc/app/directives/auto-tip/always-show.vue (3 hunks)
- examples/sites/demos/pc/app/directives/highlight-query/avoid-composition-api.vue (2 hunks)
- examples/sites/demos/pc/app/directives/highlight-query/avoid.spec.js (1 hunks)
- examples/sites/demos/pc/app/directives/highlight-query/avoid.vue (2 hunks)
- examples/sites/demos/pc/app/directives/webdoc/directives-auto-tip.js (2 hunks)
- examples/sites/demos/pc/app/directives/webdoc/directives-highlight-query.js (2 hunks)
🔇 Additional comments (11)
examples/sites/demos/pc/app/directives/auto-tip/always-show.spec.js (1)
8-8: LGTM! The tooltip assertions are correctly updated.The changes align with the updated tooltip content using
disabledReasoninstead oftip.Also applies to: 11-11
examples/sites/demos/pc/app/directives/auto-tip/always-show-composition-api.vue (3)
11-11: LGTM! Improved tooltip configuration.The directive configuration is well-structured with semantic property naming and enhanced visibility through the dark effect.
56-56: LGTM! Enhanced visual feedback.The line-through decoration provides clear visual feedback for disabled items, improving user experience.
Line range hint
1-58: Verify demo coverage of directive features.As this is a demo file, let's ensure it adequately demonstrates all key features of the auto-tip directive.
✅ Verification successful
Demo adequately demonstrates all key features of auto-tip directive
Based on the documentation and demos found, the current demo file properly covers all key features of the auto-tip directive:
- Basic text tooltip functionality with automatic overflow detection
- Always-show behavior with
always: trueoption- Custom tooltip content including both text and rich JSX content
- Dark theme via
effect: 'dark'option- Conditional tooltip application using the
falsevalueThe implementation matches the documented API which supports:
always- Controls automatic vs always-show behaviorcontent- Supports string and VNode contenteffect- Light/dark themesplacement- Tooltip positioning🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if all auto-tip features are covered in demos echo "Checking for other auto-tip demos and features..." # Look for other demo files fd -e vue -e md "auto-tip" examples/sites/demos/pc/app/directives/auto-tip/ # Check documentation for any undocumented features fd -e md "auto-tip" examples/sites/docs/Length of output: 333
Script:
#!/bin/bash # Let's try a different approach using ripgrep to find auto-tip related files and content # Find all auto-tip demo files echo "=== Auto-tip demo files ===" rg --files-with-matches "auto-tip" examples/sites/demos # Search for directive documentation and features echo -e "\n=== Auto-tip features and documentation ===" rg -i "auto-tip" -A 5 -B 5 examples/sites/demos # Look for directive implementation to understand features echo -e "\n=== Auto-tip implementation ===" rg -l "AutoTip" src/Length of output: 19873
examples/sites/demos/pc/app/directives/webdoc/directives-highlight-query.js (1)
15-17: Documentation improvements look good!The updated descriptions in both languages accurately explain the directive's functionality and maintain consistency in terminology. The proper use of
<code>tags for technical terms enhances readability.examples/sites/demos/pc/app/directives/auto-tip/always-show.vue (2)
18-18: LGTM! JSX support added correctly.The lang="jsx" attribute is properly configured for complex tooltip content.
61-61: LGTM! Clear visual feedback for disabled state.The line-through decoration provides clear visual indication of disabled items.
examples/sites/demos/pc/app/directives/highlight-query/avoid-composition-api.vue (1)
8-8: Documentation improvements enhance clarity!The updated scenario titles now clearly explain why certain approaches should be avoided, making it more educational for developers. The consistent use of the "title" class also improves visual hierarchy.
Also applies to: 13-13, 19-19, 24-24
examples/sites/demos/pc/app/directives/highlight-query/avoid.vue (2)
8-8: Documentation improvements enhance clarity!The updated titles now clearly explain why these scenarios should be avoided by adding "无法动态更新" (cannot be dynamically update). This helps developers better understand the technical limitations of the directive.
Also applies to: 13-13
5-5: Improved interaction clarity and demo consistency!The changes enhance the demo by:
- Making the button text more specific about modifying poem content
- Using
tiny-buttonconsistently in mixed content examples, which better demonstrates the directive's behaviorAlso applies to: 16-16, 27-27
examples/sites/demos/pc/app/directives/webdoc/directives-auto-tip.js (1)
49-57: Improve English translation and technical terminology.The English documentation has some areas for improvement:
- "instruction parameter" should be "directive parameter"
- "VNode array" formatting is inconsistent with other code references
- Consider using more idiomatic technical English
-The <code>v-auto-tip</code> command can receive an instruction parameter as follows: +The <code>v-auto-tip</code> directive accepts a parameter with the following structure:Let's verify if these parameter names align with the actual implementation:
✅ Verification successful
Documentation accurately reflects the implementation with minor terminology improvements needed
The documentation's parameter descriptions match the actual implementation in
auto-tip.ts, but can be improved for clarity:-The <code>v-auto-tip</code> command can receive an instruction parameter as follows: +The <code>v-auto-tip</code> directive accepts a configuration object with the following properties: -1. If the parameter is an object, its type is declared as <code>{always:boolean; content:string | VNode | Vnode[]; effect: string; placement: string }</code> +1. If the parameter is an object, its type is <code>{always:boolean; content:string | VNode | VNode[]; effect: 'dark' | 'light'; placement: string }</code>The implementation in
auto-tip.tsconfirms:
- All parameter names (
always,content,effect,placement) are correct- Types and default values match the documentation
- The directive behavior for both object configuration and
falsevalue is accurately describedThe suggested improvements are purely for readability and technical accuracy in English, without changing the technical content.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for v-auto-tip directive implementation ast-grep --pattern 'export const autoTip = { $$$ }'Length of output: 55
Script:
#!/bin/bash # Let's try different patterns to find the directive implementation rg -l "v-auto-tip|autoTip" --type js # Then search for the directive definition rg "(?s)(?:directive|bind)\s*\(\s*['\"](auto-tip|v-auto-tip)" -A 10 --type jsLength of output: 911
Script:
#!/bin/bash # Let's check the actual implementation in the basic usage examples and directive definition cat examples/sites/demos/pc/app/directives/auto-tip/basic-usage.vue cat examples/sites/demos/pc/app/directives/auto-tip/always-show.vueLength of output: 1706
Script:
#!/bin/bash # Let's check the actual directive implementation from the package cat packages/vue/src/directive/src/auto-tip.ts 2>/dev/null || cat packages/vue/src/directives/auto-tip.ts 2>/dev/null || find . -type f -name "auto-tip.ts" -o -name "auto-tip.js" | xargs catLength of output: 4893
| const hlNode = page.locator('.pc-demo-container .tiny-hl-query-node') | ||
| const button = page.locator('.pc-demo-container .tiny-button') | ||
| const button = page.locator('.pc-demo-container .tiny-button').first() |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider using a more specific selector instead of .first()
While .first() works, it makes the test dependent on button order. Consider using a more specific and robust selector that uniquely identifies the target button (e.g., by text content, role, or data attribute).
- const button = page.locator('.pc-demo-container .tiny-button').first()
+ const button = page.locator('.pc-demo-container .tiny-button[data-test="highlight-toggle"]')Don't forget to add the corresponding data attribute to the button in your Vue template:
<tiny-button data-test="highlight-toggle">
<!-- button content -->
</tiny-button>| <a href="javascript:void(0)" style="color:#f80"> | ||
| 选择 | ||
| </a> |
There was a problem hiding this comment.
Consider improving the link implementation.
The javascript:void(0) usage is not recommended as it:
- Doesn't follow accessibility best practices
- May be blocked by some Content Security Policies
Consider using a proper button or handling the click event:
-<a href="javascript:void(0)" style="color:#f80">
+<button type="button" class="link-button">
选择
-</a>
+</button>Add corresponding CSS:
.link-button {
background: none;
border: none;
color: #f80;
cursor: pointer;
padding: 0;
font: inherit;
}| disabledReason: ( | ||
| <span> | ||
| 还没有选择项目，现在去 | ||
| <a href="javascript:void(0)" style="color:#f80"> | ||
| 选择 | ||
| </a> | ||
| </span> | ||
| ) |
There was a problem hiding this comment.
Improve accessibility and interactivity of the tooltip link.
The tooltip contains an interactive link that needs improvements:
- Using javascript:void(0) is not recommended
- The link lacks onClick handler
- Color-only styling doesn't meet accessibility standards
Consider this improvement:
disabledReason: (
<span>
还没有选择项目，现在去
- <a href="javascript:void(0)" style="color:#f80">
+ <button
+ type="button"
+ onClick={() => {/* Add handler */}}
+ style={{
+ color: '#f80',
+ textDecoration: 'underline',
+ border: 'none',
+ background: 'none',
+ cursor: 'pointer',
+ padding: 0
+ }}
+ aria-label="选择项目"
+ >
选择
- </a>
+ </button>
</span>
)Committable suggestion was skipped due to low confidence.
| .title { | ||
| font-size: 18px; | ||
| font-weight: bold; | ||
| } | ||
|
|
||
| .tiny-button { | ||
| margin-left: 20px; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider using CSS custom properties for better maintainability
The styling is clean and well-organized. Consider using CSS custom properties (variables) for values that might be reused across demos:
+:root {
+ --demo-title-size: 18px;
+ --demo-title-weight: bold;
+ --demo-spacing: 20px;
+}
.title {
- font-size: 18px;
- font-weight: bold;
+ font-size: var(--demo-title-size);
+ font-weight: var(--demo-title-weight);
}
.tiny-button {
- margin-left: 20px;
+ margin-left: var(--demo-spacing);
}📝 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.
| .title { | |
| font-size: 18px; | |
| font-weight: bold; | |
| } | |
| .tiny-button { | |
| margin-left: 20px; | |
| } | |
| .demo { | |
| --demo-title-size: 18px; | |
| --demo-title-weight: bold; | |
| --demo-spacing: 20px; | |
| } | |
| .title { | |
| font-size: var(--demo-title-size); | |
| font-weight: var(--demo-title-weight); | |
| } | |
| .tiny-button { | |
| margin-left: var(--demo-spacing); | |
| } |
PR
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
Bug Fixes
Documentation
v-auto-tipandhighlight-querydirectives.Style