fix(autocomplete): [autocomplete] fix form validate error when select… #2296
Conversation
… suggestion option
WalkthroughThis pull request includes updates to multiple package files, primarily focusing on version increments and enhancements to the autocomplete component's functionality. The version of Changes
Possibly related PRs
Suggested reviewers
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: 1
🧹 Outside diff range and nitpick comments (1)
packages/renderless/src/autocomplete/vue.ts (1)
127-127: LGTM! Consider adding a comment for clarity.The addition of the
dispatchparameter to theselectfunction is a good improvement. It aligns with the PR objective to fix form validation errors and is consistent with other methods in the file.Consider adding a brief comment explaining the purpose of the
dispatchparameter for better code maintainability. For example:- select: select({ emit, nextTick, props, state, dispatch }), + // dispatch is used for form validation after selection + select: select({ emit, nextTick, props, state, dispatch }),
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (5)
- packages/renderless/package.json (1 hunks)
- packages/renderless/src/autocomplete/index.ts (2 hunks)
- packages/renderless/src/autocomplete/vue.ts (1 hunks)
- packages/vue/src/autocomplete/package.json (1 hunks)
- packages/vue/src/autocomplete/src/pc.vue (1 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/renderless/package.json
🧰 Additional context used
🔇 Additional comments (8)
packages/vue/src/autocomplete/package.json (4)
3-3: LGTM: Addition of "type": "module"The addition of
"type": "module"is a positive change. It explicitly declares that this package uses ECMAScript modules, which is consistent with modern JavaScript practices and can lead to better tree-shaking and module resolution.
4-4: LGTM: Version update to 3.18.1The version update from 3.18.0 to 3.18.1 is appropriate for this PR. It follows semantic versioning principles, indicating a patch release for bug fixes, which aligns with the PR's objective to address a validation error in the autocomplete component.
6-6: LGTM: Addition of "license": "MIT"The addition of the license field with the MIT license is a positive change. It clearly communicates the terms under which the package is distributed, which is crucial for open-source projects. The MIT license is a widely accepted and permissive license, suitable for many projects.
22-25: LGTM: Addition of devDependencies for testingThe addition of devDependencies, specifically @opentiny-internal/vue-test-utils and vitest, is a positive change. This suggests that testing capabilities have been added or restored to the package, which promotes better code quality and maintainability.
To ensure that test files exist for this package, you can run the following command:
#!/bin/bash # Search for test files in the package directory fd -e spec.ts -e spec.js -e test.ts -e test.js . packages/vue/src/autocompleteIf no test files are found, consider adding appropriate test cases for the autocomplete component.
packages/renderless/src/autocomplete/vue.ts (1)
Line range hint
1-180: Great work on addressing the form validation issue. A few suggestions to complete the PR:
- Please mark the completed items in the PR checklist.
- Consider adding unit tests to verify the fix for the form validation error when selecting suggestion options.
- Update the component documentation to reflect this change in behavior, if applicable.
To help verify the changes and ensure comprehensive test coverage, you can run the following script:
Would you like assistance in writing unit tests or updating the documentation for this change?
packages/vue/src/autocomplete/src/pc.vue (1)
27-27: Clarify the rationale for changing blur handling and ensure comprehensive testing.The change from
@blurtov-clickoutsidefor handling the blur event is interesting and potentially improves the component's behavior, especially in complex UI scenarios. This might indeed address the form validation errors mentioned in the PR objectives.However, I have a few points to consider:
Could you please clarify the specific issues this change addresses? Understanding the reasoning will help validate if this is the best approach.
This non-standard way of handling blur events might not be immediately obvious to other developers. Consider adding a comment explaining why
v-clickoutsideis used instead of the standard@blur.Ensure thorough testing of various interaction scenarios, including:
- Clicking on suggestion items
- Clicking on other form elements
- Tabbing through the form
- Interacting with overlays or modals
Verify that this change doesn't introduce any unexpected side effects in the component's behavior.
To help verify the impact of this change, you can run the following script to check for other usages of
@blurin autocomplete-related components:This will help ensure consistency across related components and identify any potential areas that might need similar updates.
✅ Verification successful
Blur handling update verified successfully.
No other instances of
@blurfound in autocomplete-related components, ensuring the change is isolated and does not impact other parts of the feature.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for @blur usage in autocomplete-related files rg --type vue '@blur' packages/vue/src/autocompleteLength of output: 79
Script:
#!/bin/bash # Search for @blur usage in .vue files within autocomplete-related directories rg '@blur' --type-add 'vue:*.vue' --type vue packages/vue/src/autocompleteLength of output: 74
packages/renderless/src/autocomplete/index.ts (2)
168-169: Ensuring Consistent Form Validation on SelectionThe addition of
dispatchin theselectfunction and the subsequent dispatch ofFORM_EVENT.changewhenstate.validateEventistrueensures that form validation is triggered upon selecting an item. This change aligns with the validation logic present in other event handlers likehandleChange, promoting consistency across the component.Also applies to: 177-183
112-114:⚠️ Potential issuePotential Breaking Change: Removal of
eventparameter inhandleBlurThe
handleBlurfunction no longer accepts theeventparameter, and the emitted 'blur' event no longer includes the event object. This change might impact consumers who rely on theeventobject in their 'blur' event handlers. Please verify that removing theeventparameter does not introduce unintended side effects or break existing functionality.To identify any 'blur' event handlers that depend on the
eventobject, you can run the following script:This script searches for 'blur' event bindings in Vue components that include arguments, indicating usage of the
eventobject.✅ Verification successful
No Breaking Change Detected:
handleBlurRemoval is SafeAfter reviewing the usage of 'blur' event handlers, no consumers rely on the
eventparameter inhandleBlur. The handler passing$eventtosaveEditis unaffected, assaveEditdoes not accept or utilize any parameters.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Search for 'blur' event handlers that use the event parameter in .vue files. rg --type vue '@blur="[^"]+"' -o --no-filename | grep -E '@blur="[^"]*\([^)]*\)"'Length of output: 111
Script:
#!/bin/bash # Description: Search for 'blur' event handlers that use the event parameter in .vue files. rg -g "*.vue" '@blur="[^"]+"' -o --no-filename | grep -E '@blur="[^"]*\([^)]*\)"'Length of output: 263
Script:
#!/bin/bash # Description: Find implementations of the `saveEdit` function to determine if it relies on the `event` parameter. rg -g "*.vue" "function\s+saveEdit\s*\(" -A 5Length of output: 1084
| "@opentiny/vue-common": "workspace:~", | ||
| "@opentiny/vue-icon": "workspace:~", | ||
| "@opentiny/vue-input": "workspace:~", | ||
| "@opentiny/vue-renderless": "workspace:~", |
There was a problem hiding this comment.
💡 Codebase verification
@opentiny/vue-icon is still in use across multiple files
The @opentiny/vue-icon dependency has been removed from the package.json, but it is still being imported in numerous files within the packages/vue/src/autocomplete directory and other related packages. This inconsistency can lead to potential runtime errors or build issues.
Please either add @opentiny/vue-icon back to the dependencies or refactor the code to eliminate its usage if it's no longer needed.
🔗 Analysis chain
Please clarify the removal of @opentiny/vue-icon from dependencies
The dependencies have been reordered, which is fine. However, @opentiny/vue-icon has been removed from the dependencies list. Could you please clarify why this dependency was removed? Is it no longer used in the package, or is it being imported differently now?
To check if @opentiny/vue-icon is still used in the package, you can run the following command:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for imports of @opentiny/vue-icon in the package
rg --type typescript --type vue "from ['\"]\@opentiny/vue-icon" packages/vue/src/autocomplete
Length of output: 205
Script:
#!/bin/bash
# Corrected script to search for imports of @opentiny/vue-icon in the autocomplete package
rg --type typescript --type vue "from ['\"]@opentiny/vue-icon['\"]" packages/vue/src/autocomplete
Length of output: 138
Script:
#!/bin/bash
# Corrected script to search for imports of @opentiny/vue-icon in the autocomplete package
rg --type ts --type vue "from ['\"]@opentiny/vue-icon['\"]" packages/vue/src/autocomplete
Length of output: 123
Script:
#!/bin/bash
# Search for imports of @opentiny/vue-icon in the autocomplete package
fd -e ts -e vue -t f packages/vue/src/autocomplete | xargs rg "from ['\"]@opentiny/vue-icon['\"]"
Length of output: 73589
* fix(autocomplete): [autocomplete] fix form validate error when select suggestion option (#2296) * fix: update @opentiny/fluent-editor version and export defaultToolbar * chore(renderless): release v3.18.6 --------- Co-authored-by: gimmyhehe <[email protected]>
… suggestion option
PR
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Issue Number: N/A
What is the new behavior?
Does this PR introduce a breaking change?
Other information
由于@blur是当鼠标点击时就触发，而选项面板中，是点击事件触发选中，点击事件是需要松开鼠标才会触发。
所以导致失焦先校验 -> 弹出校验框。 选中对应选项，弹窗消失。
修改方案：
将失焦事件触发改为click-outside，使得松开鼠标后才触发校验事件。