-
Notifications
You must be signed in to change notification settings - Fork 7.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: add examples of asynchronous form verification and verification time #4559
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughThe pull request introduces updates to the Vben Form component documentation and related source code. Key changes include the addition of type imports and a new utility function for managing default placeholders. Additionally, the form setup configuration has been modified to disable the onChange event listener for Naive UI components. The Changes
Assessment against linked issues
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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (4)
packages/@core/ui-kit/form-ui/src/types.ts (2)
36-43
: LGTM: New FormFieldOptions type enhances validation control.The introduction of
FormFieldOptions
type is a great addition. It extendsFieldOptions
with specific boolean properties for different validation triggers, providing more granular control over form field validation behavior. This aligns well with the PR objective of enhancing form validation capabilities.Consider adding JSDoc comments for each new property to improve code documentation:
export type FormFieldOptions = Partial< { /** Trigger validation when the field loses focus */ validateOnBlur?: boolean; /** Trigger validation when the field's value changes */ validateOnChange?: boolean; /** Trigger validation when the user types in the field */ validateOnInput?: boolean; /** Trigger validation when the field's v-model is updated */ validateOnModelUpdate?: boolean; } & FieldOptions >;
160-160
: LGTM: FormCommonConfig updated to use new FormFieldOptions type.The update of
formFieldProps
inFormCommonConfig
to use the newFormFieldOptions
type is consistent and appropriate. This change allows for more specific and granular configuration of form field properties across all form items, which aligns well with the PR objective of enhancing form validation capabilities.Consider updating the JSDoc comment for
formFieldProps
to reflect its new capabilities:/** * Configuration options for all form fields, including validation triggers * @default {} */ formFieldProps?: FormFieldOptions;docs/src/components/common-ui/vben-form.md (2)
88-96
: LGTM: Useful utility for default placeholdersThe
withDefaultPlaceholder
function is a great addition that enhances form usability by automatically providing default placeholders. It's well-implemented using Vue'sh
function and is generic enough to be used with various form components.Consider adding a brief comment explaining the purpose of this function for better maintainability.
131-132
: LGTM: Improved compatibility with Naive UIThe addition of the
disabledOnChangeListener
option set totrue
is a good solution to prevent console errors when using the Naive UI component library. This demonstrates careful consideration of framework-specific behaviors.Consider adding a brief comment explaining why this option is necessary for Naive UI, to help future maintainers understand the rationale behind this configuration.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
- docs/src/components/common-ui/vben-form.md (3 hunks)
- packages/@core/ui-kit/form-ui/src/types.ts (3 hunks)
- playground/src/views/examples/form/rules.vue (1 hunks)
🔇 Additional comments (8)
packages/@core/ui-kit/form-ui/src/types.ts (2)
2-2
: LGTM: Import statement updated correctly.The change from importing
Field
toFieldOptions
from 'vee-validate' aligns well with the newFormFieldOptions
type being introduced. This update suggests a more focused use of field options, which is a good practice for type specificity.
Line range hint
1-300
: Summary: Changes enhance form validation capabilities as intended.The modifications in this file successfully address the PR objectives by introducing more granular control over form field validation. The new
FormFieldOptions
type and its integration intoFormCommonConfig
provide a robust foundation for implementing various validation trigger methods and potentially supporting asynchronous validation.Key improvements:
- Updated import statement for more specific type usage.
- Introduction of
FormFieldOptions
type with detailed validation trigger options.- Integration of
FormFieldOptions
intoFormCommonConfig
for consistent usage across the form.These changes lay the groundwork for implementing the examples of validation trigger methods and asynchronous validation as requested in the linked issue #4531. The code structure is clean and consistent with the existing codebase.
To ensure these changes don't introduce any breaking changes or inconsistencies, please run the following verification script:
This script will help identify any potential issues or inconsistencies introduced by these changes.
docs/src/components/common-ui/vben-form.md (4)
34-35
: LGTM: Improved type safety with Vue importsThe addition of
Component
andSetupContext
type imports from Vue enhances type safety in the code. This change aligns well with the PR objectives of improving the useVbenForm demonstration.
114-117
: LGTM: Consistent application of default placeholdersThe
withDefaultPlaceholder
function is now applied to multiple form components (Input, InputNumber, InputPassword, Mentions, Select, Textarea, TreeSelect). This ensures consistent behavior across different form elements and improves overall usability, aligning well with the PR objectives.Also applies to: 122-122, 125-125, 127-127
Line range hint
135-140
: LGTM: Improved model property mappingsThe updates to
modelPropNameMap
provide clear and specific mappings for different form components (Checkbox, Radio, Switch, Upload). This enhances the flexibility of the form component, allowing it to work correctly with various types of form elements and their respective v-model bindings.
Line range hint
1-1000
: Overall assessment: Excellent enhancements to Vben FormThis PR successfully addresses the objectives outlined in issue #4531 by enhancing the useVbenForm demonstration. The changes improve type safety, add useful utilities for default placeholders, ensure consistent behavior across form components, and enhance compatibility with different UI frameworks. These improvements will significantly enhance the usability and functionality of the form verification process within the Vben Admin framework.
Key improvements:
- Added type imports for better type safety
- Introduced a reusable
withDefaultPlaceholder
utility- Applied consistent default placeholders across multiple form components
- Added configuration to prevent console errors with Naive UI
- Updated model property mappings for various form elements
The changes are well-implemented and align with best practices. Minor suggestions for additional comments have been made to improve maintainability.
playground/src/views/examples/form/rules.vue (2)
178-191
: Addition of 'input-blur' field effectively demonstrates blur-triggered validationThe 'input-blur' field correctly sets up validation to trigger only on blur events by configuring
validateOnChange
andvalidateOnModelUpdate
tofalse
. This aligns well with the PR objectives to showcase different validation triggers.
192-218
: Ensure the form supports asynchronous validationThe asynchronous validation for the 'input-async' field is correctly implemented using
z.refine
with an async function. Please verify that the form handling library supports asynchronous validations and that the form is configured to handle them properly to ensure the validation works as intended.
Description
close #4531
Type of change
Please delete options that are not relevant.
pnpm-lock.yaml
unless you introduce a new test example.Checklist
pnpm run docs:dev
command.pnpm test
.feat:
,fix:
,perf:
,docs:
, orchore:
.Summary by CodeRabbit
New Features
Bug Fixes
Documentation