🌱 Refactor FilterSelectOptionProps #1802
Conversation
rszwajko
force-pushed
the
optionKey
branch
2 times, most recently
from
d3fe67a to
953eb7f
Compare
There was a problem hiding this comment.
Reading through the code and testing the general function of things and everything looks ok. However, the Tags dropdown is not working correctly anymore. It doesn't want to close when text is entered in the search box:
screencast-localhost_9000-2024.04.04-17_57_26.mp4
The type changes and the subsequent refactoring looks good.
The look of the multiselect looks ok, and it seems to behave properly if you don't type in search text. Only a problem with search text entered.
Key points: 1. do not extend PF SelectOptionProps interface - this allows to add custom props to the interface and provides better control what prop are forwarded to SelectOption 2. remove key prop as it's a special prop used internally by React 3. use value prop according to PF documentation - effectively replace previously used key prop 4. add label prop - to be used as human friendly representation of the value 5. add groupLabel prop 6. add chipLabel prop - for cases where it differs from the label prop (main use case are tag items) Signed-off-by: Radoslaw Szwajkowski <rszwajko@redhat.com>
|
@sjd78 |
The menu now closes as expected. 😀 |
There was a problem hiding this comment.
LGTM
Not new with this PR, but perhaps in a followup the behavior where the search text stays present when the dropdown closes is awkward:
The only way to remove the text without clearing the selected options is to backspace or highlight/delete it. The X button within the field clears the text AND the current selections. IDK if it is a problem, just is ambiguous and awkward.
|
This PR looks to be a pre-req for #1815 which is a fix for 0.3.next. Therefore this PR should target 0.3.next and be cherry-picked. @ibolton336, does that make sense to you? |
sjd78
added
the
cherry-pick/release-0.3
Key points: 1. do not extend PF SelectOptionProps interface - this allows to add custom props to the interface and provides better control what prop are forwarded to SelectOption 2. remove key prop as it's a special prop used internally by React 3. use value prop according to PF documentation - effectively replace previously used key prop 4. add label prop - to be used as human friendly representation of the value 5. add groupLabel prop 6. add chipLabel prop - for cases where it differs from the label prop (main use case are tag items) Signed-off-by: Radoslaw Szwajkowski <rszwajko@redhat.com> Signed-off-by: Ian Bolton <ibolton@redhat.com>
Key points: 1. do not extend PF SelectOptionProps interface - this allows to add custom props to the interface and provides better control what prop are forwarded to SelectOption 2. remove key prop as it's a special prop used internally by React 3. use value prop according to PF documentation - effectively replace previously used key prop 4. add label prop - to be used as human friendly representation of the value 5. add groupLabel prop 6. add chipLabel prop - for cases where it differs from the label prop (main use case are tag items) Signed-off-by: Ian Bolton <ibolton@redhat.com> Co-authored-by: Radoslaw Szwajkowski <rszwajko@redhat.com>
Key points:
custom props to the interface and provides better control what prop
are forwarded to SelectOption
previously used key prop
value
(main use case are tag items)