-
Notifications
You must be signed in to change notification settings - Fork 6.9k
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
fix: form data that is not submitted by the form should not be carried when switching paging #4847
Conversation
…d when switching paging
|
WalkthroughThe changes in this pull request enhance 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: 0
🧹 Outside diff range and nitpick comments (4)
packages/@core/ui-kit/form-ui/src/form-api.ts (3)
45-46
: Translate Chinese comment to English for better accessibility.Consider translating the Chinese comment to English:
-// 最后一次点击提交时的表单值 +// The form values from the last submission
219-221
: Consider aligning null handling between getter and setter.The getter never returns null (falls back to empty object), but the setter accepts null. Consider either:
- Making the setter reject null values to maintain consistency, or
- Documenting why null values are accepted in the setter but transformed in the getter
-setLatestSubmissionValues(values: null | Recordable<any>) { +setLatestSubmissionValues(values: Recordable<any>) { this.latestSubmissionValues = { ...toRaw(values) }; }
Line range hint
261-266
: Consider updating latestSubmissionValues in submitForm.The submitForm method calculates rawValues but doesn't update latestSubmissionValues. Consider updating it to maintain consistency with the form's submission state:
async submitForm(e?: Event) { e?.preventDefault(); e?.stopPropagation(); const form = await this.getForm(); await form.submitForm(); const rawValues = toRaw(form.values || {}); await this.state?.handleSubmit?.(rawValues); + this.setLatestSubmissionValues(rawValues); return rawValues; }
packages/effects/plugins/src/vxe-table/use-vxe-grid.vue (1)
Line range hint
69-232
: Well-structured solution for form state management.The implementation follows a clean architecture where:
- Form values are explicitly captured at submission points (submit/reset)
- These values are stored in a dedicated state (
latestSubmissionValues
)- Grid operations consistently use these stored values
This approach effectively solves the issue of outdated form data being carried over during paging while maintaining a clear separation of concerns between form state and grid operations.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
packages/@core/ui-kit/form-ui/src/form-api.ts
(6 hunks)packages/effects/plugins/src/vxe-table/use-vxe-grid.vue
(2 hunks)
🔇 Additional comments (6)
packages/@core/ui-kit/form-ui/src/form-api.ts (3)
117-119
: LGTM! Well-designed getter method.
The method appropriately handles null values by returning an empty object, preventing potential null reference issues.
175-175
: LGTM! Proper initialization of submission values.
The mount method correctly initializes the submission values with the current form state, ensuring proper state management from the start.
272-272
: LGTM! Proper cleanup in unmount.
The unmount method appropriately cleans up the submission values state, preventing any potential issues with stale data.
packages/effects/plugins/src/vxe-table/use-vxe-grid.vue (3)
69-69
: LGTM: Properly stores form values at submission time.
The addition of setLatestSubmissionValues
with toRaw
ensures that only explicitly submitted form values are stored, directly addressing the issue of outdated form data being carried over during paging.
75-75
: LGTM: Reset properly updates stored form values.
The addition ensures that form reset operations correctly update the stored submission values, maintaining consistency with the new form state.
230-232
: LGTM: Proxy operations now use latest submitted values.
The change ensures that grid operations like paging correctly use the latest submitted form values instead of potentially stale form data.
Let's verify the initialization behavior:
✅ Verification successful
Based on the code analysis, I can now generate the final response since I have found clear evidence about the initialization behavior of getLatestSubmissionValues
:
Initialization of form values is properly handled
The implementation is safe because:
latestSubmissionValues
is initialized asnull
andgetLatestSubmissionValues()
returns an empty object{}
when it's null- During form mount,
latestSubmissionValues
is properly initialized with initial form values viasetLatestSubmissionValues({ ...toRaw(this.form.values) })
- When form is unmounted,
latestSubmissionValues
is reset tonull
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if getLatestSubmissionValues has proper initialization handling
# Look for initialization of latestSubmissionValues in FormApi class
# Search for FormApi class definition and initialization
ast-grep --pattern 'class FormApi {
$$$
private latestSubmissionValues
$$$
}'
# Search for getLatestSubmissionValues implementation
ast-grep --pattern 'getLatestSubmissionValues() {
$$$
}'
Length of output: 21828
Description
fixed #4617
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