fix: pinInput value synchronous update #5142
Conversation
mynetfan
requested review from
anncwb,
vince292007 and
a team
as code owners
|
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
apps/web-antd/src/views/_core/authentication/code-login.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 introduces a consistent modification across multiple components and localization files to standardize the verification code input length to 6 characters. This change involves updating the Changes
Assessment against linked issues
Possibly related PRs
Suggested Labels
Suggested Reviewers
Poem
Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command 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 (3)
playground/src/views/_core/authentication/code-login.vue (1)
13-13: Consider moving CODE_LENGTH to a shared configuration.While the constant definition is good practice, consider moving it to a shared configuration file since it's used across multiple components and affects the authentication system globally.
apps/web-naive/src/views/_core/authentication/code-login.vue (1)
13-13: Consider reducing code duplication across implementations.The identical changes across different UI framework implementations suggest an opportunity to reduce duplication. Consider:
- Moving the CODE_LENGTH constant to a shared authentication configuration
- Creating a shared form schema factory for the code login form
Example approach:
// @vben/common/config/auth.ts export const AUTH_CONFIG = { CODE_LENGTH: 6, } as const; // @vben/common/schemas/auth.ts export function createCodeLoginFormSchema(): VbenFormSchema[] { return [ // ... common form schema logic ]; }Also applies to: 34-34, 46-48
packages/@core/ui-kit/shadcn-ui/src/components/pin-input/input.vue (1)
50-52: Consider using computed property for better reactivity.While the current implementation works, consider refactoring to use a computed property with getter/setter for better reactivity and cleaner code:
-const inputValue = ref<string[]>([]); - -watch( - () => modelValue.value, - () => { - inputValue.value = modelValue.value?.split('') ?? []; - }, -); - -watch(inputValue, (val) => { - modelValue.value = val.join(''); -}); +const inputValue = computed({ + get: () => modelValue.value?.split('') ?? [], + set: (val: string[]) => { + modelValue.value = val.join(''); + } +});This approach:
- Eliminates the need for multiple watchers
- Provides more predictable reactivity
- Reduces the chance of sync issues
- Makes the code more maintainable
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
apps/web-antd/src/views/_core/authentication/code-login.vue(3 hunks)apps/web-ele/src/views/_core/authentication/code-login.vue(3 hunks)apps/web-naive/src/views/_core/authentication/code-login.vue(3 hunks)packages/@core/ui-kit/shadcn-ui/src/components/pin-input/input.vue(1 hunks)packages/locales/src/langs/en-US/authentication.json(1 hunks)packages/locales/src/langs/zh-CN/authentication.json(1 hunks)playground/src/views/_core/authentication/code-login.vue(3 hunks)
🔇 Additional comments (8)
packages/locales/src/langs/zh-CN/authentication.json (1)
41-41: LGTM: Improved localization message with dynamic length.
The message now properly supports dynamic verification code length, making it more specific and user-friendly.
playground/src/views/_core/authentication/code-login.vue (1)
34-34: LGTM: Improved PIN input validation.
The changes properly synchronize the PIN input length between the UI component and validation rules, which should fix the issue with form validation not triggering after partial deletion.
Let's verify the consistency of CODE_LENGTH usage across the codebase:
Also applies to: 46-48
✅ Verification successful
CODE_LENGTH constant is consistently defined and used across all UI variants
The verification shows that CODE_LENGTH is consistently defined as 6 across all UI framework variants (antd, naive, ele, and playground), and there are no hardcoded "6" values that should be using the constant instead. The changes properly maintain consistency in PIN input validation across the codebase.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any hardcoded values that should use CODE_LENGTH
rg "(?:length|Length).*[\"']6[\"']" --type ts --type vue
# Search for all CODE_LENGTH usage to ensure consistency
rg "CODE_LENGTH\s*=\s*\d+" --type ts --type vue
Length of output: 168
Script:
#!/bin/bash
# Search for any hardcoded values that should use CODE_LENGTH, without file type restrictions
rg "(?:length|Length).*[\"']6[\"']"
# Search for all CODE_LENGTH usage to ensure consistency
rg "CODE_LENGTH\s*=\s*\d+"
# Find all files that might contain CODE_LENGTH constant definition
rg "CODE_LENGTH" -l
Length of output: 656
apps/web-antd/src/views/_core/authentication/code-login.vue (2)
13-13: LGTM: Good practice using a constant for code length
Using a constant for the PIN length improves maintainability and ensures consistency across the component.
34-34: Verify if this change fixes the synchronous update issue
While the code changes look correct, I don't see direct evidence that this addresses the reported issue of "input value not updating in real-time" and "inability to trigger form validation after partially deleting values". Could you please verify if additional changes are needed in the VbenPinInput component itself?
Also applies to: 46-48
✅ Verification successful
Let's search for the component implementation using different patterns to locate the file first.
The component implementation properly handles real-time updates and validation
Looking at the implementation in input.vue, the component correctly handles synchronous updates through:
- Two-way binding with
defineModeland proper value syncing - Watch handlers that update both the internal
inputValuearray and externalmodelValue - Proper event handling for completion and validation
The code changes in code-login.vue are sufficient as the underlying component already has the necessary reactive behavior built-in. The reported issues about "input value not updating in real-time" and "inability to trigger form validation" would be resolved by using this implementation.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for VbenPinInput component implementation to check value update logic
ast-grep --pattern 'const VbenPinInput = defineComponent({
$$$
setup($props, $context) {
$$$
}
$$$
})'
Length of output: 116
Script:
#!/bin/bash
# Let's try different approaches to find the component implementation
# Search for files containing VbenPinInput
fd -t f . | xargs rg "VbenPinInput" -l
# Search for pin input related files
fd -t f "pin.*\.(vue|ts|js|tsx|jsx)$"
# Search for potential component definition patterns
rg -l "defineComponent.*Pin"
Length of output: 1329
Script:
#!/bin/bash
# Let's examine the pin input implementation in shadcn-ui since it appears to be the main component
cat packages/@core/ui-kit/shadcn-ui/src/ui/pin-input/PinInput.vue
cat packages/@core/ui-kit/shadcn-ui/src/ui/pin-input/PinInputInput.vue
# Also check how it's being used in code-login.vue
cat apps/web-antd/src/views/_core/authentication/code-login.vue
Length of output: 3481
Script:
#!/bin/bash
# Let's check the radix-vue implementation since the component is based on it
cat packages/@core/ui-kit/shadcn-ui/src/components/pin-input/index.ts
# Also check if there are any related components or types
rg -A 5 "VbenPinInput" packages/effects/common-ui/src/components/index.ts
rg -A 5 "VbenPinInput" packages/@core/ui-kit/form-ui/src/types.ts
Length of output: 575
Script:
#!/bin/bash
# Let's check the actual pin input implementation in shadcn-ui
cat packages/@core/ui-kit/shadcn-ui/src/components/pin-input/input.vue
cat packages/@core/ui-kit/shadcn-ui/src/components/pin-input/types.ts
Length of output: 3043
apps/web-ele/src/views/_core/authentication/code-login.vue (1)
13-13: LGTM: Consistent implementation across web variants
The changes mirror those in the web-antd version, maintaining consistency across different web application variants.
Also applies to: 34-34, 46-48
packages/locales/src/langs/en-US/authentication.json (1)
41-41: LGTM: Clear error message with dynamic length
The updated message clearly communicates the exact length requirement. However, we should verify that this change is consistently applied across all language files.
✅ Verification successful
Both language files are properly updated with dynamic length parameter
The verification shows that both English (en-US) and Chinese (zh-CN) localization files have been updated to use the {0} placeholder for the dynamic code length. The messages are consistent in their approach and grammatically correct in their respective languages.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if all language files have been updated with the new message format
fd --extension json --exec grep -l "codeTip" {} \; packages/locales/src/langs/
Length of output: 495
Script:
#!/bin/bash
# Check if all language files have been updated with the new message format
fd -e json . packages/locales/src/langs/ --exec grep -l "codeTip" {} \;
Length of output: 180
Script:
#!/bin/bash
# Check the actual content of codeTip in both language files
echo "=== English (en-US) ==="
grep -A 1 '"codeTip"' packages/locales/src/langs/en-US/authentication.json
echo -e "\n=== Chinese (zh-CN) ==="
grep -A 1 '"codeTip"' packages/locales/src/langs/zh-CN/authentication.json
Length of output: 396
packages/@core/ui-kit/shadcn-ui/src/components/pin-input/input.vue (2)
50-52: LGTM! This change fixes the synchronization issue.
The new watcher ensures that changes to individual PIN digits are immediately reflected in the modelValue, addressing the real-time update issue mentioned in PR #5104.
Line range hint 1-140: Verify form validation integration.
Since this component is used in form validation contexts (as mentioned in the PR objectives), we should verify that the real-time updates properly trigger form validation in parent components.
Description
修复VbenPinInput的值未能实时更新的问题，该问题会导致在完整输入Pin之后，删除部分值也不能重新触发表单校验。
fix #5104
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
Documentation