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

No visual feedback when Parent Page dropdown is loading. #16026

Open
iandunn opened this issue Jun 6, 2019 · 20 comments
Open

No visual feedback when Parent Page dropdown is loading. #16026

iandunn opened this issue Jun 6, 2019 · 20 comments
Labels
[Feature] Inspector Controls The interface showing block settings and the controls available for each block [Package] Components /packages/components [Type] Bug An existing feature does not function as intended

Comments

@iandunn
Copy link
Member

iandunn commented Jun 6, 2019

Is your feature request related to a problem? Please describe.

There's no visual feedback when the Parent Page dropdown in the Page Attributes inspector control is being populated via a REST API endpoint.

I open the Page Attributes panel, and then find myself thinking, "Wait, where's the Parent Page dropdown? I thought it was under this panel." And then, depending on how long the request takes to finish, I either go off in search of it by looking in other panels (and possibly closing Page Attributes in the process), or it appears just in time, and I realize that it was simply loading all along.

Describe the solution you'd like

Is there a compelling reason why this data needs to be fetched from the API, instead of just being passed from PHP to JS during the initial page request (i.e., with wp_add_inline_script()). The latter would make it available immediately, rather than introducing all of the overhead and latency of an HTTP request.

Although, I guess in some cases the number of pages could be so large that it would significantly slow down the initial page request. So maybe instead it'd be better to pre-fetch the data once the page has loaded, rather than waiting until the user has opened the panel?

Describe alternatives you've considered

If those options aren't good, then we should at least add immediately show the Parent Page label with a disabled dropdown, and a spinner, so that the user knows the application is loading data. When the data returns, the dropdown can be populated and enabled.

Related issues (which won't solve the underlying problem)

#13618
#15115

@iandunn iandunn added [Type] Bug An existing feature does not function as intended Needs Design Feedback Needs general design feedback. [Feature] Inspector Controls The interface showing block settings and the controls available for each block Needs Technical Feedback Needs testing from a developer perspective. labels Jun 6, 2019
@mapk
Copy link
Contributor

mapk commented Jun 25, 2019

I'm not entirely sure the reason behind the path we chose here, but I agree that we don't want to add to the initial page load time.

As discussed in today's triage in the Design channel in slack, this sounds like a good idea.

If those options aren't good, then we should at least add immediately show the Parent Page label with a disabled dropdown, and a spinner, so that the user knows the application is loading data. When the data returns, the dropdown can be populated and enabled.

I'm removing Needs Design Feedback and adding Needs Design for this particular part.

@mapk mapk added Needs Design Needs design efforts. and removed Needs Design Feedback Needs general design feedback. labels Jun 25, 2019
@karmatosed karmatosed self-assigned this Jun 26, 2019
@karmatosed karmatosed removed their assignment Jul 29, 2019
@mapk
Copy link
Contributor

mapk commented Dec 27, 2019

Here's what the disabled parent dropdown would look like. I like @iandunn's suggestion of just using PHP to solve this because I'm unaware why we're using the API here, but the lag is real and it would be great to either fix the lag, or at the very least show a disabled dropdown like this before the real dropdown appears.

disabled parent dropdown

@mapk mapk added Needs Dev Ready for, and needs developer efforts and removed Needs Design Needs design efforts. labels Dec 27, 2019
@iandunn
Copy link
Member Author

iandunn commented Dec 27, 2019

I think that would solve a big part of the problem 👍

I do worry that people would still be confused as to why it's disabled, though. What do you think about having some kind of "loading" placeholder?

@mapk
Copy link
Contributor

mapk commented Dec 27, 2019

While I really like the reasoning in that documentation, I don't think we use the blueprint loading effect in Core, do we? In this case we could include the spinner component which might look something like this:

spinner

@iandunn
Copy link
Member Author

iandunn commented Dec 27, 2019

I can't think of any instances in Core or Gutenberg, so maybe a spinner would be better. I personally feel like placeholders are better, but that's probably outside the scope of this ticket.

For now, I think a spinner here would prevent confusion 👍

@karmatosed
Copy link
Member

This brings up the potential need for a loader that adapts because not sure the circle one works. For now, though I would advise this goes beside the thing it's loading as we don't need to put below, the UI element will populate that.

image

@karmatosed karmatosed added the Needs Design Feedback Needs general design feedback. label Jan 6, 2020
@ItsJonQ
Copy link

ItsJonQ commented Jan 6, 2020

Hallooo 👋 !

@iandunn + @karmatosed I see where you folks are coming from! I agree that having a (loading) indicator of some kind would be helpful. Like you mentioned @karmatosed , having the loading circle may not be the best.

Ideally, I think the base/core/primitive components should have a built-in loading state of some kind.

Example:

Screen Shot 2020-01-06 at 9 59 19 AM

It solves the UX issue @iandunn originally mentioned - not only in this one particular case but in all parts of Gutenberg.

From a code standpoint, having the mechanism being built into the core components is incredibly valuable. It takes care of a lot of rendering/internal/accessibility complexities - simplifying the code to being something like isLoading={true/false}.

@ItsJonQ
Copy link

ItsJonQ commented Jan 6, 2020

@iandunn + @karmatosed How's this? :D

Screen Capture on 2020-01-06 at 15-39-19

Note: I didn't focus too much on aesthetics. Personally, I don't know if the current spinner design UI works well in these embedded loading contexts. For consistency, I used the Spinner as is for now

P.S. Accessibility is taken care of as well, as the isLoading trigger controls the aria-busy attribute

@iandunn
Copy link
Member Author

iandunn commented Jan 6, 2020

That looks good to me, thanks!

@mapk
Copy link
Contributor

mapk commented Jan 6, 2020

This is great, @ItsJonQ! I think using the Spinner component as you have it right now might be a good interim solution. Do we need a list of which components can benefit from this feature?

@karmatosed
Copy link
Member

Love it! That's ace @ItsJonQ.

@ItsJonQ
Copy link

ItsJonQ commented Jan 7, 2020

Thanks all!

Do we need a list of which components can benefit from this feature?
@mapk Sure. That wouldn't hurt. I'm starting off with form control based Components, so Buttons, Select, etc... :)

@MichaelArestad
Copy link
Contributor

@ItsJonQ is there a PR for this yet? 🤞

@MichaelArestad MichaelArestad removed the Needs Design Feedback Needs general design feedback. label Apr 6, 2020
@ItsJonQ
Copy link

ItsJonQ commented Apr 6, 2020

@MichaelArestad Ah, it's been a while. My apologies! I'll revisit this work/PR this afternoon 🙏

@github-actions github-actions bot added the [Status] In Progress Tracking issues with work in progress label Apr 6, 2020
@ItsJonQ
Copy link

ItsJonQ commented Apr 6, 2020

Haii! I just created a PR here for this:
#21435

The isLoading state looks like this:

Screen Capture on 2020-04-06 at 14-06-16

This is different compared to the mockups/renders earlier in this issue. I kept the changes minimal.

I'm unsure how the UIs (spinner, select, etc...) will be adjusted with the evolving editor UI (aka. G2). I can of course make best guesses based on mockups, Figma files, and shared work.

However, I feel like this design adjustment can be done as a complimentary follow up to the functional change, that is, providing the SelectControl and TreeSelect with the ability to handling loading states.

@talldan talldan removed Needs Dev Ready for, and needs developer efforts Needs Technical Feedback Needs testing from a developer perspective. labels Apr 21, 2020
@ndiego
Copy link
Member

ndiego commented Jul 12, 2022

@iandunn this issue came up in today's Editor Bug Scrub. Do you think this is still an issue? There have been some changes made to the Parent Page dropdown. It does not appear that some of the potential fixes detailed in this issue were ever finalized, but in my testing, the dropdown is now visible on page load while pages are being fetched.

I will be following up on the linked PR to see if we can move that along as well.

@iandunn
Copy link
Member Author

iandunn commented Jul 12, 2022

It's definitely better than it used to be 🍻

IMO it's still not a great UX, though. The dropdown is empty until the data is loaded, which can take several seconds on a slower connection. If you open it during that time it can be confusing.

I think it's be better if the dropdown had a disabled attribute and a spinner until it has finished loading.

@ndiego
Copy link
Member

ndiego commented Jul 12, 2022

Agreed, I will see if we can get some traction on the linked PR.

@ndiego ndiego removed the [Status] In Progress Tracking issues with work in progress label Jul 26, 2022
@mirka
Copy link
Member

mirka commented Aug 4, 2022

Noting that the Parent Page control is now a combobox, not a TreeSelect as in the previous implementation (#25267). So the loading state will need to be implemented on ComboboxControl component, not SelectControl as in the linked PR.

@mirka mirka added the [Package] Components /packages/components label Aug 4, 2022
@iandunn
Copy link
Member Author

iandunn commented Sep 11, 2023

I noticed this issue with the Author dropdown as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Inspector Controls The interface showing block settings and the controls available for each block [Package] Components /packages/components [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants