ComboboxControl: Add an isLoading prop to show a loading spinner
#68990
Conversation
Co-authored-by: Marco Ciampini <[email protected]>
This reverts commit 76488c1.
|
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 If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
| @@ -124,6 +124,7 @@ WithCustomRenderItem.args = { | |||
| </div> | |||
| ); | |||
| }, | |||
| isLoading: true, | |||
There was a problem hiding this comment.
@ciampo I added this property, is that right? I was expecting a change when I ran npm run storybook:build but there were none. Do I need to so something else to regenerate storybook assets?
There was a problem hiding this comment.
I think we can remove this. People can use controls in the storybook to toggle the value.
Screenshot
There was a problem hiding this comment.
I think we can remove this. People can use controls in the storybook to toggle the value.
I assume that storybook screenshot is your local. Since I can't typically run Docker locally I was wondering if storybook was available as part of the CI (before we merge the PR)?
There was a problem hiding this comment.
Running npm run storybook:dev should do the trick locally and doesn't require Docker. IIRC, there are no PR-specific builds for Storybook.
There was a problem hiding this comment.
Hey folks, sorry for the delayed reply — not on Gutenberg much these days.
What I meant with #68927 (comment) was to add a new Storybook example that showcases ComboboxControl being used with async loading options, as we do for the author input.
@adamsilverstein and/or @Mamaduka , would any of you be up for this as a nice follow-up?
There was a problem hiding this comment.
I'm unsure how to emulate async data in Storybook; I'm guessing with a setTimeout and Promise combo. Do we have any similar examples?
There was a problem hiding this comment.
Not sure, but yeah — that sounds reasonable. Maybe something like:
- first render with empty (or a limited set of)
options - when clicking a "load more options" button, we trigger an async request:
- the request could be arbitrarily slowed down (promise with timeout);
- while the data is loading, we show a spinner
WDYT ?
There was a problem hiding this comment.
Yeah, sounds good.
I will add it to my list, but I'm unsure when I'll have time to work on it.
There was a problem hiding this comment.
Sounds good to me, definitely not a priority, rather a nice example to have. Thank you as always 🙏
adamsilverstein
added
[Type] Enhancement
There was a problem hiding this comment.
Thank you, @adamsilverstein! I think this is almost ready for merging.
I noticed that the spacing for the spinner icon is a bit off. Overriding the default margin with 0 should fix it.
P.S. We should also add the changelog entry for this change.
Before
After
Co-authored-by: George Mamadashvili <[email protected]>
Co-authored-by: George Mamadashvili <[email protected]>
Thanks for the feedback. Adjusted in e526287 |
Good point. Added in 782d82a, please review, suggestions appreciated |
There was a problem hiding this comment.
Thanks for the follow-up, @adamsilverstein!
This looks good to merge.
P.S. CI checks failures look unrelated. It might be easier to restart all of them via rebasing.
CleanShot.2025-02-04.at.21.28.52.mp4
Mamaduka
changed the title
isLoading props) to ComboboxControlisLoading prop to show a loading spinner
|
Thank you both for shipping this one, and for doing it in a separate PR from the editor change 🙏 |
What?
Add a loading indicator to the
ComboboxControlcomponent.ComboboxControl only part of #68927