fix: form fieldMappingTime is not working
#5333
Conversation
|
|
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
packages/@core/ui-kit/form-ui/src/components/form-actions.vueOops! 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 focuses on enhancing form handling, particularly for RangePicker components and time-related field mappings. The changes span multiple files in the form UI kit, introducing a new Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
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 (4)
playground/src/views/examples/vxe-table/form.vue (1)
63-65: Simplify the date range calculation.The default value calculation uses a negative number in
subtract(-7, 'days')which is counterintuitive.- defaultValue: [dayjs().subtract(-7, 'days'), dayjs()], + defaultValue: [dayjs().add(7, 'days'), dayjs()],packages/effects/plugins/src/vxe-table/use-vxe-grid.vue (1)
251-253: Remove commented code.The commented code on line 252 should be removed as it's no longer needed.
- // props.api.reload(formApi.form?.values ?? {});packages/@core/ui-kit/form-ui/src/form-api.ts (2)
347-387: Clean up commented code alternatives.The implementation contains commented alternatives for property deletion. Choose one approach consistently.
- // delete values[startTimeKey]; - // delete values[endTimeKey]; - // delete values[field];Additionally, consider extracting the time format processing logic into a separate utility function for better maintainability.
375-380: Consider adding validation for time format strings.The code assumes that the format strings are valid. Consider adding validation to prevent runtime errors.
+ if (!startTimeFormat || !endTimeFormat) { + console.warn(`Invalid time format for field ${field}`); + return; + } values[startTimeKey] = startTime ? formatDate(startTime, startTimeFormat) : undefined;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
packages/@core/ui-kit/form-ui/src/components/form-actions.vue(3 hunks)packages/@core/ui-kit/form-ui/src/form-api.ts(6 hunks)packages/@core/ui-kit/form-ui/src/use-form-context.ts(2 hunks)packages/@core/ui-kit/form-ui/src/vben-use-form.vue(1 hunks)packages/effects/plugins/src/echarts/use-echarts.ts(1 hunks)packages/effects/plugins/src/vxe-table/use-vxe-grid.vue(3 hunks)playground/src/views/examples/vxe-table/form.vue(3 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/effects/plugins/src/echarts/use-echarts.ts
⏰ Context from checks skipped due to timeout of 90000ms (9)
- GitHub Check: Test (windows-latest)
- GitHub Check: Test (ubuntu-latest)
- GitHub Check: Lint (windows-latest)
- GitHub Check: Check (windows-latest)
- GitHub Check: post-update (windows-latest)
- GitHub Check: post-update (ubuntu-latest)
- GitHub Check: Lint (ubuntu-latest)
- GitHub Check: Check (ubuntu-latest)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (9)
packages/@core/ui-kit/form-ui/src/use-form-context.ts (1)
5-5: LGTM! Type definitions properly extended to support form API.The addition of
ExtendFormPropstype and its usage increateContextensures proper typing for the form API integration.Also applies to: 16-19
packages/@core/ui-kit/form-ui/src/components/form-actions.vue (2)
56-58: LGTM! Improved form value handling architecture.Values are now retrieved directly from formApi, centralizing the responsibility of value handling and ensuring consistent processing of time fields.
Line range hint
65-71: Verify reset behavior for time fields.The reset logic no longer explicitly handles time fields. Ensure that the reset functionality in formApi properly handles the time field mappings.
✅ Verification successful
Time field reset behavior is properly implemented
The reset functionality correctly handles time fields through:
- Preservation of time field mappings in form state
- Proper cleanup of time field values when null
- Integration with vee-validate's core reset functionality
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for time field reset handling in formApi ast-grep --pattern 'resetForm() { $$$ }'Length of output: 44
Script:
#!/bin/bash # Find form-related files fd "form" --type f # Search for reset-related functions and time field handling rg -l "reset.*form|resetForm|time.*field" --type vue --type ts --type js # Try alternative ast-grep pattern for reset methods ast-grep --pattern 'reset($$$) { $$$ }'Length of output: 1528
Script:
#!/bin/bash # Check form API implementation cat packages/@core/ui-kit/form-ui/src/form-api.ts # Check form API tests cat packages/@core/ui-kit/form-ui/__tests__/form-api.test.ts # Search with fixed file type syntax rg "reset.*form|resetForm|time.*field" -t js -t ts -t vueLength of output: 15827
packages/@core/ui-kit/form-ui/src/vben-use-form.vue (1)
56-60: LGTM! Robust value change handling implementation.The changes properly handle form value updates by:
- Using async/await for value retrieval
- Preventing mutations with cloneDeep
- Maintaining a reasonable debounce timing
playground/src/views/examples/vxe-table/form.vue (1)
25-25: LGTM! Proper configuration of fieldMappingTime.The field mapping correctly maps the 'date' field to ['start', 'end'] for the RangePicker component.
packages/effects/plugins/src/vxe-table/use-vxe-grid.vue (2)
11-12: LGTM! Type safety improvement.The addition of
SetupContextimport and explicit typing ofslotsenhances type safety and maintainability.Also applies to: 73-73
78-86: LGTM! Improved form value handling.The switch to
await formApi.getValues()ensures proper processing of form values, particularly for time-related fields, making the implementation more reliable.packages/@core/ui-kit/form-ui/src/form-api.ts (2)
370-373: LGTM! Flexible time format handling.The implementation elegantly handles both single and array format specifications for start and end times.
357-362: Verify null handling behavior.The code deletes both time fields when the range field is null. Please verify if this is the intended behavior, as it might affect form validation or downstream processing.
✅ Verification successful
Null handling behavior is correct and consistent
The code follows a consistent pattern where time range fields are cleaned up when their parent range field is null. This prevents orphaned time fields and maintains data consistency.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for usages of fieldMappingTime to understand the expected behavior rg -A 5 "fieldMappingTime.*=.*\[" .Length of output: 37
Script:
#!/bin/bash # Search for startTimeKey and endTimeKey usage rg -A 5 "startTimeKey|endTimeKey" . # Search for form validation patterns near time fields rg -B 5 -A 5 "validate.*Time" . # Look for similar Reflect.deleteProperty patterns ast-grep --pattern 'Reflect.deleteProperty($object, $key)'Length of output: 1855
|
更新后，不带搜索表单的表格，第一次自动加载失效了 |
等待修复 |
Description
修复表单的fieldMappingTime在直接点击提交按钮以外的场合不起作用的问题。
受到影响的使用方法：带搜索表单的表格第一次自动加载、submitOnChange、submitOnEnter 等
fix: #5289
fix: #5186
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
Improvements
Technical Enhancements