feat: add form handleCollapsedChange event #6893
Conversation
Elm1992
requested review from
a team,
anncwb,
jinmao88,
mynetfan and
vince292007
as code owners
|
WalkthroughAdded an optional Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant UI as User UI (vben-form)
participant Comp as vben-use-form / vben-form component
participant Consumer as Consumer (parent)
note right of Comp `#e6f7ff`: Collapse toggle triggered
UI ->> Comp: toggleCollapsed(value: boolean)
alt collapsed value changed
Comp ->> Comp: update internal state.collapsed = value
Comp ->> Consumer: call handleCollapsedChange?(value)
note right of Consumer `#f0fff0`: optional callback invoked
else no change
note right of Comp `#fff7e6`: no callback
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20–30 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
🔇 Additional comments (3)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/@core/ui-kit/form-ui/src/vben-use-form.vue (1)
46-51: Consider simplifying boolean coercion.The
!!valuecoercion is defensive but unnecessary sincevalueis already typed asboolean. You could simplify toconst collapsedValue = value;or usevaluedirectly. However, the defensive coding provides runtime safety if the type contract is violated.Apply this diff if you prefer a cleaner approach:
const handleUpdateCollapsed = (value: boolean) => { - const collapsedValue = !!value; - props.formApi?.setState({ collapsed: collapsedValue }); + props.formApi?.setState({ collapsed: value }); // 触发收起展开状态变化回调 - forward.value.handleCollapsedChange?.(collapsedValue); + forward.value.handleCollapsedChange?.(value); };packages/@core/ui-kit/form-ui/src/vben-form.vue (1)
42-47: Consider simplifying boolean coercion (same pattern as vben-use-form.vue).Similar to the implementation in
vben-use-form.vue, the!!valuecoercion is unnecessary given the typed parameter. The defensive coding is acceptable but could be simplified for clarity.Apply this diff for a cleaner approach:
const handleUpdateCollapsed = (value: boolean) => { - const collapsedValue = !!value; - currentCollapsed.value = collapsedValue; + currentCollapsed.value = value; // 触发收起展开状态变化回调 - props.handleCollapsedChange?.(collapsedValue); + props.handleCollapsedChange?.(value); };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
docs/src/components/common-ui/vben-form.md(1 hunks)packages/@core/ui-kit/form-ui/src/form-api.ts(1 hunks)packages/@core/ui-kit/form-ui/src/types.ts(1 hunks)packages/@core/ui-kit/form-ui/src/use-form-context.ts(1 hunks)packages/@core/ui-kit/form-ui/src/vben-form.vue(1 hunks)packages/@core/ui-kit/form-ui/src/vben-use-form.vue(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
packages/@core/ui-kit/form-ui/src/use-form-context.ts (1)
packages/@core/ui-kit/form-ui/src/types.ts (2)
VbenFormProps(350-434)ExtendedFormApi(436-440)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: Lint (ubuntu-latest)
- GitHub Check: Lint (windows-latest)
- GitHub Check: Check (windows-latest)
- GitHub Check: Check (ubuntu-latest)
- GitHub Check: Test (windows-latest)
- GitHub Check: post-update (ubuntu-latest)
- GitHub Check: post-update (windows-latest)
🔇 Additional comments (3)
packages/@core/ui-kit/form-ui/src/types.ts (1)
382-385: LGTM! Type definition is correct.The optional callback property is properly typed with a clear signature. The documentation comment clearly describes its purpose.
packages/@core/ui-kit/form-ui/src/form-api.ts (1)
39-39: LGTM! Default state initialization is correct.The initialization follows the established pattern for other callback handlers in the default state.
docs/src/components/common-ui/vben-form.md (1)
338-338: LGTM! Documentation is complete and accurate.The prop is properly documented with correct type signature and clear description.
| import { getDefaultsForSchema } from 'zod-defaults'; | ||
|
|
||
| type ExtendFormProps = VbenFormProps & { formApi: ExtendedFormApi }; | ||
| type ExtendFormProps = VbenFormProps & { formApi?: ExtendedFormApi }; |
There was a problem hiding this comment.
🧩 Analysis chain
Verify that all consumers handle optional formApi.
Making formApi optional in ExtendFormProps could impact consumers that inject form props via injectFormProps. Ensure all usages safely handle the case where formApi is undefined.
Run the following script to verify all formApi usages properly handle the optional case:
🏁 Script executed:
#!/bin/bash
# Description: Find all usages of injectFormProps and verify optional chaining is used for formApi access
# Find all usages of injectFormProps
echo "=== Finding injectFormProps usages ==="
rg -n "injectFormProps" --type=ts --type=vue -A 5
# Find direct formApi property accesses that might not use optional chaining
echo -e "\n=== Finding potentially unsafe formApi accesses ==="
rg -n "formApi\.[a-zA-Z]" --type=ts --type=vue -C 2 | rg -v "formApi\?\."Length of output: 147
🏁 Script executed:
#!/bin/bash
# Description: Find injectFormProps usages and formApi accesses without vue type constraints
# First, look at the file in question
echo "=== Viewing use-form-context.ts ==="
find . -name "use-form-context.ts" -type f | head -5 | xargs cat 2>/dev/null || echo "File not found"
echo -e "\n=== Finding injectFormProps usages (without type filter) ==="
rg -n "injectFormProps" -A 5 2>/dev/null | head -100
echo -e "\n=== Finding formApi direct accesses (without optional chaining) ==="
rg -n "\.formApi\." -C 2 2>/dev/null | head -100
echo -e "\n=== Finding formApi accesses with optional chaining ==="
rg -n "\.formApi\?" -C 2 2>/dev/null | head -50
echo -e "\n=== Finding all formApi references in context ==="
rg -n "formApi" --max-count=50 2>/dev/nullLength of output: 27507
Fix unsafe formApi accesses in vben-use-form.vue that don't account for optional value.
Making formApi optional in ExtendFormProps introduces a type mismatch with consumers. While form-actions.vue safely guards with if (!props.formApi) before access, vben-use-form.vue directly accesses formApi without protection:
- Line 64:
forward.value.formApi.validateAndSubmitForm() - Line 98:
cloneDeep(await forward.value.formApi.getValues())
Additionally, the Props type declaration in vben-use-form.vue (line 28) declares formApi: ExtendedFormApi as required, which conflicts with the now-optional injection contract. Update the Props declaration to formApi?: ExtendedFormApi and protect these two accesses with optional chaining or guard conditions.
🤖 Prompt for AI Agents
In packages/@core/ui-kit/form-ui/src/use-form-context.ts line 16 and
packages/@core/ui-kit/form-ui/src/components/vben-use-form.vue lines 28, 64, 98,
update types and guard unsafe accesses: change the vben-use-form.vue Props
declaration from formApi: ExtendedFormApi to formApi?: ExtendedFormApi to match
ExtendFormProps, and protect usages at lines 64 and 98 by checking for presence
(if (forward.value.formApi) ...) or using optional chaining
(forward.value.formApi?.validateAndSubmitForm(), cloneDeep(await
forward.value.formApi?.getValues() ?? {})) so no code assumes formApi is
non-null. Ensure any downstream typing uses the optional type.
1 participant
Description
增加 form 组件展开收起的事件，我的需求是收起的时候要清空所有收起项，应该还有些别的需求吧。
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
New Features
Behavior Changes