docs(form): [form] optimize form demos #2499
Conversation
WalkthroughThe pull request includes modifications to various Vue components and their corresponding test files. Key changes involve reordering form items, removing or altering attributes of components, and updating validation rules. Notably, several components have had their placeholder attributes removed or modified, and new form items have been introduced in some components. The test files have also been updated to reflect these changes, ensuring that the tests align with the new form structures and behaviors. 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: 6
🧹 Outside diff range and nitpick comments (28)
examples/sites/demos/pc/app/form/display-only.spec.ts (1)
18-20: LGTM! Consider externalizing test data.The assertions are well-structured, checking both visibility and content. However, the hardcoded Chinese text could be moved to a constants file for better maintainability.
Consider creating a test constants file:
// test-constants.ts export const FORM_TEST_DATA = { FOOD_ITEMS: { SINGLE: '黄金糕', MULTIPLE: '黄金糕; 双皮奶' } };Then update the assertions:
- await expect(getItem(4).locator('.tiny-input-display-only__content')).toHaveText('黄金糕') - await expect(getItem(5).locator('.tiny-input-display-only__content')).toHaveText('黄金糕; 双皮奶') + await expect(getItem(4).locator('.tiny-input-display-only__content')).toHaveText(FORM_TEST_DATA.FOOD_ITEMS.SINGLE) + await expect(getItem(5).locator('.tiny-input-display-only__content')).toHaveText(FORM_TEST_DATA.FOOD_ITEMS.MULTIPLE)examples/sites/demos/pc/app/form/basic-usage-composition-api.vue (1)
8-10: Consider adding validation rules for the numeric field.The numeric field could benefit from validation constraints to ensure data quality and prevent invalid inputs.
Consider adding validation like this:
<tiny-form-item label="数字"> - <tiny-numeric v-model="createData.quantity"></tiny-numeric> + <tiny-numeric + v-model="createData.quantity" + :min="0" + :max="100" + placeholder="请输入0-100的数字"> + </tiny-numeric> </tiny-form-item>examples/sites/demos/pc/app/form/form-validate-field-composition-api.vue (2)
Line range hint
52-60: Improve error handling in validateField()The current implementation has an empty if block when validation errors occur, which silently ignores validation failures. Consider displaying these errors to provide better user feedback.
async function validateField() { const errArray = [] await ruleFormRef.value.validateField(['url', 'datepicker'], (errMsg, fieldConfig) => { if (errMsg) { errArray.push(fieldConfig) } }) if (errArray.length > 0) { - // empty + TinyModal.alert('验证失败：' + errArray.map(field => field.label).join(', ')) } else { TinyModal.alert('日期和url通过校验') } }
Line range hint
52-60: Consider implementing comprehensive error handlingThe validation logic could be enhanced to provide a better user experience:
- Add error state handling
- Implement proper error recovery
- Consider adding loading states during async validation
Here's a suggested implementation:
async function validateField() { try { const errArray = [] // Show loading state isValidating.value = true await ruleFormRef.value.validateField(['url', 'datepicker'], (errMsg, fieldConfig) => { if (errMsg) { errArray.push({ ...fieldConfig, error: errMsg }) } }) if (errArray.length > 0) { // Display detailed error messages const errorMessages = errArray .map(field => `${field.label}: ${field.error}`) .join('\n') TinyModal.alert({ title: '验证失败', message: errorMessages, type: 'error' }) } else { TinyModal.alert({ title: '验证成功', message: '日期和url通过校验', type: 'success' }) } } catch (error) { // Handle unexpected errors TinyModal.alert({ title: '系统错误', message: '验证过程中发生错误，请重试', type: 'error' }) console.error('Validation error:', error) } finally { // Clear loading state isValidating.value = false } }examples/sites/demos/pc/app/form/basic-usage.vue (2)
19-19: Good UX improvement by favoring tooltip over placeholder.Removing the placeholder in favor of the tooltip is a better practice as:
- Tooltips remain visible during input, unlike placeholders that disappear
- The tooltip provides clear guidance without cluttering the input field
Line range hint
1-89: Consider enhancing the demo with validation examples.While this basic usage demo is well-structured, it could be more educational by including:
- Form validation rules
- Type definitions for the form data
- Examples of handling form submission errors
This would provide a more complete reference for developers implementing forms.
Would you like me to provide an example implementation with these enhancements?
examples/sites/demos/pc/app/form/custom-validation-rule.vue (1)
Line range hint
31-35: Consider improving password validation maintainabilityThe password validation regex is complex and could benefit from:
- Breaking down the regex into named constants
- Adding comments explaining the validation rules
- Making the error message more user-friendly
Consider refactoring like this:
+ const PASSWORD_MIN_LENGTH = 8; + const PASSWORD_PATTERN = { + LOWERCASE: /[a-z]/, + UPPERCASE: /[A-Z]/, + DIGIT: /\d/ + }; let validatePass = (rule, value, callback) => { - if (!/^(?=.*[a-z])(?=.*[A-Z])(?=.*\d)[a-zA-Z\d]{8,}$/.test(value)) { - callback(new Error('最少八个字符，至少包含一个大写字母，一个小写字母和一个数字')) + const errors = []; + if (value.length < PASSWORD_MIN_LENGTH) { + errors.push('密码长度至少为8个字符'); + } + if (!PASSWORD_PATTERN.LOWERCASE.test(value)) { + errors.push('需要包含小写字母'); + } + if (!PASSWORD_PATTERN.UPPERCASE.test(value)) { + errors.push('需要包含大写字母'); + } + if (!PASSWORD_PATTERN.DIGIT.test(value)) { + errors.push('需要包含数字'); + } + if (errors.length > 0) { + callback(new Error(errors.join('；'))); } else { callback() } }examples/sites/demos/pc/app/form/size-composition-api.vue (3)
9-11: Consider adding validation rules for the numeric input.While the implementation is correct, consider adding validation rules (min/max values, decimal places, etc.) to ensure data integrity.
Example implementation:
<tiny-form-item label="数字"> - <tiny-numeric v-model="createData.quantity"></tiny-numeric> + <tiny-numeric + v-model="createData.quantity" + :min="0" + :max="999" + :precision="2" + ></tiny-numeric> </tiny-form-item>
20-20: Enhance accessibility with aria-label.Since the placeholder was removed, consider adding an aria-label to improve accessibility and provide context about the expected input.
-<tiny-input v-model="createData.input"></tiny-input> +<tiny-input + v-model="createData.input" + aria-label="Please enter your text" +></tiny-input>
Line range hint
1-30: Consider implementing responsive design.The form has a fixed width of 380px which might not be optimal for all screen sizes. Consider making the form responsive to enhance mobile usability.
Suggestions:
- Use relative units or media queries in the CSS
- Allow the form to adapt to container width
- Consider adjusting label position for smaller screens
<style scoped> .demo-form { - width: 380px; + width: 100%; + max-width: 380px; } </style>examples/sites/demos/pc/app/form/size.vue (2)
20-20: LGTM! Improved UX by relying on tooltip instead of placeholder.The removal of the placeholder attribute is a good practice as the tooltip already provides user guidance, and the form item has a proper label for accessibility.
Consider adding
aria-describedbyto enhance accessibility by explicitly connecting the input with its tooltip:-<tiny-input v-model="createData.input"></tiny-input> +<tiny-input v-model="createData.input" aria-describedby="form-demo-tooltip"></tiny-input>
Line range hint
4-30: Consider adding form validation rules.The form includes various input types (numeric, date, text) but lacks validation rules. Adding validation would improve data integrity and user experience.
Here's a suggested approach:
<template> <tiny-form ref="createData" label-width="100px" :size="sizeValue" label-position="right" + :rules="rules" + @submit.prevent="submitClick"> <!-- ... form items ... --> </tiny-form> </template> <script> export default { data() { return { // ... existing data ... + rules: { + quantity: [{ required: true, message: '请输入数量', trigger: 'blur' }], + datepicker: [{ required: true, message: '请选择日期', trigger: 'change' }] + } } }, methods: { submitClick() { - TinyModal.alert('提交') + this.$refs.createData.validate((valid) => { + if (valid) { + TinyModal.alert('提交成功') + } + }) } } } </script>examples/sites/demos/pc/app/form/form-validation-composition-api.vue (4)
7-12: LGTM! Consider adding aria-labels for better accessibility.The new form items for level selection and IP address are well-structured and properly integrated with form validation.
Consider adding aria-labels to improve accessibility:
- <tiny-form-item label="等级" prop="radio"> + <tiny-form-item label="等级" prop="radio" aria-label="Select level"> <tiny-radio-group v-model="createData.radio" :options="options"></tiny-radio-group> </tiny-form-item> - <tiny-form-item label="IP字段" prop="ip"> + <tiny-form-item label="IP字段" prop="ip" aria-label="Enter IP address"> <tiny-ip-address v-model="createData.ip"></tiny-ip-address> </tiny-form-item>
32-33: LGTM! Consider adding loading states for better UX.The clear validation and reset buttons are properly implemented.
Consider adding loading states during async operations:
- <tiny-button @click="clearFormValid"> 移除校验 </tiny-button> - <tiny-button @click="resetForm"> 重置表单 </tiny-button> + <tiny-button @click="clearFormValid" :loading="isClearing"> 移除校验 </tiny-button> + <tiny-button @click="resetForm" :loading="isResetting"> 重置表单 </tiny-button>
Line range hint
82-87: Consider using a more realistic IP validation rule.The current IP validation rule only accepts "1.1.1.1", which might be too restrictive for a demo. Consider using a more general IP format validation.
Here's a suggested improvement:
ip: [ { - validator: (rule, value, cb) => (value === '1.1.1.1' ? cb() : cb(new Error('必填1.1.1.1'))), + validator: (rule, value, cb) => { + const ipRegex = /^(\d{1,3}\.){3}\d{1,3}$/; + if (!value) { + return cb(new Error('IP address is required')); + } + if (!ipRegex.test(value)) { + return cb(new Error('Please enter a valid IP address')); + } + const parts = value.split('.'); + const valid = parts.every(part => parseInt(part) >= 0 && parseInt(part) <= 255); + return valid ? cb() : cb(new Error('Each octet must be between 0 and 255')); + }, trigger: 'change' } ],
Line range hint
89-106: Enhance error handling in form submission.While the handlers demonstrate both callback and promise approaches well, they could benefit from better error handling and loading states.
Here's a suggested improvement:
+const isSubmitting = ref(false) + function handleSubmit() { + isSubmitting.value = true ruleFormRef.value.validate((valid) => { if (valid) { TinyModal.alert('回调用法：提交成功') } else { TinyModal.alert('回调用法：提交失败') } + }).finally(() => { + isSubmitting.value = false + }) } function handleSubmitPromise() { + isSubmitting.value = true ruleFormRef.value .validate() .then(() => { TinyModal.alert('promise用法:提交成功') }) .catch((error) => { - TinyModal.alert('promise用法:提交失败') + TinyModal.alert(`promise用法:提交失败 - ${error.message || '未知错误'}`) + }) + .finally(() => { + isSubmitting.value = false }) }Then update the submit button:
- <tiny-button type="primary" @click="validType === 'callback' ? handleSubmit() : handleSubmitPromise()"> + <tiny-button + type="primary" + :loading="isSubmitting" + @click="validType === 'callback' ? handleSubmit() : handleSubmitPromise()"> 提交 </tiny-button>examples/sites/demos/pc/app/form/form-validation.spec.ts (1)
Line range hint
1-93: Consider implementing a Page Object pattern for better test maintainability.The current test implementation tightly couples the test logic to the form structure. To improve maintainability and reduce the impact of UI changes, consider:
- Implementing a Page Object pattern to encapsulate form selectors and interactions
- Creating a central selector configuration file
- Using data-testid attributes consistently across the application
This would make the tests more resilient to form structure changes and easier to maintain.
Example implementation:
// formValidationPage.ts export class FormValidationPage { constructor(private page: Page) {} private selectors = { lengthInput: '[data-testid="length-validation-input"]', urlInput: '[data-testid="url-input"]', emailInput: '[data-testid="email-input"]' // ... other selectors } async fillLengthInput(value: string) { await this.page.locator(this.selectors.lengthInput).fill(value) } // ... other methods } // form-validation.spec.ts test('测试表单输入变化和失焦是否出现校验', async ({ page }) => { const formPage = new FormValidationPage(page) await formPage.fillLengthInput('test') // ... rest of the test })examples/sites/demos/pc/app/form/form-disabled.spec.js (2)
48-63: Consider refactoring test structure to reduce duplication.
- Line 62 duplicates the assertion from line 61.
- The enabled state assertions mirror the disabled state assertions, suggesting an opportunity for a more maintainable structure.
Consider refactoring the test to use a helper function that tests both states:
async function testFormItemStates(form, isDisabled) { const state = { class: isDisabled ? 'toHaveClass' : 'not.toHaveClass', disabled: isDisabled ? 'toBeDisabled' : 'not.toBeDisabled' } const formElements = [ { index: 0, selector: '.tiny-switch', assertion: state.class, value: /tiny-switch-disabled/ }, { index: 2, selector: 'input', assertion: state.disabled }, // ... define all form elements ] for (const element of formElements) { await expect(form.nth(element.index).locator(element.selector)) [element.assertion](element.value) } } // Usage in test await testFormItemStates(formItem, true) // Test disabled state await switchBtn.click() await testFormItemStates(formItem, false) // Test enabled state
Line range hint
1-73: Enhance test readability and maintainability.Consider these improvements:
- Add custom error messages to assertions to make test failures more descriptive
- Document the form structure being tested with comments
Example improvements:
+ // Form structure being tested: + // - Switch (index 0) + // - Text inputs (indices 2-3) + // - Button (index 6) + // - Numeric input (index 9) + // - Textarea (index 14) + // - Slider (index 18) - await expect(formItem.nth(0).locator('.tiny-switch')).toHaveClass(/tiny-switch-disabled/) + await expect(formItem.nth(0).locator('.tiny-switch')).toHaveClass( + /tiny-switch-disabled/, + 'Switch should be disabled' + )examples/sites/demos/pc/app/form/form-validation.vue (3)
7-12: Consider adding internationalization support for form labels.The form labels "等级" and "IP字段" are hardcoded in Chinese. Consider using an i18n system to support multiple languages and improve maintainability.
- <tiny-form-item label="等级" prop="radio"> + <tiny-form-item :label="t('form.level')" prop="radio"> - <tiny-form-item label="IP字段" prop="ip"> + <tiny-form-item :label="t('form.ipField')" prop="ip">
Line range hint
89-94: Improve IP address validation for better demo value.The current IP validation is overly restrictive by requiring exactly "1.1.1.1". Consider implementing a proper IP format validation to better demonstrate real-world usage.
ip: [ { - validator: (rule, value, cb) => (value === '1.1.1.1' ? cb() : cb(new Error('必填1.1.1.1'))), + validator: (rule, value, cb) => { + const ipRegex = /^(\d{1,3}\.){3}\d{1,3}$/; + if (!value) { + return cb(new Error('IP address is required')); + } + if (!ipRegex.test(value)) { + return cb(new Error('Please enter a valid IP address')); + } + const parts = value.split('.'); + const valid = parts.every(part => parseInt(part) >= 0 && parseInt(part) <= 255); + return valid ? cb() : cb(new Error('Each octet must be between 0 and 255')); + }, trigger: 'change' } ],
22-34: LGTM! Well-structured form validation implementation.The component demonstrates good practices with both callback and promise-based validation approaches. The toggle between validation methods provides excellent demo value.
Consider extracting validation messages to an i18n system for consistency with the earlier suggestion.
examples/sites/demos/pc/app/form/display-only-composition-api.vue (2)
47-52: Consider adding placeholders for better UXThe form items are well-structured, but could benefit from placeholders to guide users on expected input.
- <tiny-input v-model="formData.input"></tiny-input> + <tiny-input v-model="formData.input" placeholder="请输入文字"></tiny-input> - <tiny-input v-model="formData.textareaValue" type="textarea"></tiny-input> + <tiny-input v-model="formData.textareaValue" type="textarea" placeholder="请输入描述"></tiny-input>
149-149: Consider implementing i18n for text contentThe updated descriptive text provides better context, but should be internationalized for better maintainability and multi-language support.
Example implementation:
- 'TinyVue 为 Web 应用提供了丰富的基础 UI 组件，我们还将持续探索企业级应用的最佳 UI 实践，欢迎尝试使用 TinyVue。', + t('form.demo.tinyVueDescription') - 'OpenTiny 是一套企业级组件库解决方案，适配 PC 端 / 移动端等多端，涵盖 Vue2 / Vue3 / Angular 多技术栈，拥有主题配置系统 / 中后台模板 / CLI 命令行等效率提升工具，可帮助开发者高效开发 Web 应用。', + t('form.demo.openTinyDescription')Also applies to: 167-167
examples/sites/demos/pc/app/form/display-only.vue (2)
5-20: LGTM! Consider i18n for labels.The new form items are well-structured and properly integrated. The components are correctly bound to their respective data properties.
Consider extracting the Chinese labels into i18n translation files for better internationalization support.
165-165: Consider using shorter, more generic default values.The current default values contain lengthy marketing text about TinyVue and OpenTiny. For a form demo, consider:
- Using shorter, more generic placeholder text
- Moving product-specific content to documentation
- Extracting these strings to a constants file for easier maintenance
This would make the demo more focused and easier to maintain.
formData: { input: - 'TinyVue 为 Web 应用提供了丰富的基础 UI 组件，我们还将持续探索企业级应用的最佳 UI 实践，欢迎尝试使用 TinyVue。', + 'This is a sample long text input.', // ... textareaValue: - 'OpenTiny 是一套企业级组件库解决方案，适配 PC 端 / 移动端等多端，涵盖 Vue2 / Vue3 / Angular 多技术栈，拥有主题配置系统 / 中后台模板 / CLI 命令行等效率提升工具，可帮助开发者高效开发 Web 应用。', + 'This is a sample textarea content.',Also applies to: 183-183
examples/sites/demos/pc/app/form/form-disabled-composition-api.vue (1)
Line range hint
1-267: Well-structured form demo component.The component effectively demonstrates form functionality with:
- Logical grouping of form elements
- Proper reactive data binding
- Clean composition API implementation
- Comprehensive coverage of form components
Consider adding comments to document the purpose of each form section for better maintainability.
examples/sites/demos/pc/app/form/form-disabled.vue (1)
35-35: Remove unnecessary empty placeholders.The empty placeholder attributes in Select and Cascader components don't serve any purpose and should be removed for cleaner code.
-<tiny-select v-model="createData.select" placeholder=""> +<tiny-select v-model="createData.select"> -<tiny-cascader :options="options" :props="{ emitPath: false }" placeholder=" "> +<tiny-cascader :options="options" :props="{ emitPath: false }">Also applies to: 67-67
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (21)
examples/sites/demos/pc/app/form/basic-usage-composition-api.vue(3 hunks)examples/sites/demos/pc/app/form/basic-usage.spec.ts(0 hunks)examples/sites/demos/pc/app/form/basic-usage.vue(2 hunks)examples/sites/demos/pc/app/form/custom-validation-rule-composition-api.vue(1 hunks)examples/sites/demos/pc/app/form/custom-validation-rule.vue(1 hunks)examples/sites/demos/pc/app/form/display-only-composition-api.vue(4 hunks)examples/sites/demos/pc/app/form/display-only.spec.ts(1 hunks)examples/sites/demos/pc/app/form/display-only.vue(4 hunks)examples/sites/demos/pc/app/form/form-disabled-composition-api.vue(4 hunks)examples/sites/demos/pc/app/form/form-disabled.spec.js(2 hunks)examples/sites/demos/pc/app/form/form-disabled.vue(4 hunks)examples/sites/demos/pc/app/form/form-validate-field-composition-api.vue(1 hunks)examples/sites/demos/pc/app/form/form-validate-field.vue(1 hunks)examples/sites/demos/pc/app/form/form-validation-composition-api.vue(2 hunks)examples/sites/demos/pc/app/form/form-validation.spec.ts(2 hunks)examples/sites/demos/pc/app/form/form-validation.vue(2 hunks)examples/sites/demos/pc/app/form/size-composition-api.vue(2 hunks)examples/sites/demos/pc/app/form/size.vue(2 hunks)examples/sites/demos/pc/app/form/validate-debounce-composition-api.vue(1 hunks)examples/sites/demos/pc/app/form/validate-debounce.vue(1 hunks)packages/theme/src/form-item/index.less(2 hunks)
💤 Files with no reviewable changes (1)
- examples/sites/demos/pc/app/form/basic-usage.spec.ts
✅ Files skipped from review due to trivial changes (4)
- examples/sites/demos/pc/app/form/custom-validation-rule-composition-api.vue
- examples/sites/demos/pc/app/form/form-validate-field.vue
- examples/sites/demos/pc/app/form/validate-debounce-composition-api.vue
- examples/sites/demos/pc/app/form/validate-debounce.vue
🔇 Additional comments (18)
examples/sites/demos/pc/app/form/basic-usage-composition-api.vue (2)
33-33: LGTM! Clean import statement.
Good cleanup of unused 'ref' import while keeping the necessary 'reactive' import.
19-19: Verify if placeholder removal was intentional.
The removal of the placeholder attribute might impact user experience by removing helpful input guidance.
examples/sites/demos/pc/app/form/form-validate-field-composition-api.vue (1)
15-16: LGTM! Button styling changes are consistent with project-wide updates.
The removal of type="primary" from these buttons aligns with the broader changes across the codebase to standardize button styling.
examples/sites/demos/pc/app/form/basic-usage.vue (1)
8-10: LGTM! Form item placement is logically organized.
The numeric form item is properly implemented with correct data binding and initialization.
examples/sites/demos/pc/app/form/custom-validation-rule.vue (2)
12-12: LGTM! Improved visual hierarchy
The removal of type="primary" from the "Change validation rules" button helps maintain proper visual hierarchy, making the "Register" button the clear primary action.
Line range hint 63-71: Security concern: Password validation rules are dropped
The changeRule method has two potential issues:
- It completely removes password validation rules, which could lead to security vulnerabilities
- The method name doesn't clearly indicate that it's removing password validation
Consider this safer implementation:
changeRule() {
this.rules = {
username: [
{ required: true, message: '必填', trigger: 'blur' },
{ min: 2, max: 11, message: '长度必须不小于2', trigger: 'change' }
- ]
+ ],
+ password: this.rules.password // Preserve password validation rules
}
}Let's verify if this component is used in tests or other demos:
examples/sites/demos/pc/app/form/size.vue (1)
9-11: LGTM! Form item ordering aligns with other form demos.
The numeric form item is correctly implemented with proper data binding and follows the standardized form layout pattern observed across other form components.
examples/sites/demos/pc/app/form/form-validation.vue (1)
33-34: LGTM! Button improvements look good.
The removal of redundant type attributes and the addition of clear validation and reset form buttons improve the user experience.
examples/sites/demos/pc/app/form/display-only-composition-api.vue (1)
5-20: LGTM: Well-structured form control additions
The new form controls (switch, checkbox, checkbox group) enhance the demo's comprehensiveness by showcasing more input types. The implementation is clean and properly bound to the data model.
examples/sites/demos/pc/app/form/display-only.vue (1)
47-52: LGTM! Improved form organization.
The relocation of these form items enhances the logical flow of the form. The components maintain proper data bindings and functionality.
examples/sites/demos/pc/app/form/form-disabled-composition-api.vue (3)
8-10: LGTM: New form items enhance the demo's completeness.
The addition of Link, IpAddress, and ButtonGroup form items improves the demonstration of form capabilities and disabled state handling.
Also applies to: 25-30
168-168: LGTM: Improved column width for better content visibility.
The increase in ID column width from 40 to 60 enhances readability of the grid content.
264-264: LGTM: Enhanced title alignment.
The added left margin improves the visual alignment of the title with the form content.
examples/sites/demos/pc/app/form/form-disabled.vue (3)
8-10: LGTM: New form components enhance functionality.
The addition of Link, IpAddress, and ButtonGroup components enriches the form's capabilities while maintaining consistency with the form's disabled state functionality.
Also applies to: 25-30
193-193: LGTM: Improved ID column width.
The increase in ID column width from 40 to 60 enhances readability.
291-291: Verify title margin alignment with design guidelines.
The title margin-left has been reduced from 100px to 80px. Please confirm this change aligns with the design system's spacing guidelines.
✅ Verification successful
Let me check the specific margin values used in these files to verify consistency.
Title margin change is consistent with other form components
The margin-left value of 80px is already being used consistently across form components, specifically in both the standard and composition API versions of the form-disabled component. The change from 100px to 80px aligns the component with the existing pattern in the codebase.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for similar title margin styles in other form-related components
rg -l 'margin-left:.*px' 'examples/sites/demos/pc/app/form/'
Length of output: 290
Script:
#!/bin/bash
# Search for title margin values in form components
rg 'margin-left:.*px' 'examples/sites/demos/pc/app/form/' -B 2 -A 0
Length of output: 944
packages/theme/src/form-item/index.less (2)
24-25: LGTM: New prefix class variables follow established patterns.
The added prefix class variables for Cascader and Autocomplete components follow the existing naming convention and are properly scoped.
189-190: LGTM: Width styling is consistent with other form controls.
The width styling for Cascader and Autocomplete components is appropriately grouped with other full-width form controls, maintaining consistency in the form layout.
| @@ -9,13 +9,13 @@ test('表单仅展示', async ({ page }) => { | |||
| const getItem = (index: number) => demo.locator('.tiny-form-item__content').nth(index) | |||
|
|
|||
| await switchBtn.click() | |||
| await expect(demo.locator('.tiny-form-item').first().locator('input')).toBeVisible() | |||
| await expect(getItem(10).locator('input')).toBeVisible() | |||
There was a problem hiding this comment.
💡 Codebase verification
Index 10 visibility check is incorrect
The test file is verifying form display functionality with a switch toggle between editable and display-only modes. Based on the Vue template and test file analysis:
- The form has 12 items in total
- Index 10 corresponds to "超长文字" (long text) input field
- The test is checking visibility after clicking the switch, but the assertions pattern shows that form items should become non-visible after the switch click
- Other assertions at indices 1, 4, and 5 follow a consistent pattern of checking for non-visibility
The visibility check at index 10 is inconsistent with the test's logic flow and should be checking for non-visibility like other assertions.
🔗 Analysis chain
Verify form item index alignment.
The significant change in form item index (now checking index 10) suggests substantial form restructuring. Please verify that this index correctly corresponds to the intended form item in the updated form structure.
Let me generate another script to get more context about the test file structure and assertions:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check the form structure to verify item indices
# Look for form item definitions in the corresponding Vue component
# Find the related Vue component file
fd -e vue "display-only" -x cat {} | grep -A 5 "tiny-form-item"
Length of output: 7312
Script:
#!/bin/bash
# Get the full test file content to understand the test structure and assertions
cat examples/sites/demos/pc/app/form/display-only.spec.ts
Length of output: 1015
| const emailInput = formItem.nth(5).locator('input') | ||
| const validEmail = '111@test.com' |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Replace numeric index with email-specific selector.
The email input field should be identifiable by its validation rules or input type rather than its position in the form.
Example refactor:
- const emailInput = formItem.nth(5).locator('input')
+ const emailInput = form.locator('[data-testid="email-input"]')
// or
+ const emailInput = form.locator('input[type="email"]')Committable suggestion skipped: line range outside the PR's diff.
| const urlInput = formItem.nth(4).locator('input') | ||
| const validUrl = 'https://test.com' |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Replace numeric index with URL-specific selector.
The URL input field should be identifiable by its validation rules or input type rather than its position in the form.
Example refactor:
- const urlInput = formItem.nth(4).locator('input')
+ const urlInput = form.locator('[data-testid="url-input"]')
// or
+ const urlInput = form.locator('input[type="url"]')Committable suggestion skipped: line range outside the PR's diff.
| const firstInput = formItem.nth(2).locator('input') | ||
| await firstInput.click() |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider using more maintainable selectors instead of numeric indices.
Using numeric indices (nth(2)) for form item selection makes the tests brittle and prone to breaking when form items are reordered. Consider using more reliable selectors such as:
- data-testid attributes
- form field labels
- input names or other unique identifiers
Example refactor:
- const firstInput = formItem.nth(2).locator('input')
+ const firstInput = form.locator('[data-testid="length-validation-input"]')Committable suggestion skipped: line range outside the PR's diff.
| await expect(formItem.nth(0).locator('.tiny-switch')).toHaveClass(/tiny-switch-disabled/) | ||
| await expect(formItem.nth(2).locator('input').first()).toBeDisabled() | ||
| await expect(formItem.nth(3).locator('input').nth(1)).toBeDisabled() | ||
| await expect(formItem.nth(3).locator('input').first()).toBeDisabled() | ||
| await expect(formItem.nth(6).locator('button').first()).toHaveClass(/disabled/) | ||
| await expect(formItem.nth(7).locator('input')).toBeDisabled() | ||
| await expect(formItem.nth(8).locator('input')).toBeDisabled() | ||
| await expect(formItem.nth(9).locator('.tiny-numeric__input')).toHaveClass(/is-disabled/) | ||
| await expect(formItem.nth(10).locator('input').first()).toBeDisabled() | ||
| await expect(formItem.nth(11).locator('input').first()).toBeDisabled() | ||
| await expect(formItem.nth(12).locator('input')).toBeDisabled() | ||
| await expect(formItem.nth(13).locator('button').first()).toHaveClass(/disabled/) | ||
| await expect(formItem.nth(14).locator('input')).toBeDisabled() | ||
| await expect(formItem.nth(15).locator('input').first()).toBeDisabled() | ||
| await expect(formItem.nth(13).locator('input')).toBeDisabled() | ||
| await expect(formItem.nth(14).locator('textarea')).toBeDisabled() | ||
| await expect(formItem.nth(15).locator('input')).toBeDisabled() | ||
| await expect(formItem.nth(15).locator('input')).toBeDisabled() | ||
| await expect(formItem.nth(16).locator('input').first()).toBeDisabled() |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove duplicate assertion and consider using constants for form item indices.
- Line 35 duplicates the assertion from line 34, checking the same input element twice.
- Consider using named constants for form item indices to improve test maintainability and make the relationship between indices and form elements clear.
+ // At the top of the file
+ const FORM_ITEMS = {
+ SWITCH: 0,
+ INPUT_FIRST: 2,
+ INPUT_MULTIPLE: 3,
+ BUTTON: 6,
+ // ... add more meaningful names
+ }
- await expect(formItem.nth(0).locator('.tiny-switch')).toHaveClass(/tiny-switch-disabled/)
+ await expect(formItem.nth(FORM_ITEMS.SWITCH).locator('.tiny-switch')).toHaveClass(/tiny-switch-disabled/)
- await expect(formItem.nth(15).locator('input')).toBeDisabled()
- await expect(formItem.nth(15).locator('input')).toBeDisabled() // Remove duplicate
+ await expect(formItem.nth(15).locator('input')).toBeDisabled()Committable suggestion skipped: line range outside the PR's diff.
| <tiny-form-item label="Input"> | ||
| <tiny-input v-model="createData.user"></tiny-input> | ||
| </tiny-form-item> | ||
| <tiny-form-item label="Select"> | ||
| <tiny-select v-model="createData.select" placeholder=""> | ||
| <tiny-option v-for="item in selectOptions" :key="item.value" :label="item.label" :value="item.value"> | ||
| </tiny-option> | ||
| </tiny-select> | ||
| </tiny-form-item> | ||
| <tiny-form-item label="Numeric"> | ||
| <tiny-numeric v-model="createData.quantity"></tiny-numeric> | ||
| </tiny-form-item> |
There was a problem hiding this comment.
💡 Codebase verification
Based on the extensive search results, I can now make a determination about the placeholder usage in form elements:
Empty placeholders are not the standard pattern in this codebase
Looking at the search results, I found that:
-
Most form components consistently use meaningful placeholders across the codebase:
- Input components typically use placeholders like "请输入内容", "请输入", "Please input"
- Select components use placeholders like "请选择", "Please select"
- Search components use placeholders like "请输入关键词"
-
Empty placeholders (
placeholder="") or space-only placeholders (placeholder=" ") are rare exceptions rather than the norm. -
The form-disabled example appears to be an outlier in using empty placeholders, while most other form examples follow the pattern of providing meaningful placeholder text.
Therefore, the review comment's concern about empty placeholders is valid, as it goes against the established patterns in the codebase.
🔗 Analysis chain
Verify empty placeholder usage in form elements.
Several form elements have empty or removed placeholders. While this might be intentional, consider:
- Adding meaningful placeholders to improve user experience
- Ensuring consistency with design guidelines
Also applies to: 57-57, 64-64, 67-67
Let me try a different approach to verify the placeholder usage patterns.
This script will:
- Search for placeholder attributes in Vue files with surrounding context
- Look for form component usage patterns to understand the general conventions
The -U flag will help us see multiline matches, and we explicitly add .vue extension for ripgrep to recognize Vue files.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check placeholder usage across form components
# Look for inconsistencies in placeholder usage
echo "Checking placeholder usage in form components..."
rg -A 1 'placeholder="[^"]*"' --type vue
echo "Checking components without placeholders..."
ast-grep --pattern 'tiny-input|tiny-select|tiny-autocomplete'
Length of output: 338
Script:
#!/bin/bash
# Search for placeholder patterns in .vue files
echo "Searching for placeholder usage patterns..."
rg -U --type-add 'vue:*.vue' --type vue 'placeholder=' -A 2
# Search for form component usage patterns
echo -e "\nSearching for form component patterns..."
rg -U --type-add 'vue:*.vue' --type vue '<tiny-(input|select|autocomplete)' -A 2
Length of output: 493763
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
Improvements
Bug Fixes
Styling