Add loading indicator to combo box control author selector #68927

adamsilverstein · 2025-01-28T23:05:53Z

This PR is a rework of the effort in #27865 - see that ticket for additional details.

Testing instructions

Create a large number of editors (see below for command).
Load a page in the editor
Open DevTools and throttle network connections to a 3G speed to slow down the rest API responses
Click the author to edit, then try typing in part of a user name

Expected results:

A spinner is shown while the search callback executes
the "No items found' string only shows when the search completes (and no items are found)

Helpful wp-cli commands for testing:

Generate many users to test searching: wp user generate --role=editor --count=100
Delete test users: wp user delete $(wp user list --role=editor --field=ID) --reassign=2

Screencasts

Before changes (trunk)

before2.mp4

Add spinner

(note message "No items found" shows during search callback)

spinner-only.mp4

Final PR

(including removal of "No items found" during load)

after.mp4

adamsilverstein · 2025-01-28T23:09:36Z

Note: the drop-down shows "No items found." during the search, a bug clearly visible currently (before this PR) when throttling the search callback.

github-actions · 2025-01-29T14:30:51Z

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: adamsilverstein <[email protected]>
Co-authored-by: Mamaduka <[email protected]>
Co-authored-by: ciampo <[email protected]>

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

adamsilverstein · 2025-01-29T14:31:25Z

I updated the PR to avoid showing "No items found" string while the results are still loading.

Mamaduka · 2025-01-29T14:45:03Z

packages/editor/src/components/post-author/combobox.js

@@ -44,13 +45,14 @@ export default function PostAuthorCombobox() {
 		<ComboboxControl
 			__nextHasNoMarginBottom
 			__next40pxDefaultSize
-			label={ __( 'Author' ) }
+			label={ __( 'Authorz' ) }


ha, this was me trying to get my build working and not seeing changes. will revert!

Mamaduka · 2025-01-29T15:38:23Z

packages/components/src/form-token-field/suggestions-list.tsx

@@ -157,7 +158,7 @@ export function SuggestionsList<
 				);
 				/* eslint-enable jsx-a11y/click-events-have-key-events */
 			} ) }
-			{ suggestions.length === 0 && (
+			{ suggestions.length === 0 && ! isLoading && (


The SuggestionsList component is shared, which is why we're passing the isLoading prop, correct?

Mamaduka

Thanks, @adamsilverstein!

My suggestion is to limit new loading state changes to the ComboboxControl. The FormTokenField component is a bit tricky, as input and tokens can wrap on multiple lines. So, I'm not sure if the spinner suffix design will work for it.

Do you mind including a screenshot or screencast in the PR description?

Hopefully, the v2 design will account for the loading state of #64086.

cc @WordPress/gutenberg-components

adamsilverstein · 2025-01-29T21:26:10Z

My suggestion is to limit new loading state changes to the ComboboxControl. The FormTokenField component is a bit tricky, as input and tokens can wrap on multiple lines. So, I'm not sure if the spinner suffix design will work for it.

Thanks for the review.

I will add some screenshots/screencasts to show what this change does (when I am back at my desk Friday). I can also take a look at other consumers of the component to make sure they are unaffected. The main goal of the change here is to avoid showing "No items found" string while the (author) search is underway. Since it isn't directly related to adding a spinner, I can break it out into a follow up PR and we can explore the issue separately.

Mamaduka · 2025-01-30T03:45:25Z

I think it's okay to change the SuggestionsList here, but let's avoid adding the isLoading prop to the FormTokenField. This way we can focus only on ComboboxControl improvements.

ciampo

Finally, I would also suggest

packages/components/src/combobox-control/index.tsx

ciampo · 2025-01-31T14:35:08Z

packages/editor/src/components/post-author/combobox.js

 			options={ authorOptions }
 			value={ authorId }
 			onFilterValueChange={ debounce( handleKeydown, 300 ) }
 			onChange={ handleSelect }
 			allowReset={ false }
 			hideLabelFromVision
+			isLoading={ isLoading }


My preference would we to focus this PR solely on the ComboboxControl component.

We should add a Storybook example here to allow for easy testing of the component in isolation.

In a separate, follow-up PR, we could then tweak the post author combobox to use the new prop

The entire point of this PR is to improve the author selector - changing only the combobox won't actually change anything, shouldn't we aim for PRs that actually improve things?

I can look into adding to the storybook, would this just be a variation with the isLoading prop?

The @wordpress/component package is not only used in the Gutenberg repository, but it's also used by third-party developers. Therefore, we usually try to keep changes to the "library" separate from changes to the Gutenberg app (hence why I suggest testing the changes in isolation in Storybook).

Having smaller and more focused PRs is also easier to review, and understand for a future developer stumbling upon it.

Having smaller and more focused PRs is also easier to review, and understand for a future developer stumbling upon it.

I understand the principal, however this entire PR is currently less than 35 lines modified so I think is already easy to review and understand! Given the testing instructions I provided, I feel having the PR do something makes it easier to test and understand.

That said, I sense that you feel strongly about this and I will defer to your more recent experience building here. I will split the combobox and author changes into separate PRs.

I created a follow up PR with just the Comobox changes here:

#68990

This is a follow up PR to introduce the post author changes:

#68991

Let's continue the conversation on those PRs.

packages/components/src/combobox-control/types.ts

ciampo · 2025-01-31T14:43:41Z

packages/components/src/combobox-control/index.tsx

@@ -126,6 +127,7 @@ function ComboboxControl( props: ComboboxControlProps ) {
 		help,
 		allowReset = true,
 		className,
+		isLoading = false,


Adding this prop won't affect existing usages, although I wonder if it's a good idea, API-wise.

The component doesn't have a canonical example of how to use it with asynchronously loaded options, and I wonder if adding this prop will improve the DevX (related to #55574) — @WordPress/gutenberg-components what do you think?

If we want to encourage this pattern, we should at least add a Storybook example about it?

I wonder if adding this prop will improve the DevX

Not sure about DevX, this change is about improving User Experience. As is, the combobox selector provides a poor UX when the internet connection is slow or unreliable. The user has no way of knowing that a search is underway during the async search. This shortcoming was overlooked when we originally built the component and I feel this more of a bug than an enhancement.

adamsilverstein · 2025-01-31T14:59:02Z

I think it's okay to change the SuggestionsList here, but let's avoid adding the isLoading prop to the FormTokenField. This way we can focus only on ComboboxControl improvements.

I will remove these changes for now and we can consider adding them in a follow up PR.

I have included screencasts above so you can see why I added this change in the first place.

Co-authored-by: Marco Ciampini <[email protected]>

This reverts commit 76488c1.

adamsilverstein · 2025-01-31T15:01:50Z

I think it's okay to change the SuggestionsList here, but let's avoid adding the isLoading prop to the FormTokenField. This way we can focus only on ComboboxControl improvements.

Reverted in fd97afc

adamsilverstein · 2025-01-31T15:23:59Z

@ciampo - In 3a357c4 I added the !isLoading conditional so the suggestion list only shows when the component isn't loading data, what do you think? I will add a screencast to show how this works.

ciampo · 2025-01-31T15:34:51Z

packages/components/src/combobox-control/README.md

@@ -112,13 +113,20 @@ If the control is clicked, the dropdown will expand regardless of this prop.
 -   Required: No
 -   Default: `true`

-### placeholder
+#### placeholder


ciampo · 2025-01-31T15:35:15Z

packages/components/src/combobox-control/README.md


 If passed, the combobox input will show a placeholder string if no values are present.

 -   Type: `string`
 -   Required: No

+#### isLoading
+
+If true, the dropdown will show a loading indicator.


Let's match the same description as the one from types.ts

adamsilverstein · 2025-01-31T16:40:45Z

Closing in favor of #68991 and #68990

Add loading indicator to author selector

f73720d

adamsilverstein mentioned this pull request Jan 28, 2025

ComboboxControl: add a spinner when isLoading. #27865

Closed

6 tasks

Mamaduka added [Type] Enhancement A suggestion for improvement. Needs Design Feedback Needs general design feedback. [Package] Components /packages/components labels Jan 29, 2025

Only show "No items found" when isLoading is false

76488c1

adamsilverstein marked this pull request as ready for review January 29, 2025 14:30

adamsilverstein requested a review from ajitbohra as a code owner January 29, 2025 14:30

adamsilverstein requested review from Mamaduka and removed request for ajitbohra January 29, 2025 14:30

adamsilverstein changed the title ~~Add loading indicator to author selector~~ Add loading indicator to combo box control author selector Jan 29, 2025

Mamaduka reviewed Jan 29, 2025

View reviewed changes

Merge branch 'trunk' into add/combobox-loading-spinner-refresh

3c7baa6

ciampo reviewed Jan 31, 2025

View reviewed changes

Remove unitended change

07690d6

adamsilverstein and others added 2 commits January 31, 2025 07:59

Update packages/components/src/combobox-control/types.ts

736c5ff

Co-authored-by: Marco Ciampini <[email protected]>

Revert "Only show "No items found" when isLoading is false"

fd97afc

This reverts commit 76488c1.

adamsilverstein added 2 commits January 31, 2025 08:19

Only show the suggestion list when the component isn’t loading data

3a357c4

Document isLoading prop in readme

374b2b8

ciampo reviewed Jan 31, 2025

View reviewed changes

adamsilverstein added 2 commits January 31, 2025 09:17

Add is loading to Combobox storybook

45bbc6b

Match readme description to type.js description

cc0e64a

This was referenced Jan 31, 2025

Add a loading indicator (isLoading props) to ComboboxControl #68990

Open

Add/combobox loading spinner author part #68991

Open

adamsilverstein closed this Jan 31, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add loading indicator to combo box control author selector #68927

Add loading indicator to combo box control author selector #68927

adamsilverstein commented Jan 28, 2025 •

edited

Loading

adamsilverstein commented Jan 28, 2025 •

edited

Loading

github-actions bot commented Jan 29, 2025 •

edited

Loading

adamsilverstein commented Jan 29, 2025

Mamaduka Jan 29, 2025

adamsilverstein Jan 29, 2025

Mamaduka Jan 29, 2025

adamsilverstein Jan 29, 2025

Mamaduka left a comment

adamsilverstein commented Jan 29, 2025

Mamaduka commented Jan 30, 2025

ciampo left a comment

ciampo Jan 31, 2025

adamsilverstein Jan 31, 2025

ciampo Jan 31, 2025

adamsilverstein Jan 31, 2025

adamsilverstein Jan 31, 2025

ciampo Jan 31, 2025

adamsilverstein Jan 31, 2025 •

edited

Loading

adamsilverstein commented Jan 31, 2025

adamsilverstein commented Jan 31, 2025

adamsilverstein commented Jan 31, 2025

ciampo Jan 31, 2025

ciampo Jan 31, 2025

adamsilverstein Jan 31, 2025

adamsilverstein commented Jan 31, 2025

Add loading indicator to combo box control author selector #68927

Add loading indicator to combo box control author selector #68927

Conversation

adamsilverstein commented Jan 28, 2025 • edited Loading

Testing instructions

Helpful wp-cli commands for testing:

Screencasts

Before changes (trunk)

Add spinner

Final PR

adamsilverstein commented Jan 28, 2025 • edited Loading

github-actions bot commented Jan 29, 2025 • edited Loading

adamsilverstein commented Jan 29, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Mamaduka left a comment

Choose a reason for hiding this comment

adamsilverstein commented Jan 29, 2025

Mamaduka commented Jan 30, 2025

ciampo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

adamsilverstein Jan 31, 2025 • edited Loading

Choose a reason for hiding this comment

adamsilverstein commented Jan 31, 2025

adamsilverstein commented Jan 31, 2025

adamsilverstein commented Jan 31, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

adamsilverstein commented Jan 31, 2025

adamsilverstein commented Jan 28, 2025 •

edited

Loading

adamsilverstein commented Jan 28, 2025 •

edited

Loading

github-actions bot commented Jan 29, 2025 •

edited

Loading

adamsilverstein Jan 31, 2025 •

edited

Loading