feat: add fieldMapToTime prop to FormRenderProps #4838
Conversation
|
WalkthroughThe pull request introduces enhancements to form handling in 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 (
|
![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 (6)
playground/src/views/examples/vxe-table/form.vue (2)
66-70: Consider adding componentProps for better UXThe RangePicker component could benefit from additional configuration:
{ component: 'RangePicker', fieldName: 'dateRangePicker', label: 'DateRange', + componentProps: { + placeholder: ['Start Date', 'End Date'], + format: 'YYYY-MM', + valueType: 'timestamp', + allowClear: true + } },This would:
- Add helpful placeholders
- Ensure consistent date formatting
- Specify value type for form submission
- Allow clearing the selection
Line range hint
24-70: Enhance example implementation with validation and error handlingSince this is an example implementation in the playground, consider enhancing it to demonstrate best practices:
- Add form validation rules for the date range
- Show error handling for invalid date selections
- Add comments explaining the fieldMapToTime configuration
Would you like me to provide an example of these enhancements?
packages/@core/ui-kit/form-ui/src/components/form-actions.vue (1)
64-74: Consider improving the destructuring pattern.While the time field cleanup logic is correct, the destructuring pattern
[_, [startTimeKey, endTimeKey]]could be more explicit.Consider this improvement:
- props.fieldMapToTime.forEach(([_, [startTimeKey, endTimeKey]]) => { + props.fieldMapToTime.forEach(([fieldName, [startTimeKey, endTimeKey]]) => {This makes the code more self-documenting by using a meaningful variable name instead of the underscore.
packages/@core/ui-kit/form-ui/src/types.ts (3)
208-212: Enhance type definition with documentation and type aliases.While the type structure is correct, consider these improvements for better maintainability and developer experience:
+/** Time format string for date-fns or moment */ +type TimeFormat = string; + +/** Tuple of field names for start and end time */ +type TimeFieldNames = [string, string]; + +/** Format specification for time fields - either a single format or separate formats for start/end */ +type TimeFormatSpec = [TimeFormat, TimeFormat] | TimeFormat; + +/** + * Maps form fields to time range fields with optional formatting + * @example + * const fieldMapToTime: FieldMapToTime = [ + * // Map 'dateRange' field to 'startTime' and 'endTime' with 'yyyy-MM-dd' format + * ['dateRange', ['startTime', 'endTime'], 'yyyy-MM-dd'], + * // Map with different formats for start and end times + * ['timeRange', ['start', 'end'], ['HH:mm', 'HH:mm:ss']] + * ]; + */ -export type FieldMapToTime = [ - string, - [string, string], - ([string, string] | string)?, -][]; +export type FieldMapToTime = [ + string, + TimeFieldNames, + TimeFormatSpec?, +][];
274-277: Enhance property documentation with examples and use cases.The documentation could be more descriptive to help developers understand the property's purpose and usage.
- /** - * 表单字段映射成时间格式 - */ + /** + * Maps form fields to time range fields with optional formatting. + * Use this to transform time range picker values into separate start/end time fields + * during form submission. + * @example + * fieldMapToTime: [ + * // Basic mapping with default format + * ['dateRange', ['startDate', 'endDate']], + * // With custom format + * ['timeRange', ['startTime', 'endTime'], 'HH:mm:ss'], + * // With different formats for start/end + * ['customRange', ['start', 'end'], ['yyyy-MM-dd', 'dd/MM/yyyy']] + * ] + */
Implementation needs additional time formatting functionality
The current implementation only defines the
FieldMapToTimetype but lacks the complete implementation for handling time range fields. While there is a basicformatTimefunction inform-actions.vue, it doesn't fully support the time range mapping functionality described in the type definition. Specifically:
- The
formatTimefunction only handles single timestamps- No implementation found for handling the time range arrays as defined in
FieldMapToTimetype- Missing logic for processing the optional third element in the
FieldMapToTimetupleThe implementation should be extended to include:
- Support for processing time range arrays
[string, string]- Logic for handling the optional format parameter
- Integration with the form value transformation pipeline
🔗 Analysis chain
Line range hint
208-277: Implementation successfully addresses the requirements.The type system implementation effectively supports the time range field formatting feature requested in issue #4642. The solution is:
- Type-safe and flexible
- Backward compatible
- Well-integrated into the existing form props structure
Let's verify the integration with the form component:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the form component's usage of fieldMapToTime # Check for form component implementations using fieldMapToTime echo "Checking for form component implementations..." rg -l "fieldMapToTime.*:\s*\[" --type ts # Check for any existing time-related form field handling echo "Checking for existing time field handling..." rg -l "time.*format|format.*time" --type tsLength of output: 290
Script:
#!/bin/bash # Let's try a broader search to understand the implementation and usage # Check for any references to the new FieldMapToTime type echo "Checking for FieldMapToTime references..." rg "FieldMapToTime" --type ts -A 2 # Check for time-related form field transformations echo "Checking for time field transformations..." rg "getFieldsValue|setFieldsValue.*time" --type ts -A 3 # Look for form component implementations echo "Looking for main form component..." fd -e ts -e tsx "Form\.tsx$|Form\.ts$"Length of output: 773
Script:
#!/bin/bash # Let's check the implementation details more thoroughly # Look for the form component implementation in the form-ui directory echo "Checking form-ui directory for implementation..." rg -t ts -t tsx "class.*Form|function.*Form" packages/@core/ui-kit/form-ui/src/ -A 5 # Check for any time-related transformations or utilities echo "Checking for time-related utilities..." rg "transform.*time|format.*time|time.*format" packages/@core/ui-kit/form-ui/src/ -A 3 # Check for form value handling echo "Checking form value handling..." rg "handleFormValues|transformFormValues" packages/@core/ui-kit/form-ui/src/ -A 5Length of output: 1474
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
packages/@core/ui-kit/form-ui/src/components/form-actions.vue(1 hunks)packages/@core/ui-kit/form-ui/src/types.ts(2 hunks)playground/src/views/examples/vxe-table/form.vue(2 hunks)
🔇 Additional comments (2)
playground/src/views/examples/vxe-table/form.vue (1)
24-24: Verify if YYYY-MM format meets requirements
The time format is set to 'YYYY-MM' which only captures year and month. Please confirm if this is the intended format or if it should include day/time components (e.g., 'YYYY-MM-DD').
packages/@core/ui-kit/form-ui/src/components/form-actions.vue (1)
55-57: LGTM: Time range processing added to form submission.
The addition of handleRangeTimeValue properly processes time range fields before form submission, aligning with the PR objectives.
Let's verify the form submission handling:
✅ Verification successful
Time range processing is correctly isolated to form-actions component
The handleRangeTimeValue function is only defined and used in form-actions.vue, which is the centralized component for form submissions in the UI kit. This is the correct place for this functionality as:
- It's part of the core form submission flow in
@core/ui-kit/form-ui - The function is conditionally applied based on
fieldMapToTimeprop, making it an opt-in feature - All form submissions in the codebase go through this component, ensuring consistent time range processing when needed
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for form submission handlers to ensure consistent time range processing
ast-grep --pattern 'handleSubmit($$$) {
$$$
handleRangeTimeValue($$$)
$$$
}'
Length of output: 84
Script:
#!/bin/bash
# Let's first check where handleRangeTimeValue is defined
rg "handleRangeTimeValue" -A 3
# Also search for other form submission handlers
rg "handleSubmit" -A 3
# Check for any other time range processing in form submissions
ast-grep --pattern 'const values = $$$($$$form.values$$$)'
Length of output: 24305
Description
feat: add fieldMapToTime prop to FormRenderProps
fix: #4642 Form表单添加fieldMapToTime格式化时间范围字段
Type of change
Please delete options that are not relevant.
pnpm-lock.yamlunless you introduce a new test example.Summary by CodeRabbit
New Features
RangePickercomponent for selecting date ranges in forms.Type Definitions
FieldMapToTimeadded to support mapping form fields to time formats.