WordPress / gutenberg

The Block Editor project for WordPress and beyond. Plugin is available from the official repository.
https://wordpress.org/gutenberg/
Other
10.5k stars 4.19k forks source link

ComboboxControl component: Account for the `value` when displaying the available options #64056

Open leogermani opened 3 months ago

leogermani commented 3 months ago

Description

When using the ComboboxControl component, the available options are filtered by what's in the label of each option, but the value is discarded. This produces some unexpected behaviors where options that should be visible disappear.

For example. let's say you use this component to search/select entities that have a name and a slug, and the slug is used as the value - and that you allow the search to be performed by both fields.

If the slug does not match the name, and you search by that slug, the option will be hidden.

What I expected is that the component would not hide the options that the value matches what the user has typed.

Step-by-step reproduction instructions

Set up a ComboboxControl component with an option that has a different string in label and value. Example:

options = [
    {
        label: 'A label value',
        value: 'slug',
    },
];

Type in slug in the text field and see that the option is not displayed. (Actually sometimes you can see it blink)

Screenshots, screen recording, code snippet

No response

Environment info

No response

Please confirm that you have searched existing issues in the repo.

Please confirm that you have tested with all plugins deactivated except Gutenberg.

imrraaj commented 2 months ago

I can reproduce the issue. It would be better to allow the user to search based on the value field. I have a local copy of the changes that will enable this. It basically checks option items' label and value fields.

https://github.com/WordPress/gutenberg/blob/924dfb3870eb1eab3f4dbfc0746c181528ea68c6/packages/components/src/combobox-control/index.tsx#L153-L167

const matchingSuggestions = useMemo( () => {
        // use set to filter out common options
    const startsWithMatch = new Set< ComboboxControlOption >();
    const containsMatch = new Set< ComboboxControlOption >();
    const match = normalizeTextString( inputValue );
    options.forEach( ( option ) => {

        let searchFrom = [ option.label, option.value ];
        searchFrom.forEach( ( searchField ) => {
            const matchIndex =
                normalizeTextString( searchField ).indexOf( match );
            if ( matchIndex === 0 ) {
                startsWithMatch.add( option );
            } else if ( matchIndex > 0 ) {
                containsMatch.add( option );
            }
        } );

    } );
        // return the content of set as array
    return [ ...startsWithMatch, ...containsMatch ];
}, [ inputValue, options ] );
leogermani commented 2 months ago

Awesome @imrraaj , are you planning to submit a PR with this?

imrraaj commented 2 months ago

Thanks @leogermani I'll raise a PR

leogermani commented 2 months ago

I didn't test the changes, and I'm not sure I'm reading this right. But will it display the option if the search string exists in the label OR the value? (currently it only looks in the label)

imrraaj commented 2 months ago

Yes, after applying the changes, it will display the option if the label or value matches to passed string.