Migrate deprecated Select component to PF5, remove unsupported/unused components #1435
Conversation
components Reference: https://issues.redhat.com/browse/MTV-1051 - Migrate the deprecated Select component to PF5 - see https://v5-archive.patternfly.org/components/menus/select/react-deprecated - Remove unused common component: AutocompleteFilter - Remove components that use the inline filter which is no longer supported by PF5: SearchableEnumFilter, SearchableGroupedEnumFilter - Fix bugs regarding the Select component, e.g. changing the Select menu to be scrollable when needed. Signed-off-by: Sharon Gratch <[email protected]>
sgratch
added
the
enhancement
|
|
Codecov ReportAttention: Patch coverage is
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## main #1435 +/- ##
==========================================
- Coverage 36.81% 36.40% -0.41%
==========================================
Files 158 156 -2
Lines 2548 2516 -32
Branches 599 593 -6
==========================================
- Hits 938 916 -22
+ Misses 1428 1418 -10
Partials 182 182 ☔ View full report in Codecov by Sentry. |
| @@ -82,4 +82,8 @@ | |||
|
|
|||
| .forklift--create-plan--wizard-appearance-order { | |||
| z-index: 1; | |||
| } | |||
|
|
|||
| .odf-backing-storage__selection--width{ | |||
There was a problem hiding this comment.
is it used anywhere? tried to look in the PR but couldn't find, maybe I'm missing something
| ref={toggleRef} | ||
| onClick={onToggleClick} | ||
| isExpanded={isOpen} | ||
| style={ |
| }; | ||
|
|
||
| const onToggle: (isExpanded: boolean, event: ToggleEventType) => void = (isExpanded) => { | ||
| setExpanded(isExpanded); | ||
| const toggle = (toggleRef: React.Ref<MenuToggleElement>) => ( |
There was a problem hiding this comment.
please try to import all the things from React as named imported and not React.
| toggle={toggle} | ||
| shouldFocusToggleOnSelect | ||
| shouldFocusFirstItemOnOpen={false} | ||
| isScrollable={true} |
| _event: React.MouseEvent<Element, MouseEvent> | undefined, | ||
| value: string | number | undefined, | ||
| ) => { | ||
| const label = typeof value === 'string' ? value : value?.toString(); |
There was a problem hiding this comment.
u think this is neccessry? for example IIRC also string has toString() method so no need to check for === string - please check.
| value: string | number | undefined, | ||
| ) => { | ||
| const label = typeof value === 'string' ? value : value?.toString(); | ||
| const id = label2enum[label] ? label2enum[label].id : label; |
There was a problem hiding this comment.
const id = label2enum?.[label] ? label2enum[label]?.id : label;
| const onToggle: (isExpanded: boolean, event: ToggleEventType) => void = (isExpanded) => { | ||
| setExpanded(isExpanded); | ||
| const onToggleClick = () => { | ||
| setIsOpen(!isOpen); |
There was a problem hiding this comment.
setIsOpen(open => !open);
|
I didn't repeat comments please treat comments as it was for all the duplicated places in the PR |
Reference: https://issues.redhat.com/browse/MTV-1051
AutocompleteFilterSearchableEnumFilter,SearchableGroupedEnumFilter