fix search when passing items #3070
Conversation
src/components/picker/types.tsx
Outdated
| filteredChildren?: ReactNode | undefined, | ||
| filteredItems?: Pick<PickerItemProps, 'label' | 'value' | 'disabled'>[] |
There was a problem hiding this comment.
The user should not have to do something like:
onSearchChange={(value, _bla, changed) => {
console.warn('value', value);
console.warn('changed', changed);
}}
in order to get the changes.
You can simply change the name and type of the second value (this should be changed in the API of course)
| const filteredItems = useMemo(() => { | ||
| if (showSearch && !_.isEmpty(searchValue)) { | ||
| return _.filter(items, item => { | ||
| const {label, value} = item; | ||
| const itemLabel = getItemLabelPresenter(label, value); | ||
| return !shouldFilterOut(searchValue, itemLabel); | ||
| }); | ||
| } | ||
|
|
||
| return items; | ||
| }, [showSearch, searchValue, items]); |
There was a problem hiding this comment.
Can you reduce this code duplication?
IMO there should be a single function that uses both children and items
src/components/picker/index.tsx
Outdated
| @@ -224,7 +225,7 @@ const Picker = React.forwardRef((props: PickerProps, ref) => { | |||
| useWheelPicker={useWheelPicker} | |||
| mode={mode} | |||
| useDialog={useDialog} | |||
| items={useItems ? items : undefined} | |||
| items={useItems ? filteredItems || items : undefined} | |||
There was a problem hiding this comment.
IMO this should be filteredItems and not filteredItems || items, just like below there's only {filteredChildren}
| const {label, value, getItemLabel: childGetItemLabel} = child.props; | ||
| const itemLabel = getItemLabelPresenter(label, value, childGetItemLabel || getItemLabel); | ||
| return _.filter(items, item => { | ||
| const {label, value, getItemLabel: itemGetItemLabel} = item.props || item; |
There was a problem hiding this comment.
getItemLabel seems clear (clearer than itemGetItemLabel IMO)
Maybe you can rename getItemLabel to something else like getItemLabelProps (line 9) which will make this simpler.
Not sure you even need to send it (getItemLabelFunction) as an input to the filterItems function
|
|
||
| return children; | ||
| }, [showSearch, searchValue, children]); | ||
| const filteredItems = useMemo(() => filterItems(children || items, getItemLabel), [showSearch, searchValue, items]); |
There was a problem hiding this comment.
You probably need children as a dependency here
| const filterItems = (items: any, getItemLabelFunction: any) => { | ||
| if (showSearch && !_.isEmpty(searchValue)) { | ||
| // @ts-expect-error need to fix children type | ||
| return _.filter(children, child => { | ||
| // @ts-expect-error need to fix children type to be based on PickerItemProps | ||
| const {label, value, getItemLabel: childGetItemLabel} = child.props; | ||
| const itemLabel = getItemLabelPresenter(label, value, childGetItemLabel || getItemLabel); | ||
| return _.filter(items, item => { | ||
| const {label, value, getItemLabel} = item.props || item; | ||
| const itemLabel = getItemLabelPresenter(label, value, getItemLabel || getItemLabelFunction); | ||
| return !shouldFilterOut(searchValue, itemLabel); | ||
| }); | ||
| } | ||
| return items; | ||
| }; | ||
|
|
||
| return children; | ||
| }, [showSearch, searchValue, children]); | ||
| const filteredItems = useMemo(() => filterItems(children || items, getItemLabel), [showSearch, searchValue, items, children]); |
There was a problem hiding this comment.
Lets move everything into the useMemo?
Description
Picker fix search when using
itemsinstead ofchildren.usePickerSearch hook returns filteredItems now.
Changelog
Picker fix search when using
itemsinstead ofchildren.Additional info
MADS-4193