feat(form): add merge form functionality #4495
Conversation
chore(deps): bump tailwindcss in the non-breaking-changes group (vbenjs#4369)
|
WalkthroughThe changes introduce enhancements to the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Recent review detailsConfiguration used: CodeRabbit UI Files selected for processing (1)
Files skipped from review as they are similar to previous changes (1)
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: 6
Outside diff range and nitpick comments (5)
packages/effects/common-ui/src/components/captcha/hooks/useCaptchaPoints.ts (1)
11-13: LGTM! Consider a more concise alternative.The
clearPoints()function correctly clears all points from the array. It aligns well with the PR objectives of enhancing form handling capabilities.For a more concise implementation, consider:
function clearPoints() { - points.splice(0, points.length); + points.length = 0; }This achieves the same result but is slightly more efficient and readable.
playground/src/router/routes/modules/examples.ts (1)
102-109: LGTM! Consider a minor adjustment for consistency.The new
FormMergeExampleroute is well-structured and consistent with the existing routes. It correctly uses dynamic import for the component and follows the established pattern for internationalization.For complete consistency with other form example routes, consider adding an empty line before this route definition. This would match the spacing pattern seen in the rest of the file. Here's a suggested change:
}, + { name: 'FormMergeExample', path: '/examples/form/merge', component: () => import('#/views/examples/form/merge.vue'), meta: { title: $t('page.examples.form.merge'), }, },playground/src/views/examples/form/merge.vue (1)
53-78: Specify Submit Button Text for SecondFormIn
SecondForm, thesubmitButtonOptionsare not specified. To maintain consistency and improve user experience, consider specifying the submit button text.Example:
const [SecondForm, secondFormApi] = useVbenForm({ // ...existing configurations + submitButtonOptions: { + text: '提交', + }, schema: [ // ...existing schema ], });packages/@core/ui-kit/form-ui/src/form-api.ts (2)
127-159: Specify proper types for proxy handler parametersIn the proxy's
gethandler, the parameterstargetandpropare currently typed asany. To improve type safety and leverage TypeScript's capabilities, consider specifying more precise types for these parameters.Apply this diff to specify types:
const proxy = new Proxy(formApi, { - get(target: any, prop: any) { + get(target: FormApi, prop: string | symbol) {
124-163: Prevent potential memory leaks from unboundedchaingrowthThe
mergemethod creates a closure over thechainarray and returns a proxy that allows chainingFormApiinstances indefinitely. If not managed carefully, this could lead to memory leaks due to thechainarray retaining references to all mergedFormApiinstances.Consider implementing a mechanism to manage the size of the
chainarray or provide a way to dispose of it when it's no longer needed.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (6)
- packages/@core/ui-kit/form-ui/src/form-api.ts (1 hunks)
- packages/effects/common-ui/src/components/captcha/hooks/useCaptchaPoints.ts (1 hunks)
- playground/src/locales/langs/en-US.json (1 hunks)
- playground/src/locales/langs/zh-CN.json (1 hunks)
- playground/src/router/routes/modules/examples.ts (1 hunks)
- playground/src/views/examples/form/merge.vue (1 hunks)
Additional comments not posted (5)
packages/effects/common-ui/src/components/captcha/hooks/useCaptchaPoints.ts (2)
Line range hint
14-18: LGTM! Proper exposure of the new function.The addition of
clearPointsto the return statement is correct. It properly exposes the new functionality to consumers of this hook, which is consistent with the PR objectives.
Line range hint
1-18: Overall assessment: Good to mergeThe changes in this file are minimal, focused, and align well with the PR objectives. The new
clearPoints()function enhances the hook's functionality by allowing users to reset the points array. The implementation is correct and non-breaking.A minor optimization suggestion was provided for the
clearPoints()function, but it's not critical for merging this PR.The changes successfully introduce the ability to merge multiple forms into a single submission, as described in the PR summary, by providing a way to clear existing points when needed.
playground/src/locales/langs/zh-CN.json (1)
82-83: LGTM! The translation aligns with the PR objectives.The addition of "merge": "合并表单" is correct and appropriate. It accurately translates "merge form" to Chinese, which is consistent with the PR's goal of introducing form merging functionality. The placement and formatting of this new entry are also correct within the existing structure.
playground/src/locales/langs/en-US.json (1)
82-83: LGTM! The new localization entry aligns with the PR objectives.The addition of
"merge": "Merge Form"is structurally correct and consistent with the existing entries. It properly introduces the localization for the new merge form feature described in the PR objectives.playground/src/views/examples/form/merge.vue (1)
11-16: FirstForm Submission Handler Works CorrectlyThe
onFirstSubmitfunction correctly handles the form submission by displaying a success message and updatingcurrentTabto proceed to the next form.
vince292007
changed the title
Description
在某些场景下，例如分步表单，需要合并多个表单并统一提交。默认情况下，使用 Object.assign 规则合并表单数据，如果多个表单有相同字段等需求，可以传参自行处理数据。
也可以对合并方法进行扩展。
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
mergefunctionality in the form API, allowing users to combine multiple forms and submit them collectively.Localization
Enhancements