docs(form): [form] optimize demo and docs #2401
Conversation
WalkthroughThe changes in this pull request primarily focus on localization adjustments and minor modifications to form component properties within various Vue files. Specifically, the label text for the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
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
🧹 Outside diff range and nitpick comments (9)
examples/sites/demos/pc/app/form/popper-options-composition-api.vue (1)
Line range hint
4-13: Enhance form accessibility.The form lacks proper ARIA attributes for validation states and messages.
Consider adding:
aria-invalidattributes on invalid inputsaria-describedbyfor error messagesrole="alert"for validation messages<tiny-form ref="formRef" :model="createData" label-width="60px" :popper-options="{ bubbling: true }"> - <tiny-form-item label="姓名" prop="name" required> + <tiny-form-item label="姓名" prop="name" required aria-label="Name field"> <tiny-input v-model="createData.name"> </tiny-input> </tiny-form-item>examples/sites/demos/pc/app/form/label-position-composition-api.vue (1)
Line range hint
1-16: Consider enhancing the demo documentation.Since this is a demo component, it would be helpful to add comments explaining the different label position options and their use cases. This would improve the documentation value for developers using the component.
Add comments above the button group:
<div class="demo-form"> <div class="title"> + <!-- Label position options: + - left: Labels aligned to the left (default) + - right: Labels aligned to the right + - top: Labels positioned above the input fields + --> 标签位置： <tiny-button-group :data="labelPositionList" v-model="labelPositionValue"></tiny-button-group> </div>examples/sites/demos/pc/app/form/smb-required-composition-api.vue (1)
Line range hint
52-54: Enhance the form validation demo.The empty validation callback doesn't effectively demonstrate form validation handling. Consider adding proper success/error handling for educational purposes.
function handleSubmit() { ruleFormRef.value.validate((valid) => { - // empty + if (valid) { + // Demo success handling + alert('Form validation passed!') + } else { + // Demo error handling + console.warn('Form validation failed') + } }) }examples/sites/demos/pc/app/form/smb-required.vue (2)
Line range hint
61-65: Enhance demo with validation feedbackThe empty validation callback doesn't demonstrate proper form validation handling. Consider enhancing the demo to show success/error states:
handleSubmit() { this.$refs.ruleFormRef.validate((valid) => { - // empty + if (valid) { + // Demo success state + this.$message.success('Form validation passed!') + } else { + // Demo error state + this.$message.error('Please check the form fields') + } }) }
Line range hint
1-30: Consider internationalizing demo textAs this is a demo file, consider using i18n for Chinese text to make it accessible to international developers:
<div class="demo-form"> <div class="mb-12"> - 是否隐藏星号： + {{ t('form.hideAsterisk') }} <tiny-switch v-model="hideRequiredAsterisk"></tiny-switch> </div> <tiny-form ref="ruleFormRef" :hide-required-asterisk="hideRequiredAsterisk" :model="createData" :rules="rules" label-width="108px" validate-type="text" message-type="block" > - <tiny-form-item label="用户名" prop="username"> + <tiny-form-item :label="t('form.username')" prop="username"> <tiny-input v-model="createData.username"></tiny-input> </tiny-form-item> - <tiny-form-item label="密码" prop="password"> + <tiny-form-item :label="t('form.password')" prop="password"> <tiny-input v-model="createData.password" type="password" show-password></tiny-input> </tiny-form-item> - <tiny-form-item label="昵称（可选）" prop="nickname"> + <tiny-form-item :label="t('form.nickname')" prop="nickname"> <tiny-input v-model="createData.nickname"></tiny-input> </tiny-form-item> <tiny-form-item> - <tiny-button type="primary" @click="handleSubmit()"> 注册 </tiny-button> + <tiny-button type="primary" @click="handleSubmit()">{{ t('form.register') }}</tiny-button> </tiny-form-item> </tiny-form> </div>Would you like me to help create the corresponding translation files and i18n setup?
examples/sites/demos/pc/app/form/slot-label-composition-api.vue (3)
14-14: Consider using i18n for the label text.The hardcoded Chinese text "超过两行文字，省略显示" should be internationalized for better maintainability and global usage.
Example implementation:
-<div class="custom-label" v-auto-tip>超过两行文字，省略显示</div> +<div class="custom-label" v-auto-tip>{{ t('form.labelOverflowText') }}</div>
40-40: Document the AutoTip directive's functionality.While the directive is properly imported, consider adding a comment explaining its purpose and usage for better maintainability.
Example:
+// AutoTip directive handles text overflow with automatic tooltips import { AutoTip as VAutoTip } from '@opentiny/vue-directive'Also, consider maintaining naming consistency:
-import { AutoTip as VAutoTip } from '@opentiny/vue-directive' +import { AutoTip } from '@opentiny/vue-directive'
Line range hint
58-63: Consider implementing a comprehensive i18n strategy.The validation messages in the rules object contain hardcoded strings. Consider implementing a centralized i18n strategy for form validation messages to support multiple languages.
Example approach:
const rules = ref({ radio: [ - { required: true, message: '必填', trigger: 'blur' } + { required: true, message: t('form.validation.required'), trigger: 'blur' } ], users: [ - { required: true, message: '必填', trigger: 'blur' }, - { min: 2, max: 11, message: '长度必须不小于2', trigger: 'change' } + { required: true, message: t('form.validation.required'), trigger: 'blur' }, + { min: 2, max: 11, message: t('form.validation.minLength', { min: 2 }), trigger: 'change' } ], // ... other rules })examples/sites/demos/pc/app/form/slot-label.vue (1)
Line range hint
82-93: Consider enhancing the label overflow handling.The current CSS implementation for
.custom-labelrelies heavily on webkit-specific properties. Consider adding fallbacks for better cross-browser support..custom-label { display: -webkit-box; + display: flex; width: 100%; -webkit-line-clamp: 2; line-height: 1.2; overflow: hidden; -webkit-box-orient: vertical; + text-overflow: ellipsis; white-space: wrap; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (11)
- examples/sites/demos/pc/app/form/form-validation-composition-api.vue (1 hunks)
- examples/sites/demos/pc/app/form/form-validation.vue (1 hunks)
- examples/sites/demos/pc/app/form/label-position-composition-api.vue (1 hunks)
- examples/sites/demos/pc/app/form/label-position.vue (1 hunks)
- examples/sites/demos/pc/app/form/popper-options-composition-api.vue (1 hunks)
- examples/sites/demos/pc/app/form/popper-options.vue (1 hunks)
- examples/sites/demos/pc/app/form/slot-label-composition-api.vue (3 hunks)
- examples/sites/demos/pc/app/form/slot-label.vue (4 hunks)
- examples/sites/demos/pc/app/form/smb-required-composition-api.vue (2 hunks)
- examples/sites/demos/pc/app/form/smb-required.vue (2 hunks)
- examples/sites/demos/pc/app/form/webdoc/form.js (1 hunks)
✅ Files skipped from review due to trivial changes (3)
- examples/sites/demos/pc/app/form/form-validation-composition-api.vue
- examples/sites/demos/pc/app/form/form-validation.vue
- examples/sites/demos/pc/app/form/popper-options.vue
🔇 Additional comments (13)
examples/sites/demos/pc/app/form/popper-options-composition-api.vue (2)
43-43: Consider responsive design implications of fixed dimensions.The fixed width and padding might cause issues on smaller screens or when the component is used in different contexts.
Consider:
- Using relative units (rem/em) or responsive widths
- Adding media queries for different screen sizes
- Making the width configurable via props
.demo-form { - width: 420px; + width: min(420px, 100%); } .scroll-parent { height: 200px; overflow: auto; - padding-right: 40px; + padding-right: clamp(20px, 5%, 40px); }Also applies to: 48-48
45-47: Review scrollable container height limitation.The fixed height of 200px on the scroll parent might truncate content in certain scenarios.
Consider:
- Making the height dynamic based on content or available space
- Adding a minimum height instead of a fixed height
- Documenting the height limitation in component documentation
examples/sites/demos/pc/app/form/label-position-composition-api.vue (1)
6-6: Verify the impact of removing fixed label width.The removal of
label-widthmakes the form more flexible and responsive. However, ensure this change doesn't cause unintended layout shifts when switching between different label positions, especially in the 'left' and 'right' positions.examples/sites/demos/pc/app/form/label-position.vue (2)
6-6: LGTM! Removal of label-width improves demo focus.The removal of the fixed
label-widthmakes the demo more focused on demonstrating label positioning functionality, which is its primary purpose.
Line range hint
6-16: Verify form layout with longer labels.Since the fixed label width constraint has been removed, please verify that the form maintains proper alignment when using longer label text in different positions (left/right/top).
Let's check if there are any existing form demos with long labels that might be affected:
✅ Verification successful
The previous command failed because the file type wasn't recognized. I've modified it to add Vue file type recognition and relaxed the label length constraint to see all form labels in context. This will help us understand the current label usage patterns and potential impact of label position changes.
Based on the search results, I found several examples that demonstrate form layouts with longer labels, particularly in the overflow-title and group-form demos. The most relevant examples show:
overflow-title.vuehas labels like "请输入人员信息请输入人员信息" (long Chinese text)group-form.vuehas labels like "标题内容超出六个字" and "站点地址（可选）"- The forms handle these long labels properly with different label positions through the
:label-positionpropSince there are working examples showing proper alignment with longer labels in different positions, and the component already has built-in support for handling label overflow through the
overflow-titleprop, I can conclude that the form maintains proper alignment with longer labels.The form component correctly handles label alignment with long text in all positions.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for form demos with potentially long labels rg -l --type vue 'label="[^"]{20,}"' examples/sites/demos/pc/app/form/Length of output: 100
Script:
#!/bin/bash # Search for form labels in Vue files, showing context rg -U --type-add 'vue:*.vue' --type vue 'label="[^"]+"' -B1 -A1 examples/sites/demos/pc/app/form/Length of output: 72606
examples/sites/demos/pc/app/form/smb-required-composition-api.vue (1)
Line range hint
61-69: LGTM! Clean and well-scoped styles.The added styles provide consistent demo presentation with appropriate scoping.
examples/sites/demos/pc/app/form/smb-required.vue (1)
33-33: LGTM! Removed unused Modal importGood cleanup of unused imports, improving code maintainability.
examples/sites/demos/pc/app/form/slot-label-composition-api.vue (1)
3-3: LGTM! Form configuration follows Vue best practices.The form setup with label-width and proper model/rules binding is well-structured.
examples/sites/demos/pc/app/form/slot-label.vue (3)
3-3: LGTM! Form configuration looks good.The form configuration with
label-width="100px"provides consistent label alignment across form items.
31-31: LGTM! Proper directive integration.The AutoTip directive is correctly imported and registered in the component.
Also applies to: 42-42
14-14: Verify tooltip behavior with long text.The
v-auto-tipdirective has been added to handle text overflow, working in conjunction with the CSS-webkit-line-clampproperty. Please ensure that:
- The tooltip appears only when text is actually truncated
- The full text is visible in the tooltip
✅ Verification successful
Let me search for the directive implementation and CSS styles to verify the tooltip behavior.
Based on the implementation details found, I can now provide the final response:
The tooltip behavior is correctly implemented and will work as expected.
The code shows that:
- The tooltip only appears when text is truncated (via
isEllipsischeck that compares scrollWidth/Height with clientWidth/Height)- The full text content is automatically used as tooltip content when no custom content is provided
- The CSS class
.custom-labelproperly sets up text truncation with-webkit-line-clamp: 2The
v-auto-tipdirective's implementation ensures the tooltip appears only when needed and displays the complete text, meeting both verification requirements.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for other usages of v-auto-tip to understand the expected behavior rg "v-auto-tip" -A 2Length of output: 8171
Script:
#!/bin/bash # Search for the directive implementation cat packages/vue-directive/src/auto-tip.ts # Search for CSS styles related to the custom-label class rg "custom-label" -A 3Length of output: 7476
examples/sites/demos/pc/app/form/webdoc/form.js (2)
282-282: LGTM! Documentation improvements enhance clarity.The changes improve the documentation by:
- Using a more descriptive demo name that better reflects its purpose
- Providing clearer context about when the
bubblingproperty is needed- Explaining the specific scenario (scrolling parent element) where tooltip misalignment occurs
Also applies to: 287-287
273-275: Verify the impact of changed default behavior.The documentation indicates that
hide-required-asterisknow defaults totrue, which means the red asterisk will be hidden by default. This is a significant change in default behavior that could affect existing forms.Let's verify the actual implementation:
| @@ -9,7 +9,7 @@ | |||
| :hide-required-asterisk="hideRequiredAsterisk" | |||
| :model="createData" | |||
| :rules="rules" | |||
| label-width="100px" | |||
| label-width="108px" | |||
There was a problem hiding this comment.
💡 Codebase verification
Label width change to 108px is inconsistent with other form demos
The change from 100px to 108px in smb-required-composition-api.vue creates inconsistency, as most form demos use label-width="100px". Only two files use 108px:
- examples/sites/demos/pc/app/form/smb-required.vue
- examples/sites/demos/pc/app/form/smb-required-composition-api.vue
This inconsistency should be addressed by either:
- Reverting these two files back to 100px to maintain consistency with other demos
- Updating all form demos to use 108px if there's a specific design requirement
🔗 Analysis chain
LGTM! Verify label-width consistency across demos.
The label-width adjustment improves visual alignment. Let's verify this change is consistent across related demos.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check label-width consistency across form demos
# Expect: All form demos should use the same label-width value
rg -l 'label-width="(?!108px)' 'examples/sites/demos/pc/app/form/*.vue'
Length of output: 312
Script:
#!/bin/bash
# Description: Check label-width values across form demos
# Using simpler pattern matching without look-ahead
rg -l 'label-width="' 'examples/sites/demos/pc/app/form/*.vue' | \
xargs rg 'label-width="[^"]*"' -A 1
Length of output: 18284
| @@ -9,7 +9,7 @@ | |||
| :hide-required-asterisk="hideRequiredAsterisk" | |||
| :model="createData" | |||
| :rules="rules" | |||
| label-width="100px" | |||
| label-width="108px" | |||
There was a problem hiding this comment.
💡 Codebase verification
Label width inconsistency detected
The change to 108px in smb-required.vue creates an inconsistency with most form demos in the codebase:
- Most form demos use
100pxas the standard label width - Only a few specific demos use different widths:
60pxin message-type and popper-options demos150pxin form-disabled demos108pxonly in smb-required and smb-required-composition-api
Consider maintaining consistency by keeping the label width at 100px unless there's a specific requirement for the change.
🔗 Analysis chain
Verify label-width consistency across form demos
The label width change from 100px to 108px should be consistent with other form demos to maintain visual harmony.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for label-width values in other form demos
rg -l 'label-width="(?!108px)' examples/sites/demos/pc/app/form/
Length of output: 305
Script:
#!/bin/bash
# Search for label-width values in form demos using a simpler pattern
rg 'label-width="[^"]*"' examples/sites/demos/pc/app/form/ -A 1
Length of output: 7489
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
Release Notes
New Features
urlinput field using a new tooltip directive.Bug Fixes
Style
Chores