feat: three new submissions have been added #5373
Conversation
wingjie
requested review from
anncwb,
vince292007,
mynetfan and
a team
as code owners
|
|
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/adapter/form.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 a new Changes
Sequence DiagramsequenceDiagram
participant Form as Form Component
participant Validator as Form Validator
participant Field as Form Field
Form->>Validator: Setup form with multipleRequired rule
Validator-->>Form: Validation rules configured
Form->>Field: Initialize field
Field->>Field: Check autoDefaultValue
alt autoDefaultValue is true
Field-->>Form: Set default value
end
Form->>Validator: Validate form
Validator->>Validator: Check multipleRequired rule
alt Invalid multiple selection
Validator-->>Form: Return localized error message
else Valid selection
Validator-->>Form: Validation passed
end
Possibly related PRs
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: 0
🧹 Nitpick comments (10)
apps/web-ele/src/adapter/form.ts (1)
31-36: Consider differentiatingmultipleRequiredfromrequiredruleThe
multipleRequiredrule currently uses identical validation logic to therequiredrule but returns a different error message. This could lead to confusion about which rule to use. Consider:
- Documenting the specific use case for this rule
- Using a distinct error message key for multiple selection validation
multipleRequired: (value, _params, ctx) => { if (value === undefined || value === null || value.length === 0) { - return $t('ui.formRules.selectRequired', [ctx.label]); + return $t('ui.formRules.multipleSelectRequired', [ctx.label]); } return true; },playground/src/adapter/form.ts (1)
24-29: Maintain consistent rule ordering across adaptersThe
multipleRequiredrule is placed before other rules, while in other adapters it's placed after. Consider maintaining consistent ordering across all adapters for better maintainability.Move the rule after the
selectRequiredrule to match other adapters.apps/web-antd/src/adapter/form.ts (2)
39-44: Consider extracting common validation logicThe
multipleRequiredrule is identical across all adapters. Consider extracting this common validation logic into a shared utility to improve maintainability and ensure consistent behavior.Consider moving the validation logic to a shared location, such as:
- A common validation utility file
- The
@vben/common-uipackage
This would reduce duplication and make future updates easier to manage.
Line range hint
31-36: Architectural Recommendations for Form ValidationTo improve the form validation implementation across adapters:
- Create a shared validation utility in
@vben/common-uito eliminate code duplication- Define distinct error message keys for different validation scenarios
- Document the specific use cases for each validation rule
- Consider creating a validation rule registry to maintain consistent rule ordering
This will improve maintainability, reduce duplication, and provide clearer guidance for developers.
Would you like me to provide a detailed implementation plan for these improvements?
Also applies to: 35-40, 24-29, 39-44
docs/src/_env/adapter/form.ts (1)
27-32: Consider enhancing the multipleRequired validation.While the implementation is functional, consider these improvements:
- Use a more specific translation key for multiple selection scenarios instead of reusing
selectRequired.- Add type checking to ensure the value is an array before checking length.
multipleRequired: (value, _params, ctx) => { - if (value === undefined || value === null || value.length === 0) { + if (value === undefined || value === null || + (Array.isArray(value) && value.length === 0)) { - return $t('ui.formRules.selectRequired', [ctx.label]); + return $t('ui.formRules.multipleRequired', [ctx.label]); } return true; },packages/@core/ui-kit/shadcn-ui/src/components/pin-input/input.vue (1)
17-19: Simplify the handleSendCode implementation.The function always returns
true, making the boolean return value redundant. Consider either:
- Removing the return value if it's not needed, or
- Implementing actual validation logic if there are conditions where code sending should be prevented.
packages/@core/ui-kit/form-ui/src/types.ts (1)
402-406: Consider enhancing the multipleRequired type definition.The return type could be more specific to better reflect the actual implementation.
multipleRequired?: ( value: any, params: any, ctx: Record<string, any>, -) => boolean | string; +) => true | string;packages/@core/ui-kit/form-ui/src/form-render/form-field.vue (3)
168-177: Consider consolidating duplicate prop computation logic.The
computedItemPropscomputed property duplicates logic fromcomputedProps. Consider extracting the common logic into a shared function to improve maintainability.+function computeComponentProps(values: any, formApi: any) { + return isFunction(componentProps) + ? componentProps(values, formApi!) + : componentProps; +} const computedProps = computed(() => { - const finalComponentProps = isFunction(componentProps) - ? componentProps(values.value, formApi!) - : componentProps; + const finalComponentProps = computeComponentProps(values.value, formApi); return { ...commonComponentProps, ...finalComponentProps, ...dynamicComponentProps.value, }; }); const computedItemProps = computed(() => { - const finalComponentProps = isFunction(componentProps) - ? componentProps(values.value, formApi!) - : componentProps; + const finalComponentProps = computeComponentProps(values.value, formApi); return { ...finalComponentProps, ...dynamicComponentProps.value, }; });
216-231: Add error handling for the automatic default value assignment.While the logic is correct, consider adding error handling for cases where
formApiis undefined or when the change handler throws an error.onMounted(() => { if ( autoDefaultValue && Reflect.has(commonComponentProps, 'change') && defaultValue ) { + try { const value = (formApi && formApi.values[fieldName]) ?? defaultValue; commonComponentProps.change(value, { e: value, ...computedProps.value, emptyStateValue, formApi, name: fieldName, }); + } catch (error) { + console.error('Failed to set default value:', error); + } } });
243-254: Add type safety to event handlers.The event handling logic is comprehensive but could benefit from TypeScript type definitions for the event parameters and handler functions.
+interface ChangeHandlerParams { + e: any[]; + formApi: any; + emptyStateValue?: any; + [key: string]: any; +} +type ChangeHandler = (value: any, params: ChangeHandlerParams) => void; -const change = (...e: any) => { +const change = (...e: any[]): void => { handler(...e); if (Reflect.has(commonComponentProps, 'change')) { - commonComponentProps.change(e[0], { + (commonComponentProps.change as ChangeHandler)(e[0], { e, ...slotProps.componentField, ...computedProps.value, [bindEventField || '']: value === undefined ? emptyStateValue : value, formApi, }); } };Also applies to: 294-309, 318-323
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
apps/web-antd/src/adapter/form.ts(1 hunks)apps/web-ele/src/adapter/form.ts(1 hunks)apps/web-naive/src/adapter/form.ts(1 hunks)docs/src/_env/adapter/form.ts(1 hunks)docs/src/components/common-ui/vben-form.md(2 hunks)packages/@core/ui-kit/form-ui/src/form-render/form-field.vue(8 hunks)packages/@core/ui-kit/form-ui/src/form-render/form.vue(2 hunks)packages/@core/ui-kit/form-ui/src/types.ts(2 hunks)packages/@core/ui-kit/shadcn-ui/src/components/pin-input/input.vue(2 hunks)playground/src/adapter/form.ts(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: post-update (windows-latest)
- GitHub Check: post-update (ubuntu-latest)
🔇 Additional comments (7)
apps/web-naive/src/adapter/form.ts (1)
35-40: Verify null handling for Naive UI componentsGiven the comment that Naive UI components use
nullfor empty values (notundefined), verify that themultipleRequiredvalidation handles this correctly. Consider adding a test case specifically for Naive UI's null behavior.✅ Verification successful
Null handling for Naive UI components is correctly implemented ✅
The
multipleRequiredvalidation properly handles Naive UI's null-based empty values, and the form configuration ensures consistent behavior during form resets. No changes needed.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for tests related to form validation with null values rg -l "null.*multipleRequired|multipleRequired.*null" --type tsLength of output: 65
Script:
#!/bin/bash # Search for comments and code related to Naive UI form handling rg -g "*.ts" -g "*.vue" "naive.*form|form.*naive" -A 2 -B 2 # Search for validation rule implementations and usage ast-grep --pattern 'const $_ = { $$$ multipleRequired: $_, $$$ }'Length of output: 135
Script:
#!/bin/bash # Look for form validation rules in the codebase rg -g "*.ts" "formRules|validation.*rules" -A 2 -B 2 # Check the specific adapter file content cat apps/web-naive/src/adapter/form.tsLength of output: 6897
packages/@core/ui-kit/shadcn-ui/src/components/pin-input/input.vue (1)
64-65: Verify the necessity of the early return condition.The early return check
if (!bool)will never execute sincehandleSendCodealways returnstrue.If early return is needed, consider implementing actual validation in
handleSendCode.packages/@core/ui-kit/form-ui/src/form-render/form.vue (1)
88-88: LGTM! Clean integration of autoDefaultValue.The new property is properly integrated into both the configuration destructuring and the computed schema, following the established patterns.
Also applies to: 115-115
packages/@core/ui-kit/form-ui/src/types.ts (1)
140-143: LGTM! Well-documented property addition.The
autoDefaultValueproperty is properly documented and follows the established pattern for form configuration options.packages/@core/ui-kit/form-ui/src/form-render/form-field.vue (2)
29-34: LGTM! Props are correctly defined.The new
autoDefaultValueprop is properly initialized with a default value offalse, anddefaultValueis correctly destructured from props.
114-116: LGTM! Validation rule check is properly updated.The
shouldRequiredcomputed property now correctly includes the new 'multipleRequired' validation rule.docs/src/components/common-ui/vben-form.md (1)
51-56: LGTM! Documentation is clear and comprehensive.The documentation clearly explains both the new
multipleRequiredvalidation rule and theautoDefaultValueproperty, including their purposes and usage.Also applies to: 368-371
wingjie
changed the title
wingjie
changed the title
|
1、添加判断来决定是否触发倒计时，这样可以先校验手机邮箱号，通过后再进行开始计时； 2、增加了多项选择的强制检查预设；3、为表单的每一项添加一个触发钩子函数和一个全局钩子函数，方便开发需要 |
wingjie
changed the title
Description
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
Release Notes
New Features
multipleRequiredvalidation rule for form fields to ensure multiple selections are madeautoDefaultValueconfiguration to automatically set default values for form fieldsImprovements
Documentation