-
-
Notifications
You must be signed in to change notification settings - Fork 341
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Display an error when duplicate values entered into Autocompelte #2945
base: master
Are you sure you want to change the base?
Conversation
…4/mui-toolpad into 2923_limit_same_value
…4/mui-toolpad into 2923_limit_same_value
@apedroferreira help me review this ? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good job validating on the 2 different inputs! I left a few suggestions but this is a good start.
There are also a few more cases where it seems that you need to clear the error message or it does not reset, can you please cover all of these? Some examples, not sure if there's any more cases than these:
- User closes the modal, by either pressing "Close" or clicking outside
- User clicks "Delete all" to delete all options
- User presses the back button from an option
packages/toolpad-app/src/toolpad/propertyControls/SelectOptions.tsx
Outdated
Show resolved
Hide resolved
packages/toolpad-app/src/toolpad/propertyControls/SelectOptions.tsx
Outdated
Show resolved
Hide resolved
packages/toolpad-app/src/toolpad/propertyControls/SelectOptions.tsx
Outdated
Show resolved
Hide resolved
packages/toolpad-app/src/toolpad/propertyControls/SelectOptions.tsx
Outdated
Show resolved
Hide resolved
@apedroferreira help to review the rest |
packages/toolpad-app/src/toolpad/propertyControls/SelectOptions.tsx
Outdated
Show resolved
Hide resolved
packages/toolpad-app/src/toolpad/propertyControls/SelectOptions.tsx
Outdated
Show resolved
Hide resolved
packages/toolpad-app/src/toolpad/propertyControls/SelectOptions.tsx
Outdated
Show resolved
Hide resolved
packages/toolpad-app/src/toolpad/propertyControls/SelectOptions.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some notes around the structure of the changes
@apedroferreira @bharatkashyap help me to review this PR |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
switchErrorState((item) =>
typeof item !== 'string' ? item.value === newOptionValue : item === value,
);
There is some replication of the above code.
The idea with this feature is that you accept an input value, compare it with the array of existing options, and flip the state of the error in case a duplicate is found. You can refactor this into a single, memoized function that does all of this, with inputText
being the only required parameter.
In my opinion, separate validateOptionValue
and switchErrorState
functions are not needed.
…_limit_same_value
all done |
Fixes #2923