Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Combobox, FormTokenField: show message when no matches found #66142

Conversation

vykes-mac
Copy link
Contributor

@vykes-mac vykes-mac commented Oct 15, 2024

What?

The PR allows the ComboxControl and FormTokenField components to show a message when there are no suggestions matching the input text.

Why?

This PR is more of an enhancement and provides a better user experience when searching the ComboboxControl and FormTokenField components.

How?

Displays a "No items found" message when no suggestions are found in the SuggestionsList component.

Testing Instructions

Open Storybook and pick the Components > ComboboxControl component. Search for an item that doesn't exist
and you should see No items found state.

Screenshots or screencast

image

Copy link

github-actions bot commented Oct 15, 2024

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: vykes-mac <[email protected]>
Co-authored-by: mirka <[email protected]>
Co-authored-by: ciampo <[email protected]>
Co-authored-by: tyxla <[email protected]>
Co-authored-by: jasmussen <[email protected]>
Co-authored-by: matt-west <[email protected]>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

@tyxla tyxla added [Package] Components /packages/components [Type] Enhancement A suggestion for improvement. labels Oct 16, 2024
@tyxla tyxla requested a review from a team October 16, 2024 07:21
@ciampo
Copy link
Contributor

ciampo commented Oct 16, 2024

Thank you for opening this PR!

@WordPress/gutenberg-components , what do we think about adding a similar feature directly to the component (no props) and displaying a more generic message? Something like "No items found".

@tyxla
Copy link
Member

tyxla commented Oct 16, 2024

@WordPress/gutenberg-components , what do we think about adding a similar feature directly to the component (no props) and displaying a more generic message? Something like "No items found".

Yeah, I think it might be a better experience than what we're doing now (showing nothing):

Screenshot 2024-10-16 at 13 42 03

@WordPress/gutenberg-design any thoughts or preferences?

@jasmussen
Copy link
Contributor

Something like "No items found".

Sounds good to me. Any downsides?

@tyxla
Copy link
Member

tyxla commented Oct 16, 2024

Something like "No items found".

Sounds good to me. Any downsides?

Can't think of any. Even that string exists already and will inherit existing translations immediately:

@ciampo
Copy link
Contributor

ciampo commented Oct 16, 2024

@vykes-mac could you change this PR so that the message is always shown (no extra props needed) and the text reads "No items found" ? Thank you 🙏

@matt-west
Copy link
Contributor

Thanks for implementing this @vykes-mac. 🙌

@vykes-mac
Copy link
Contributor Author

@ciampo What if we want to customise the copy that's displayed there? Eg. the list is showing Authors instead of No items found I would like to pass my custom messaging No authors found .

@mirka
Copy link
Member

mirka commented Oct 16, 2024

@ciampo What if we want to customise the copy that's displayed there? Eg. the list is showing Authors instead of No items found I would like to pass my custom messaging No authors found .

Would "No matches found" be a more generic wording in this case?

To be honest I prefer not adding a prop right now to customize this, as it will naturally be customizable in the planned version 2 of this component, which will be based on Ariakit and more modular.

@vykes-mac
Copy link
Contributor Author

@ciampo What if we want to customise the copy that's displayed there? Eg. the list is showing Authors instead of No items found I would like to pass my custom messaging No authors found .

Would "No matches found" be a more generic wording in this case?

To be honest I prefer not adding a prop right now to customize this, as it will naturally be customizable in the planned version 2 of this component, which will be based on Ariakit and more modular.

@mirka I'll proceed with the No items found found then since translation already exist for the string

Copy link
Member

@mirka mirka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the follow-up! I found a styling issue, but otherwise it's testing well both in FormTokenField and ComboboxControl.

@@ -156,6 +157,11 @@ export function SuggestionsList<
);
/* eslint-enable jsx-a11y/click-events-have-key-events */
} ) }
{ suggestions.length === 0 && (
<li className="components-form-token-field__suggestion is-empty">
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Were you planning on styling the is-empty modifier? I'm thinking we at least need to move the cursor style:

	&:not(.is-empty) {
		cursor: pointer;
	}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the class is more of placeholder so that where the component is used the user can easily overide the styling with is-empty class. But your suggestion here make sense, I'll add it

Copy link
Contributor

@ciampo ciampo Oct 18, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

where the component is used the user can easily overide the styling with is-empty class.

That is something that we actively discourage — internal classnames are not public APIs and should not be considered as such.

Please do not use the .is-empty classname as a way to apply style overrides.

@@ -19,6 +19,7 @@

- `Modal`: Modal dialog small improvement for elementShouldBeHidden ([#65941](https://github.com/WordPress/gutenberg/pull/65941)).
- `Tabs`: revamped vertical orientation styles ([#65387](https://github.com/WordPress/gutenberg/pull/65387)).
- `ComboboxControl`: display `No items found` when there are no matches found ([#66142](https://github.com/WordPress/gutenberg/pull/66142))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like this also affects the FormTokenField component when the __experimentalExpandOnFocus prop is enabled.

Copy link
Contributor

@ciampo ciampo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 🚀

Before merging, could you move the two CHANGELOG entries under the "Unreleased" section? This is necessary because a new version of the package was released since this PR was opened.

Thank you 🙏

@ciampo ciampo changed the title Combobox/add custom element when suggestions not found Combobox, FormTokenField: show message when no matches found Oct 21, 2024
@ciampo ciampo enabled auto-merge (squash) October 21, 2024 15:21
@ciampo ciampo merged commit 6fbca55 into WordPress:trunk Oct 21, 2024
62 checks passed
@github-actions github-actions bot added this to the Gutenberg 19.6 milestone Oct 21, 2024
@Mamaduka
Copy link
Member

As a follow-up, can we modify this by allowing a custom component? I think it might help us resolve the first item on this list: #64086 (comment).

@ciampo
Copy link
Contributor

ciampo commented Oct 22, 2024

As Lena mentioned above, we'd rather unlock this level of customizability in the v2 of the component, rather than putting in work on the v1 (unless we're talking about a very important feature / urgent hotfix). There's already a lot on our plate and we're trying to prioritise our focus where it's needed the most.

@Mamaduka
Copy link
Member

V2 sounds good 👍

karthick-murugan pushed a commit to karthick-murugan/gutenberg that referenced this pull request Nov 13, 2024
…ss#66142)

Co-authored-by: vykes-mac <[email protected]>
Co-authored-by: mirka <[email protected]>
Co-authored-by: ciampo <[email protected]>
Co-authored-by: tyxla <[email protected]>
Co-authored-by: jasmussen <[email protected]>
Co-authored-by: matt-west <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Package] Components /packages/components [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants