Add delete button to generic filters in autocomplete #3628
Conversation
Signed-off-by: Florent MILLOT <florent.millot_externe@rte-france.com>
| display: 'inline-flex', | ||
| }, | ||
| }} | ||
| onClick={(e) => { |
There was a problem hiding this comment.
should it be better in a separated method ?
There was a problem hiding this comment.
renderOption is getting big, it could even become a separated component. I expect it to grow even more soon. But it is not important yet.
There was a problem hiding this comment.
You're right, I will create a new component for render option
For the onclick method I'm not sure it's useful to extract it
| ); | ||
| break; | ||
| default: | ||
| content = <OverflowableText text={label} />; |
There was a problem hiding this comment.
should it be width="100%" like the case for Generic-Filter ?
There was a problem hiding this comment.
No need here, for the other one it's because it was alongside an icon in the same block
Mathieu-Deharbe
left a comment
Mathieu-Deharbe
left a comment
There was a problem hiding this comment.
Works fine. Only some optional remarks.
| display: 'inline-flex', | ||
| }, | ||
| }} | ||
| onClick={(e) => { |
There was a problem hiding this comment.
renderOption is getting big, it could even become a separated component. I expect it to grow even more soon. But it is not important yet.
src/components/results/common/global-filter/global-filter-autocomplete.tsx
Outdated
Show resolved
Hide resolved
| selectedGlobalFilters.find((f) => f.uuid === option.uuid) && | ||
| onChange(selectedGlobalFilters.filter((f) => f.uuid !== option.uuid)); |
There was a problem hiding this comment.
I find this syntax less clear than a good old if.
| selectedGlobalFilters.find((f) => f.uuid === option.uuid) && | |
| onChange(selectedGlobalFilters.filter((f) => f.uuid !== option.uuid)); | |
| if (selectedGlobalFilters.find((f) => f.uuid === option.uuid)) { | |
| onChange(selectedGlobalFilters.filter((f) => f.uuid !== option.uuid)); | |
| } |
But I am old. Do as you wish.
There was a problem hiding this comment.
okay I will change it
| dispatch(removeFromRecentGlobalFilters(option.uuid as UUID)); // generic filter so has uuid | ||
| }} | ||
| > | ||
| <DeleteIcon fontSize="small" /> |
There was a problem hiding this comment.
Are you sure that Stéphane want this icon to be small ? Usually when we use it it is always regularly sized.
| <DeleteIcon fontSize="small" /> | |
| <DeleteIcon /> |
(I know it would mean a slight change in the option css)
There was a problem hiding this comment.
I tried both and I think it looks better like that, no ?
Co-authored-by: Mathieu Deharbe <148252167+Mathieu-Deharbe@users.noreply.github.com>
- Extract `RenderOption` and `formatVoltageRange` into standalone components for better readability and reusability. - Remove the inline `renderOption` logic. Signed-off-by: Florent MILLOT <75525996+flomillot@users.noreply.github.com>
|
To merge before #3626