fix: disabledOnChangeListener not work in form
#5146
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/form-render/form-field.vueOops! Something went wrong! :( ESLint: 9.16.0 Error [ERR_MODULE_NOT_FOUND]: Cannot find module '/node_modules/@vben/eslint-config/dist/index.mjs' imported from /eslint.config.mjs WalkthroughThe pull request focuses on modifying 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 (
|
![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 (2)
packages/@core/ui-kit/form-ui/src/form-render/form-field.vue (2)
234-237: LGTM! Consider adding a comment for clarity.The changes correctly implement the disabling of input and change event handlers when no
bindEventFieldis present. This fixes the core issue wheredisabledOnChangeListenerwasn't working.Consider adding a comment to explain the fallback behavior:
+ // Fallback case when no bindEventField is present return { ...(disabledOnInputListener ? { onInput: undefined } : {}), ...(disabledOnChangeListener ? { onChange: undefined } : {}), };
Line range hint
239-252: LGTM! Consider adding type safety for event handlers.The changes correctly handle the precedence of event handlers from
computedPropsusingReflect.has. This ensures proper overriding of event handlers when specified in component props.Consider adding type safety for the event handlers:
+type EventHandler = (e: any) => void; + function createComponentProps(slotProps: Record<string, any>) { const bindEvents = fieldBindEvent(slotProps); const binds = { ...slotProps.componentField, ...computedProps.value, ...bindEvents, ...(Reflect.has(computedProps.value, 'onChange') - ? { onChange: computedProps.value.onChange } + ? { onChange: computedProps.value.onChange as EventHandler } : {}), ...(Reflect.has(computedProps.value, 'onInput') - ? { onInput: computedProps.value.onInput } + ? { onInput: computedProps.value.onInput as EventHandler } : {}), }; return binds; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/@core/ui-kit/form-ui/src/form-render/form-field.vue(1 hunks)
🔇 Additional comments (1)
packages/@core/ui-kit/form-ui/src/form-render/form-field.vue (1)
Line range hint 234-252: Verify the fix addresses the reported issue.
The changes look good and directly address the reported issue in #5144. Let's verify that the fix works as expected across different form field types.
✅ Verification successful
The search results show that the disabledOnChangeListener and disabledOnInputListener functionality is consistently implemented across the form-ui module. Let me verify one more aspect to ensure the fix is complete.
The fix for disabledOnChangeListener is properly implemented and verified
The implementation is correct and consistent across the codebase:
- The feature is properly typed in
types.tsas part ofFormCommonConfig - Default configuration in
config.tssets both listeners totrueby default - The fix in
form-field.vuecorrectly implements the disabling logic by setting handlers toundefined - The implementation maintains proper precedence with
computedPropsvalues
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for other form field components that might be affected by this change
# Look for form field components that use onChange or onInput events
ast-grep --pattern 'onChange: $_'
ast-grep --pattern 'onInput: $_'
# Look for other instances where these listeners might be disabled
rg "disabledOnChangeListener|disabledOnInputListener"
Length of output: 1782
Script:
#!/bin/bash
# Check the configuration and type definitions to ensure the fix aligns with the intended behavior
# Check the default configuration
cat packages/@core/ui-kit/form-ui/src/config.ts
# Check the type definitions
cat packages/@core/ui-kit/form-ui/src/types.ts
Length of output: 10445
Description
修复
disabledOnChangeListener在form中不生效的问题fix #5144
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
Bug Fixes
Refactor