fix(autocomplete): return value instead of label in form submission#3375
fix(autocomplete): return value instead of label in form submission#3375wingkwong wants to merge 33 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
🦋 Changeset detectedLatest commit: 09bcf2b The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. 🗂️ Base branches to auto review (5)
Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThe changes in this pull request modify the behavior of the autocomplete component to return the actual value of the selected option instead of its label during form submission. This adjustment addresses issues #3353 and #3343. Additionally, the Changes
Assessment against linked issues
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
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (2)
packages/components/autocomplete/src/use-autocomplete.ts (2)
118-125: LGTM: NewInputDatainterface andinputDataWeakMap addedThe addition of the
InputDatainterface andinputDataWeakMap is a good approach for managing input-related data. The use of a WeakMap is efficient and allows for garbage collection of unusedComboBoxStateinstances.However, it would be helpful to add a comment explaining the purpose of the
inputDataWeakMap for better code maintainability.Consider adding a comment above the
inputDatadeclaration, like this:// WeakMap to associate ComboBoxState instances with their corresponding InputData // This allows for efficient storage and retrieval of input-related data export const inputData = new WeakMap<ComboBoxState<any>, InputData>();
Line range hint
181-224: LGTM with suggestion: Implementation ofonSelectionChangeThe implementation of
onSelectionChangein theuseComboBoxStateconfiguration is good. It correctly handles selection changes and updates the form value, addressing the main objective of returning the actual value instead of the label.However, there's a potential issue when accessing
hiddenInputRef.current.Add a null check before accessing
hiddenInputRef.currentto prevent potential runtime errors:onSelectionChange: (keys) => { onSelectionChange?.(keys); if (onChange && typeof onChange === "function") { onChange({ target: { name: hiddenInputRef?.current?.name, value: keys, }, } as React.ChangeEvent<HTMLInputElement>); } },Consider refactoring to:
onSelectionChange: (keys) => { onSelectionChange?.(keys); if (onChange && typeof onChange === "function") { const name = hiddenInputRef?.current?.name; onChange({ target: { name, value: keys, }, } as React.ChangeEvent<HTMLInputElement>); } },
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
- packages/components/autocomplete/src/use-autocomplete.ts (9 hunks)
🧰 Additional context used
🪛 Biome
packages/components/autocomplete/src/use-autocomplete.ts
[error] 464-464: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
🔇 Additional comments (3)
packages/components/autocomplete/src/use-autocomplete.ts (3)
113-117: LGTM: NewonSelectionChangeprop addedThe addition of the
onSelectionChangeprop is a good improvement. It allows users to handle selection changes, which is crucial for the new functionality of returning the actual value instead of the label.
128-131: LGTM: UpdatedUseAutocompletePropstypeThe update to the
UseAutocompleteProps<T>type, excludingonSelectionChangefrom theOmit<InputProps, ...>part, is correct. This change ensures that the newly addedonSelectionChangeprop is properly typed and doesn't conflict with any similarly named prop fromInputProps.
Line range hint
553-609: LGTM: Addition ofgetHiddenInputPropsand related changesThe addition of the
getHiddenInputPropsfunction and the related changes are well-implemented:
- The
getHiddenInputPropsfunction is correctly memoized to optimize performance.- The
inputDataWeakMap is properly populated with the necessary input-related data.- The addition of
getHiddenInputPropsto the hook's return value allows consumers to easily apply the hidden input properties.These changes effectively support the new functionality of using a hidden input to store the actual value, which is a key objective of this PR.
|
is there any progress on this issue? please merge so i can use the autocomplete component |
|
Thanks. really appreciate it 🙌👌 |
|
@wingkwong @jrgarciadev For example, if the key is a sent as the value, the Custom value feature will send the key instead of the user's input. This would not be useful. |
|
let's have an internal discussion. |
|
As mentioned in this comment, autocomplete is not a select, so the label is submitted. |
4 participants
📝 Description
Similar to Select, use a hidden input to hold the actual value.
⛳️ Current behavior (updates)
The value in Input is actually label. In form submission, users need the value instead of the label.
🚀 New behavior
💣 Is this a breaking change (Yes/No):
Yes. Existing form values will be different after this change.
📝 Additional Information
Summary by CodeRabbit
New Features
WithFormTemplatefor submitting selected values, displaying alerts upon submission.Bug Fixes
Tests
Autocompletecomponent, ensuring accurate form value retrieval and validation.Documentation