-
Notifications
You must be signed in to change notification settings - Fork 15
feat(SearchField): WEB-2317 Search field empty suggestion and improvements #1473
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
base: master
Are you sure you want to change the base?
Conversation
|
Size stats
|
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 my last pr with import sorting XD
| boxShadow: | ||
| '0px 2px 1px -1px rgba(0,0,0,0.2), 0px 1px 1px 0px rgba(0,0,0,0.14), 0px 1px 3px 0px rgba(0,0,0,0.12)', | ||
| background: 'white', | ||
| boxSizing: 'border-box', |
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.
Easier to set a fixed width without having to take padding in account
| </div> | ||
| )} | ||
| renderSuggestionsContainer={(options) => { | ||
| if (!isEmptyCase && options.children === null) { |
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.
I had to add this because of the padding. An empty container was rendered when there were no options
|
Deploy preview for mistica-web ready! ✅ Preview Built with commit ce05d90. |
|
Accessibility report ℹ️ You can run this locally by executing |
|
Screenshot tests report ✔️ All passing |
| return allSuggestions.filter((item) => item.toLowerCase().includes(value.toLowerCase())); | ||
| }; | ||
|
|
||
| test('Show show suggestions on focus when shouldShowSuggestions is "focus"', async () => { |
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.
| test('Show show suggestions on focus when shouldShowSuggestions is "focus"', async () => { | |
| test('Should show suggestions on focus when shouldShowSuggestions is "focus"', async () => { |
| didClearFieldRef.current = false; | ||
| inputRef?.current?.blur(); | ||
| inputRef?.current?.focus(); |
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.
can't we execute this on clearInput callback instead of using an effect?
maybe something like this:
const clearInput = React.useCallback(() => {
flushSync(() => {
handleChangeValue('', '');
if (inputRef.current) {
onChange?.(createChangeEvent(inputRef.current, ''));
}
});
inputRef.current?.blur();
inputRef.current?.focus();
}, [handleChangeValue, onChange]);| {texts.searchFieldSuggestionsEmptyCase || | ||
| t(tokens.searchFieldSuggestionsEmptyCase)} |
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.
maybe this should be configurable at field level
| setIsEmptyCase(matchedSuggestions.length === 0); | ||
| setSuggestions(matchedSuggestions); |
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.
isn't this repeated state? you don't need an extra state for this. isEmptyCase state is derived from suggestions state:
const isEmptyCase = suggestions.length === 0
No description provided.