-
Notifications
You must be signed in to change notification settings - Fork 4
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
fix(element-search): remove freesolo and improve typing #456
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Joris Mancini <[email protected]>
Signed-off-by: Joris Mancini <[email protected]>
…s-ui into jorism/remove-free-solo
Signed-off-by: Joris Mancini <[email protected]>
@@ -54,8 +53,8 @@ const ElementSearchDialog = (props: ElementSearchDialogProps) => { | |||
|
|||
const displayedValue = useMemo(() => { | |||
return searchTermDisabled || searchTermDisableReason | |||
? searchTermDisableReason | |||
: searchTerm; | |||
? searchTermDisableReason ?? 'search disabled' |
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.
manage translation for this default value
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.
done
// we don't want to change "searchTerm" when clicking a result | ||
if (!searchTermDisabled && reason !== 'reset') { | ||
onInputChange={(_event, value) => { | ||
if (!searchTermDisabled) { |
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.
must restore reason
to avoid calling onSearchTermChange
when selecting an option. Otherwise it will fetch serach one more time
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.
done
5f5748a
to
ef23da5
Compare
? searchTermDisableReason ?? | ||
intl.formatMessage({ | ||
id: 'element_search/searchDisabled', | ||
}) | ||
: searchTerm ?? ''; | ||
}, [searchTermDisabled, searchTermDisableReason, intl, searchTerm]); |
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.
This is usually not a good pratice to add complexity to ternary.
you probably need to change to if/else statement
Quality Gate passedIssues Measures |
freesolo is not the wished behavior for the element search bar so we can remove it, it simplifies component typing.