Repository navigation
feat: collapsible component and support formitem collapsible - #7816
Merged
Merged
Conversation
* perf: replace `onUnMounted` to `tryOnScopeDispose` * perf: replace `onUnMounted` to `tryOnScopeDispose`
1. remove unknown rule out of oxlint 2. add the corresponding back to eslint-config 3. fixed the eslint error for package.json
- 关闭 vitest/require-mock-type-parameters 规则
- 添加 rootDir 编译选项指向 ./src 目录 - 保持现有编译配置不变 - 排除测试文件和 node_modules 目录
feat: add collapsible 组件，form表单增加单项可折叠，支持schema配置默认关闭/开启 - shadcn-ui 增加 collapsible组件，collapsible-params组件 - form新增支持单项折叠 - collapsible-params组件在Form表单应用
…llll/vue-vben-admin into feature-collapsible-component
2yllll
requested review from
a team,
anncwb,
jinmao88,
mynetfan and
vince292007
as code owners
April 14, 2026 01:54
|
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a collapsible form UI feature: new VbenCollapsible and VbenCollapsibleParams components and types, integrates collapsible behavior into form-field and form types, threads ExtendedFormApi into dependency callbacks, supplies an example/demo, locales/routes, and adds a workspace dependency for shadcn-ui. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User
participant VbenForm as VbenForm
participant Deps as useDependencies
participant Controller as ExtendedFormApi
participant VbenCollapsible as VbenCollapsible
participant VbenParams as VbenCollapsibleParams
participant Item as CollapsibleParamsItem
User->>VbenForm: interact / toggle field / submit
VbenForm->>Deps: evaluate dependency callbacks (show/if/rules/props)
Deps->>Controller: derive controller (injectFormProps) and pass it
Deps->>Controller: call callbacks (formValues, formApi, controller)
VbenForm->>VbenCollapsible: render field, bind v-model:open
VbenCollapsible->>VbenParams: manage open state, provide toggle/reset/update
VbenParams->>Item: render rows and model binding
Item->>VbenForm: emit update:value on change
User->>VbenCollapsible: click trigger -> toggle (ChevronsDown rotates)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (1)
packages/@core/ui-kit/form-ui/src/types.ts (1)
148-151: Consider documenting defaults consistently for collapse flags.
defaultCollapsedhas@default false, butcollapsiblelacks a default note. Adding@default falsetocollapsiblewould make API docs clearer and consistent.Also applies to: 164-168
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/`@core/ui-kit/form-ui/src/types.ts around lines 148 - 151, Add consistent `@default` tags for the collapse-related props: annotate collapsible with "@default false" to match defaultCollapsed, and add the same "@default false" note to the other collapse flag(s) around the same block (e.g., the prop defined near lines 164-168) so the API docs consistently show that those boolean collapse flags default to false; update the JSDoc comments for the properties named "collapsible" and the other collapse-related prop(s) referenced in this file accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/web-naive/src/views/demos/form/basic.vue`:
- Around line 120-128: The paramsValidator's max_length range disagrees with
paramsSchema's min (paramsSchema sets min: 500 while paramsValidator uses [1,
131_072]); update the max_length call in paramsValidator (the
getNumberValidator('max_length', [...] entry) to use the same minimum as
paramsSchema (min 500) and keep the same max (131_072) so both paramsSchema and
paramsValidator are aligned.
- Around line 32-45: getNumberValidator currently returns
z.number(...).default(null), which is a type mismatch because z.number() cannot
accept null; update the schema to allow nulls by using nullable() before
default, i.e. change the returned schema from getNumberValidator so that the
z.number(...) chain includes .nullable() and then .default(null); ensure the
change is applied inside the getNumberValidator function where z.number,
validator, and .default(null) are referenced.
In `@packages/`@core/ui-kit/form-ui/src/form-render/form-label.vue:
- Line 29: The extra slot is currently rendered inside the label element (slot
name="extra" in form-label.vue) which causes interactive controls (the toggle
rendered by form-field.vue) to be children of a <label>; move the <slot
name="extra"> out of the label so the toggle/control is a sibling, not a
descendant. Update the template in form-label.vue to render the slot
after/adjacent to the label text container, preserve any classes/ARIA attributes
used by the label, and verify that form-field.vue's toggle handlers (the
interactive element referenced around lines 373-378) still function and keep
necessary event modifiers and focus/aria relationships so semantics and
accessibility are correct.
In
`@packages/`@core/ui-kit/shadcn-ui/src/components/collapsible/collapsible-params-item.vue:
- Around line 81-86: The range display condition incorrectly uses a truthy check
so a valid min of 0 is hidden; update the conditional on the span that
references data.option.min and data.option.max to explicitly check for presence
(e.g., ensure data.option.min and data.option.max are not null/undefined) rather
than relying on truthiness — change the condition that currently reads like
"data.option.min && data.option.max" to an explicit existence check (such as
using !== undefined / !== null or != null) for data.option.min and
data.option.max so ranges with min: 0 render correctly.
In `@playground/src/locales/langs/en-US/examples.json`:
- Line 29: Update the English label for the "collapsible" key in examples.json
to a more natural UI phrase; replace the current value "Collapsible FormItem
Content" with a cleaner option such as "Collapsible Form Item" (or "Collapsible
Form Field") so the UI text reads naturally.
In `@playground/src/views/examples/form/collapsible.vue`:
- Around line 105-113: The max_length bounds are inconsistent between
paramsSchema (which sets min: 500) and paramsValidator (which currently uses
getNumberValidator('max_length', [1, 131_072])), causing the UI and validator to
disagree; update paramsValidator to use getNumberValidator('max_length', [500,
131_072]) so the validator matches paramsSchema, ensuring both paramsSchema and
paramsValidator (and any calls to getNumberValidator for 'max_length') enforce
the same minimum.
- Around line 199-206: The string "设置表单值" is orphaned inside the RadioGroup and
won't render because RadioGroup renders from its options prop; remove the inline
text from the RadioGroup block (or, if intended as a separate label, move it
outside the <RadioGroup>), keeping the existing props (options="layouts",
v-model:value="layout", `@update`:value="onLayoutChange") intact so only the
options drive the RadioGroup content.
- Around line 22-35: The getNumberValidator function currently returns
z.number().default(null) which is invalid; update the schema returned by
getNumberValidator to allow null or undefined instead of forcing a numeric
default — for example, change the chain on the validator inside
getNumberValidator to use .nullable().default(null) if null should be permitted,
or .optional() (and remove the default) if the field may be absent; adjust the
limit handling to still apply to the underlying number schema
(validator.min/max) so references to getNumberValidator and the validator
variable remain correct.
---
Nitpick comments:
In `@packages/`@core/ui-kit/form-ui/src/types.ts:
- Around line 148-151: Add consistent `@default` tags for the collapse-related
props: annotate collapsible with "@default false" to match defaultCollapsed, and
add the same "@default false" note to the other collapse flag(s) around the same
block (e.g., the prop defined near lines 164-168) so the API docs consistently
show that those boolean collapse flags default to false; update the JSDoc
comments for the properties named "collapsible" and the other collapse-related
prop(s) referenced in this file accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d9c0307c-1e7d-4f07-87b9-7843fa9b42f3
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (17)
apps/web-naive/package.jsonapps/web-naive/src/views/demos/form/basic.vuepackages/@core/base/icons/src/lucide.tspackages/@core/ui-kit/form-ui/src/components/form-actions.vuepackages/@core/ui-kit/form-ui/src/form-render/form-field.vuepackages/@core/ui-kit/form-ui/src/form-render/form-label.vuepackages/@core/ui-kit/form-ui/src/types.tspackages/@core/ui-kit/shadcn-ui/src/components/collapsible/collapsible-params-item.vuepackages/@core/ui-kit/shadcn-ui/src/components/collapsible/collapsible-params.vuepackages/@core/ui-kit/shadcn-ui/src/components/collapsible/collapsible.vuepackages/@core/ui-kit/shadcn-ui/src/components/collapsible/index.tspackages/@core/ui-kit/shadcn-ui/src/components/collapsible/type.tspackages/@core/ui-kit/shadcn-ui/src/components/index.tsplayground/src/locales/langs/en-US/examples.jsonplayground/src/locales/langs/zh-CN/examples.jsonplayground/src/router/routes/modules/examples.tsplayground/src/views/examples/form/collapsible.vue
💤 Files with no reviewable changes (1)
- packages/@core/ui-kit/form-ui/src/components/form-actions.vue
Comment on lines
+32
to
+45
| function getNumberValidator(key: string, limit?: [number, number]) { | ||
| let validator = z.number({ | ||
| required_error: `${key} 值不能为空`, | ||
| invalid_type_error: `${key} 值只能为数字`, | ||
| }); | ||
|
|
||
| if (limit) { | ||
| validator = validator | ||
| .min(limit[0], { message: `${key} 值不在区间范围内` }) | ||
| .max(limit[1], { message: `${key} 值不在区间范围内` }); | ||
| } | ||
|
|
||
| return validator.default(null); | ||
| } |
Contributor
There was a problem hiding this comment.
Type mismatch: z.number().default(null) is invalid.
Same issue as the playground example—null is not a valid number for a z.number() schema. Use z.number().nullable().default(null) if null values should be allowed.
🔧 Proposed fix
function getNumberValidator(key: string, limit?: [number, number]) {
let validator = z.number({
required_error: `${key} 值不能为空`,
invalid_type_error: `${key} 值只能为数字`,
- });
+ }).nullable();
if (limit) {
validator = validator
.min(limit[0], { message: `${key} 值不在区间范围内` })
.max(limit[1], { message: `${key} 值不在区间范围内` });
}
return validator.default(null);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| function getNumberValidator(key: string, limit?: [number, number]) { | |
| let validator = z.number({ | |
| required_error: `${key} 值不能为空`, | |
| invalid_type_error: `${key} 值只能为数字`, | |
| }); | |
| if (limit) { | |
| validator = validator | |
| .min(limit[0], { message: `${key} 值不在区间范围内` }) | |
| .max(limit[1], { message: `${key} 值不在区间范围内` }); | |
| } | |
| return validator.default(null); | |
| } | |
| function getNumberValidator(key: string, limit?: [number, number]) { | |
| let validator = z.number({ | |
| required_error: `${key} 值不能为空`, | |
| invalid_type_error: `${key} 值只能为数字`, | |
| }).nullable(); | |
| if (limit) { | |
| validator = validator | |
| .min(limit[0], { message: `${key} 值不在区间范围内` }) | |
| .max(limit[1], { message: `${key} 值不在区间范围内` }); | |
| } | |
| return validator.default(null); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web-naive/src/views/demos/form/basic.vue` around lines 32 - 45,
getNumberValidator currently returns z.number(...).default(null), which is a
type mismatch because z.number() cannot accept null; update the schema to allow
nulls by using nullable() before default, i.e. change the returned schema from
getNumberValidator so that the z.number(...) chain includes .nullable() and then
.default(null); ensure the change is applied inside the getNumberValidator
function where z.number, validator, and .default(null) are referenced.
Contributor
Author
There was a problem hiding this comment.
设置zod-default提取默认值为null，不设置null,在reset后会绕过vee-validate的校验
Contributor
There was a problem hiding this comment.
@2yllll, understood! Using z.number().default(null) (without .nullable()) is intentional here — the null default is used as a sentinel so that after a form reset, vee-validate skips re-validation on that field. Adding .nullable() would make null a valid accepted value and break that bypass behavior. I'll withdraw my concern.
✏️ Learnings added
Learnt from: 2yllll
Repo: vbenjs/vue-vben-admin PR: 7816
File: apps/web-naive/src/views/demos/form/basic.vue:32-45
Timestamp: 2026-04-14T02:42:19.844Z
Learning: In `apps/web-naive/src/views/demos/form/basic.vue` and `playground/src/views/examples/form/collapsible.vue` (Vue 3, vee-validate + Zod), using `z.number().default(null)` (intentionally without `.nullable()`) is a deliberate pattern: the `null` default acts as a sentinel value so that after a form reset, vee-validate skips re-validation on that field. Adding `.nullable()` would make `null` a valid accepted value and break this bypass behavior. Do not flag this as a type mismatch.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
Comment on lines
+120
to
+128
| const paramsValidator = z.object({ | ||
| micro_batch_size: getNumberValidator('micro_batch_size', [8, 1024]), | ||
| learning_rate: getNumberValidator('learning_rate'), | ||
| eval_steps: getNumberValidator('eval_steps', [1, 2_147_483_647]), | ||
| num_train_epochs: getNumberValidator('num_train_epochs', [1, 200]), | ||
| max_length: getNumberValidator('max_length', [1, 131_072]), | ||
| warmup_ratio: getNumberValidator('warmup_ratio', [0, 1]), | ||
| save_steps: getNumberValidator('save_steps', [1, 2_147_483_647]), | ||
| }); |
Contributor
There was a problem hiding this comment.
Validator/schema min value inconsistency for max_length.
Same as the playground example: paramsSchema defines max_length with min: 500 (line 94), but paramsValidator uses [1, 131_072] (line 125). Align the limits.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web-naive/src/views/demos/form/basic.vue` around lines 120 - 128, The
paramsValidator's max_length range disagrees with paramsSchema's min
(paramsSchema sets min: 500 while paramsValidator uses [1, 131_072]); update the
max_length call in paramsValidator (the getNumberValidator('max_length', [...]
entry) to use the same minimum as paramsSchema (min 500) and keep the same max
(131_072) so both paramsSchema and paramsValidator are aligned.
| <VbenHelpTooltip v-if="help" trigger-class="size-3.5 ml-1"> | ||
| <VbenRenderContent :content="help" /> | ||
| </VbenHelpTooltip> | ||
| <slot name="extra"></slot> |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify whether interactive controls are being rendered into the `extra` slot.
rg -n -C4 '<slot name="extra"|<template `#extra`|<template v-slot:extra|@click|<button|role="button"|tabindex=' \
packages/@core/ui-kit/form-ui/src/form-render/form-label.vue \
packages/@core/ui-kit/form-ui/src/form-render/form-field.vueRepository: vbenjs/vue-vben-admin
Length of output: 2128
🏁 Script executed:
# Find and examine the FormLabel component definition
find . -type f -name "*.vue" -o -name "*.ts" -o -name "*.tsx" -o -name "*.js" | xargs grep -l "FormLabel" | grep -E "(components?|export)" | head -20Repository: vbenjs/vue-vben-admin
Length of output: 47
🏁 Script executed:
# Look for FormLabel export/definition
rg -l "export.*FormLabel|component.*FormLabel" --type vue --type ts --type tsx --type jsRepository: vbenjs/vue-vben-admin
Length of output: 92
🏁 Script executed:
# Look for FormLabel component definition - check form-label.vue directly
fd "form-label" --extension vue --extension ts --extension tsx --extension jsRepository: vbenjs/vue-vben-admin
Length of output: 125
🏁 Script executed:
# Read the form-label.vue file to understand what element FormLabel renders as
cat packages/@core/ui-kit/form-ui/src/form-render/form-label.vueRepository: vbenjs/vue-vben-admin
Length of output: 863
🏁 Script executed:
# Find the FormLabel component definition
fd "FormLabel" --extension vue --extension ts --extension tsx --extension jsRepository: vbenjs/vue-vben-admin
Length of output: 122
🏁 Script executed:
# Search for shadcn-ui package location
find . -type d -name "shadcn-ui" 2>/dev/nullRepository: vbenjs/vue-vben-admin
Length of output: 98
🏁 Script executed:
# Read the FormLabel component definition
cat packages/@core/ui-kit/shadcn-ui/src/ui/form/FormLabel.vueRepository: vbenjs/vue-vben-admin
Length of output: 477
🏁 Script executed:
# Check the Label component definition
cat packages/@core/ui-kit/shadcn-ui/src/ui/label/index.tsRepository: vbenjs/vue-vben-admin
Length of output: 112
🏁 Script executed:
# Read the Label component
cat packages/@core/ui-kit/shadcn-ui/src/ui/label/Label.vueRepository: vbenjs/vue-vben-admin
Length of output: 680
Move interactive controls outside the label element.
Line 29 places the extra slot inside a <label> element. The slot renders an interactive toggle button (in form-field.vue line 373-378) which violates semantic HTML—clickable elements should not be children of <label>. Even with @click.prevent, this creates accessibility issues and ambiguous semantics. Render the toggle control outside the label.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/`@core/ui-kit/form-ui/src/form-render/form-label.vue at line 29, The
extra slot is currently rendered inside the label element (slot name="extra" in
form-label.vue) which causes interactive controls (the toggle rendered by
form-field.vue) to be children of a <label>; move the <slot name="extra"> out of
the label so the toggle/control is a sibling, not a descendant. Update the
template in form-label.vue to render the slot after/adjacent to the label text
container, preserve any classes/ARIA attributes used by the label, and verify
that form-field.vue's toggle handlers (the interactive element referenced around
lines 373-378) still function and keep necessary event modifiers and focus/aria
relationships so semantics and accessibility are correct.
Comment on lines
+22
to
+35
| function getNumberValidator(key: string, limit?: [number, number]) { | ||
| let validator = z.number({ | ||
| required_error: `${key} 值不能为空`, | ||
| invalid_type_error: `${key} 值只能为数字`, | ||
| }); | ||
|
|
||
| if (limit) { | ||
| validator = validator | ||
| .min(limit[0], { message: `${key} 值不在区间范围内` }) | ||
| .max(limit[1], { message: `${key} 值不在区间范围内` }); | ||
| } | ||
|
|
||
| return validator.default(null); | ||
| } |
Contributor
There was a problem hiding this comment.
Type mismatch: z.number().default(null) is invalid.
Setting null as the default for a z.number() schema is a type error—null is not a valid number. This will cause validation issues when the form initializes with null values.
Consider using z.number().nullable() if null should be allowed, or z.number().optional() if the field can be undefined.
🔧 Proposed fix
function getNumberValidator(key: string, limit?: [number, number]) {
let validator = z.number({
required_error: `${key} 值不能为空`,
invalid_type_error: `${key} 值只能为数字`,
- });
+ }).nullable();
if (limit) {
validator = validator
.min(limit[0], { message: `${key} 值不在区间范围内` })
.max(limit[1], { message: `${key} 值不在区间范围内` });
}
return validator.default(null);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| function getNumberValidator(key: string, limit?: [number, number]) { | |
| let validator = z.number({ | |
| required_error: `${key} 值不能为空`, | |
| invalid_type_error: `${key} 值只能为数字`, | |
| }); | |
| if (limit) { | |
| validator = validator | |
| .min(limit[0], { message: `${key} 值不在区间范围内` }) | |
| .max(limit[1], { message: `${key} 值不在区间范围内` }); | |
| } | |
| return validator.default(null); | |
| } | |
| function getNumberValidator(key: string, limit?: [number, number]) { | |
| let validator = z.number({ | |
| required_error: `${key} 值不能为空`, | |
| invalid_type_error: `${key} 值只能为数字`, | |
| }).nullable(); | |
| if (limit) { | |
| validator = validator | |
| .min(limit[0], { message: `${key} 值不在区间范围内` }) | |
| .max(limit[1], { message: `${key} 值不在区间范围内` }); | |
| } | |
| return validator.default(null); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@playground/src/views/examples/form/collapsible.vue` around lines 22 - 35, The
getNumberValidator function currently returns z.number().default(null) which is
invalid; update the schema returned by getNumberValidator to allow null or
undefined instead of forcing a numeric default — for example, change the chain
on the validator inside getNumberValidator to use .nullable().default(null) if
null should be permitted, or .optional() (and remove the default) if the field
may be absent; adjust the limit handling to still apply to the underlying number
schema (validator.min/max) so references to getNumberValidator and the validator
variable remain correct.
Contributor
Author
There was a problem hiding this comment.
设置zod-default提取默认值为null,不设置null,在reset后会绕过vee-validate的校验
Contributor
There was a problem hiding this comment.
@2yllll, 感谢您的解释！明白了——将 null 作为 z.number().default(null) 的默认值（刻意不加 .nullable()）是一种哨兵值模式：在表单 reset 后，vee-validate 会提取该默认值并跳过对该字段的重新校验。如果加上 .nullable()，null 就会成为合法的可接受值，反而会破坏这个绕过校验的行为。我的建议有误，抱歉造成困扰。
该模式已记录在我的知识库中，后续不会再对此标记问题。
🧠 Learnings used
Learnt from: 2yllll
Repo: vbenjs/vue-vben-admin PR: 7816
File: apps/web-naive/src/views/demos/form/basic.vue:32-45
Timestamp: 2026-04-14T02:42:19.844Z
Learning: In `apps/web-naive/src/views/demos/form/basic.vue` and `playground/src/views/examples/form/collapsible.vue` (Vue 3, vee-validate + Zod), using `z.number().default(null)` (intentionally without `.nullable()`) is a deliberate pattern: the `null` default acts as a sentinel value so that after a form reset, vee-validate skips re-validation on that field. Adding `.nullable()` would make `null` a valid accepted value and break this bypass behavior. Do not flag this as a type mismatch.
Collaborator
|
很实用的功能，我跑了一下 还有一些警告需要处理 |
feat(form-ui): 在 dependencies 里提供访问extendApi的能力
2yllll
changed the title
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
packages/@core/ui-kit/shadcn-ui/src/components/collapsible/collapsible-params.vue (1)
146-156: Make the built-in labels configurable.
Name/Value/DescriptionandFold/Unfoldare hardcoded inside a shared UI-kit component. Exposing them via props or slots will keep this reusable in localized apps.Also applies to: 202-213
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/`@core/ui-kit/shadcn-ui/src/components/collapsible/collapsible-params.vue around lines 146 - 156, The template hardcodes labels ("Name", "Value", "Description", and the "Fold"/"Unfold" texts) which breaks localization/reuse; update the component (e.g., CollapsibleParams in collapsible-params.vue) to accept configurable labels by adding props (e.g., nameLabel, valueLabel, descriptionLabel, foldLabel, unfoldLabel with defaults) and replace the literal strings in the template with those props, or alternatively expose named slots (e.g., slot="header-name", "header-value", "header-description", "toggle-fold") so parent components can provide localized content; ensure default values remain the current English text and update any usages/tests to pass the new props or slots.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/`@core/ui-kit/form-ui/src/form-render/dependencies.ts:
- Around line 42-47: The current code eagerly casts injectFormProps() into a
non-null controller which can be undefined if formApi is absent or bound later;
change controller to be resolved lazily by reading extendApi inside a small
accessor function (e.g., getController() / resolveController) that checks
whether extendApi (and its formApi) exist and throws a clear, fast-fail error if
missing; update all dependency callbacks that currently use controller directly
to call this accessor so they always get a validated ExtendedFormApi (reference
symbols: injectFormProps, extendApi, formApi, controller, ExtendedFormApi).
In
`@packages/`@core/ui-kit/shadcn-ui/src/components/collapsible/collapsible-params.vue:
- Around line 103-115: The updateValues function currently ignores keys not yet
present on modelValue and mutates modelValue inside the loop, which drops
initial writes for newly added params; modify updateValues (the function named
updateValues and variable modelValue.value) to accept and merge incoming keys
regardless of whether they already exist on modelValue (remove or relax the
Object.hasOwn(modelValue.value, key) guard) and perform a single merge
assignment after the loop (e.g., build newValue for all owned keys from the
incoming values, then set modelValue.value = { ...modelValue.value, ...newValue
}) so newly added params are not lost on first write.
In `@playground/src/views/examples/form/collapsible.vue`:
- Around line 39-42: The max() branch uses the wrong bound in its error message:
inside the validator = validator.max(...) call replace the interpolated limit[0]
with the upper bound limit[1] so the message reads that the value cannot be
greater than the actual max (use limit[1] in the template), ensuring the max
validation error reports the correct bound.
---
Nitpick comments:
In
`@packages/`@core/ui-kit/shadcn-ui/src/components/collapsible/collapsible-params.vue:
- Around line 146-156: The template hardcodes labels ("Name", "Value",
"Description", and the "Fold"/"Unfold" texts) which breaks localization/reuse;
update the component (e.g., CollapsibleParams in collapsible-params.vue) to
accept configurable labels by adding props (e.g., nameLabel, valueLabel,
descriptionLabel, foldLabel, unfoldLabel with defaults) and replace the literal
strings in the template with those props, or alternatively expose named slots
(e.g., slot="header-name", "header-value", "header-description", "toggle-fold")
so parent components can provide localized content; ensure default values remain
the current English text and update any usages/tests to pass the new props or
slots.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6db038a5-ff76-462f-82d0-86213afd2dd2
📒 Files selected for processing (8)
apps/web-naive/src/views/demos/form/basic.vuepackages/@core/ui-kit/form-ui/src/form-render/dependencies.tspackages/@core/ui-kit/form-ui/src/index.tspackages/@core/ui-kit/form-ui/src/types.tspackages/@core/ui-kit/shadcn-ui/src/components/collapsible/collapsible-params-item.vuepackages/@core/ui-kit/shadcn-ui/src/components/collapsible/collapsible-params.vueplayground/src/locales/langs/en-US/examples.jsonplayground/src/views/examples/form/collapsible.vue
✅ Files skipped from review due to trivial changes (2)
- packages/@core/ui-kit/form-ui/src/index.ts
- playground/src/locales/langs/en-US/examples.json
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/web-naive/src/views/demos/form/basic.vue
Contributor
Author
应该处理完了 |
Collaborator
|
我playground测试，控制台有一些警告 |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/@core/ui-kit/shadcn-ui/src/components/collapsible/collapsible-params.vue (2)
94-106: Add braces for single-statement for-loops.While syntactically valid, omitting braces in for-loops reduces readability and increases risk of future maintenance errors.
♻️ Suggested change
function resetValues() { - if (visibleRefs.value) - for (const rowRef of visibleRefs.value) { + if (visibleRefs.value) { + for (const rowRef of visibleRefs.value) { rowRef?.reset(); } + } - if (collapsibleRefs.value) - for (const rowRef of collapsibleRefs.value) { + if (collapsibleRefs.value) { + for (const rowRef of collapsibleRefs.value) { rowRef?.reset(); } + } init(true); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/`@core/ui-kit/shadcn-ui/src/components/collapsible/collapsible-params.vue around lines 94 - 106, The for-loops in the resetValues function use single-statement bodies without braces; update the function to add braces around each for loop body to improve readability and safety: in resetValues, wrap the rowRef?.reset(); lines inside { } for both loops that iterate visibleRefs.value and collapsibleRefs.value, and keep the final init(true); call unchanged.
10-10: Consider using@vben/iconsfor consistency.The icon is imported directly from
lucide-vue-next, but according to context snippet 2 (packages/@core/base/icons/src/lucide.ts),ChevronsDownis already re-exported through the project's icon barrel. Using the internal re-export maintains consistency with the rest of the codebase.♻️ Suggested change
-import { ChevronsDown } from 'lucide-vue-next'; +import { ChevronsDown } from '@vben-core/icons';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/`@core/ui-kit/shadcn-ui/src/components/collapsible/collapsible-params.vue at line 10, Replace the direct lucide import with the project's icon barrel to keep icon usage consistent: in collapsible-params.vue swap the import of ChevronsDown from 'lucide-vue-next' to the internal re-export (e.g., import { ChevronsDown } from '@vben/icons' or the project's icon barrel that re-exports lucide icons), so the component uses the same ChevronsDown symbol as defined in the project's lucide.ts re-exports.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@packages/`@core/ui-kit/shadcn-ui/src/components/collapsible/collapsible-params.vue:
- Around line 251-260: The gradient color stops in the .trigger-bar.collapsed
rule are out of ascending order (the final stop uses 31.76% after a 33.43%
stop); update the stops so percentages increase monotonically — for example
change the last stop from "var(--color-background) 31.76%" to
"var(--color-background) 33.43%" (or reorder stops so background comes after the
33.43% border stop) in the .trigger-bar .collapsed block.
---
Nitpick comments:
In
`@packages/`@core/ui-kit/shadcn-ui/src/components/collapsible/collapsible-params.vue:
- Around line 94-106: The for-loops in the resetValues function use
single-statement bodies without braces; update the function to add braces around
each for loop body to improve readability and safety: in resetValues, wrap the
rowRef?.reset(); lines inside { } for both loops that iterate visibleRefs.value
and collapsibleRefs.value, and keep the final init(true); call unchanged.
- Line 10: Replace the direct lucide import with the project's icon barrel to
keep icon usage consistent: in collapsible-params.vue swap the import of
ChevronsDown from 'lucide-vue-next' to the internal re-export (e.g., import {
ChevronsDown } from '@vben/icons' or the project's icon barrel that re-exports
lucide icons), so the component uses the same ChevronsDown symbol as defined in
the project's lucide.ts re-exports.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e31b9434-0d75-4ac0-a92b-194b4c88fa16
📒 Files selected for processing (4)
packages/@core/ui-kit/shadcn-ui/src/components/collapsible/collapsible-params.vuepackages/@core/ui-kit/shadcn-ui/src/components/collapsible/type.tsplayground/src/adapter/component/index.tsplayground/src/views/examples/form/collapsible.vue
✅ Files skipped from review due to trivial changes (1)
- packages/@core/ui-kit/shadcn-ui/src/components/collapsible/type.ts
Comment on lines
+251
to
+260
| .trigger-bar { | ||
| &.collapsed { | ||
| background-image: linear-gradient( | ||
| hsl(var(--foreground) / 0%) 0%, | ||
| hsl(var(--foreground) / 12%) 31.76%, | ||
| var(--color-border) 31.76%, | ||
| var(--color-border) 33.43%, | ||
| var(--color-background) 31.76% | ||
| ); | ||
| } |
Contributor
There was a problem hiding this comment.
Gradient color stop ordering error.
The last color stop var(--color-background) 31.76% uses a percentage smaller than the preceding stop (33.43%). CSS linear gradients expect color stops in ascending order; this creates undefined rendering behavior across browsers.
🐛 Proposed fix
background-image: linear-gradient(
hsl(var(--foreground) / 0%) 0%,
hsl(var(--foreground) / 12%) 31.76%,
var(--color-border) 31.76%,
var(--color-border) 33.43%,
- var(--color-background) 31.76%
+ var(--color-background) 33.43%
);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| .trigger-bar { | |
| &.collapsed { | |
| background-image: linear-gradient( | |
| hsl(var(--foreground) / 0%) 0%, | |
| hsl(var(--foreground) / 12%) 31.76%, | |
| var(--color-border) 31.76%, | |
| var(--color-border) 33.43%, | |
| var(--color-background) 31.76% | |
| ); | |
| } | |
| .trigger-bar { | |
| &.collapsed { | |
| background-image: linear-gradient( | |
| hsl(var(--foreground) / 0%) 0%, | |
| hsl(var(--foreground) / 12%) 31.76%, | |
| var(--color-border) 31.76%, | |
| var(--color-border) 33.43%, | |
| var(--color-background) 33.43% | |
| ); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@packages/`@core/ui-kit/shadcn-ui/src/components/collapsible/collapsible-params.vue
around lines 251 - 260, The gradient color stops in the .trigger-bar.collapsed
rule are out of ascending order (the final stop uses 31.76% after a 33.43%
stop); update the stops so percentages increase monotonically — for example
change the last stop from "var(--color-background) 31.76%" to
"var(--color-background) 33.43%" (or reorder stops so background comes after the
33.43% border stop) in the .trigger-bar .collapsed block.
Contributor
Author
看到了，处理掉了 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
UI
Behavior
Documentation