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

Provide loading UX when UI is dependent on an API request #6723

Open
danielbachhuber opened this issue May 12, 2018 · 27 comments
Open

Provide loading UX when UI is dependent on an API request #6723

danielbachhuber opened this issue May 12, 2018 · 27 comments
Labels
[Feature] Document Settings Document settings experience [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). Needs Dev Ready for, and needs developer efforts REST API Interaction Related to REST API
Milestone

Comments

@danielbachhuber
Copy link
Member

Some parts of the Gutenberg UI won't display until an API request has completed. When the API request takes a non-zero amount of time, the end user can be confused as to why the UI isn't yet displaying.

For example, here's a GIF of a scenario where the Parent Page selection UI takes 4 seconds to load:

pagesloading

It would be helpful to have some form of visual cue to indicate that UI is meant to be there, but contingent on the result of an API request.

@danielbachhuber danielbachhuber added [Feature] Document Settings Document settings experience Needs Design Feedback Needs general design feedback. labels May 12, 2018
@danielbachhuber danielbachhuber added this to the WordPress 5.0 milestone May 12, 2018
@danielbachhuber danielbachhuber added Needs Design Needs design efforts. [Priority] Low Used to indicate that the issue at hand isn't a top priority to address and can be handled later and removed Needs Design Feedback Needs general design feedback. labels May 15, 2018
@karmatosed
Copy link
Member

karmatosed commented May 19, 2018

I have concerns over loaders flashing up, can we set this to be a longer time than 'no-zero' seconds? I say this as flashing loaders are worse than no loaders for visual issues.

Loader wise @jasmussen for consistency regarding icons, can you suggest a loader for this please? My preference would be a simple '. . .' type loader.

@jasmussen
Copy link
Contributor

jasmussen commented May 22, 2018

I have concerns over loaders flashing up, can we set this to be a longer time than 'no-zero' seconds? I say this as flashing loaders are worse than no loaders for visual issues.

Seems like this could be cool. I.e. the loader could be there always for loading, but have a CSS transition to "fade in and show itself" after 1 or 2 seconds passed. Then for any situations where the loading was faster than that, it would never fade in, and provide an "optimistic loading" scenario.

Loader wise @jasmussen for consistency regarding icons, can you suggest a loader for this please? My preference would be a simple '. . .' type loader.

I feel like this is one of the situations where we should use the spinner that core comes with, and then redesign that one day. Referring to this little fella:

screen shot 2018-05-22 at 09 27 16

(sorry that's not animated, it was hard to screen cap, but it rotates)

It's not the best spinner in the world, and it definitely deserves updating. But I feel like it should be the same across WordPress, i.e. "redesigned upstream" if need be. By the way I think we already wrapped this spinner in a component.

@karmatosed
Copy link
Member

karmatosed commented May 22, 2018

Seems like this could be cool. I.e. the loader could be there always for loading, but have a CSS transition to "fade in and show itself" after 1 or 2 seconds passed. Then for any situations where the loading was faster than that, it would never fade in, and provide an "optimistic loading" scenario.

That seems like the best of both worlds so yes.

It's not the best spinner in the world, and it definitely deserves updating. But I feel like it should be the same across WordPress, i.e. "redesigned upstream" if need be. By the way I think we already wrapped this spinner in a component.

Works for me.

@karmatosed
Copy link
Member

I am removing design tag as the loader has been decided for start to be the one use across WordPress.

@karmatosed karmatosed added Needs Dev Ready for, and needs developer efforts and removed Needs Design Needs design efforts. labels May 29, 2018
@mtias
Copy link
Member

mtias commented Jun 22, 2018

I'd avoid using spinners for this and instead build "loading" states into each component to reflect the final UI. Related: https://wpcalypso.wordpress.com/devdocs/docs/reactivity.md

Designing the states in which various UI pieces can be is important and should not be replaced just with generic spinners.

@karmatosed
Copy link
Member

The thinking was to use the loaders as a base to be just like WP. If states can be used then absolutely we should. The only caution here is we may want to patch throughout WordPress on merge to do that if possible. A state where we have some as loaders and some as states could be less than great.

@mtias mtias added the Needs Design Needs design efforts. label Jul 19, 2018
@hedgefield
Copy link

True, optimizing for first paint and async population of a skeleton blueprint would look nicer, as it reserves the space needed for the elements. Though even then some parts may take a while. So whether the skeleton is animated or it has a loader on/under/next to it is isn't a super big difference, or no? Could a loader be a simple v1 to alleviate this on the short term?

As a related example, we implemented loading spinners as placeholders for our analysis bullets to show you when it's still 'thinking'. (No gif but you get the point)

loading 1

@jasmussen
Copy link
Contributor

Sidenote — killer work on the Yoast sidebar Tim! I noticed the panel titles are a larger font size than stock Gutenberg panel sizes — any reason for that?

@hedgefield
Copy link

Thanks! Yes, that's the result of us needing to accomodate two subheading levels in the analysis section, so starting from an 'H1' that is 13px didn't leave much room to differentiate with size 😄

@jasmussen
Copy link
Contributor

Thanks! Yes, that's the result of us needing to accomodate two subheading levels in the analysis section, so starting from an 'H1' that is 13px didn't leave much room to differentiate with size 😄

Just to be clear, I'm not trying to influence anything here — I'm only excited to see a plugin itilize the sidebar API so well ;) — I think it's a great experience. But why do you have to differentiate those sizes?

@hedgefield
Copy link

Well, we have quite a bit of text in the analysis sections, divided into different areas, plus collapsible subheaders. If all of it was the same size it would be hard to parse what's what, even when using boldness or color. The help text in each of our sections, too, is the very smallest that we could make it for a11y compliance, because the body font of sidebar text is already quite small on modern resolutions.

But hit me up on slack if you want to talk shop more, don't want to derail this issue ;)

@jasmussen
Copy link
Contributor

Thanks Tim, really appreciate it, and sorry for the slight derailment, it was just intriguing!

@mtias mtias removed the [Priority] Low Used to indicate that the issue at hand isn't a top priority to address and can be handled later label Oct 12, 2018
@mtias
Copy link
Member

mtias commented Oct 12, 2018

I think this should be addressed.

@danielbachhuber
Copy link
Member Author

Moving into the 4.1 milestone as this can unblock #6694. If we have this UX, then we can load data into Gutenberg with ?per_page=100&page=1, ?per_page=100&page=2.

@karmatosed
Copy link
Member

karmatosed commented Oct 15, 2018

Can we bring in the circular indicator here please:

2018-10-15 at 15 11

To be clear we'd be removing using the current loaders from buttons like publish as part of this and using this status indicator throughout.

Publish buttons would:

  • Not show 'updating' they would just show this indicator.
  • Save button would become disabled while post is updating.

2018-10-15 at 15 38

Any place that needs a loader would use this circular indicator: https://material.io/design/components/progress-indicators.html#circular-progress-indicators.

@afercia
Copy link
Contributor

afercia commented Oct 15, 2018

Not show 'updating' they would just show this indicator.

From an a11y perspective, UI controls should always communicate the available action, not the underlying state. Also, a button should always have text or, less preferably, at least an aria-label

Save button would become disabled while post is updating

This should be carefully implemented to avoid a focus loss. When using a keyboard, the button has focus. After activation, the button becomes disabled and thus is not focusable focus loss. I'd suggest to consider to noop the button instead of disabling it.

@afercia afercia added the [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). label Oct 15, 2018
@moorscode
Copy link
Contributor

Not show 'updating' they would just show this indicator.

What are you trying to solve with this approach?
I think it's not a great idea to remove the context to what the button is for.

I'm assuming you are concerned with the size the button would be if an icon is added to the copy?

@karmatosed
Copy link
Member

Thanks for the rapid feedback. Can I just check something:

a button should always have text or, less preferably, at least an aria-label

Could I be educated a little on this requirement? I would love to do some archaeology if you have any links there please in standards documentation, specifically around it being less preferable to have an aria-label for loading indications, thanks in advance.

@karmatosed
Copy link
Member

What are you trying to solve with this approach?
I think it's not a great idea to remove the context to what the button is for.

I'm assuming you are concerned with the size the button would be if an icon is added to the copy?

Actually no the concern isn't the size, although having it adjust to a smaller size could be helpful, it still says 'publish' and 'update' so that's not really the motivation here. The key here is for this issue we need a progress indicator and having one universal one makes a lot of sense. Having different designed states meaning the same thing isn't great. This would bring a chance to unify.

A little snippet from the Google docs on these indicators I think is good to context:

They should be used for short, indeterminate activities (between 2-5 seconds). Longer activities may require alternate methods of communication, such as snackbars or notifications.

The idea is these are short not lengthy and often having a flash of text appear then go is also not great as an experience. Where right now we have no indication, adding something also makes a lot of sense to improve usability.

@karmatosed karmatosed added Needs Design Feedback Needs general design feedback. and removed Needs Design Needs design efforts. Needs Design Feedback Needs general design feedback. labels Oct 15, 2018
@karmatosed
Copy link
Member

For now, let's implement this loader just on the API request points. @danielbachhuber are you able to implement this to fit in with the API work?

@danielbachhuber
Copy link
Member Author

are you able to implement this to fit in with the API work?

@kadamwhite is picking it up over the next couple of days.

@gziolo
Copy link
Member

gziolo commented Oct 18, 2018

We are wrapping up 4.1 today and I don't see any PR referenced. I'm moving it to 4.2, feel free to move it back to 4.1 if you have something to review before we release 4.1 😃

@youknowriad youknowriad modified the milestones: 4.2, WordPress 5.0 Oct 27, 2018
@danielbachhuber
Copy link
Member Author

@kadamwhite Are you still planning to work on this or do you want to remove your assignment?

@danielbachhuber
Copy link
Member Author

From #11524 (comment), another imprecise behavior to address: even though the request for terms failed, the Tag Post Setting appears like this:

image

I would've expected the Tag Post Setting to better communicate it's incomplete / failed state.

@danielbachhuber
Copy link
Member Author

Moving to WordPress 5.0.x Follow Ups because I don't see this as a 5.0 release blocker and we should focus on those first.

@tomdevisser
Copy link
Member

@danielbachhuber @jasmussen @youknowriad Does anyone have this on their radar? It's been inactive for 4 years, is it still relevant? Can I help moving this ticket forward or can it be closed? :)

@danielbachhuber
Copy link
Member Author

@tomdevisser It's still relevant:

CleanShot.2023-03-13.at.17.49.57.mp4

You're welcome to move the ticket forward, if you'd like. I imagine it's somewhat of a can of worms.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Document Settings Document settings experience [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). Needs Dev Ready for, and needs developer efforts REST API Interaction Related to REST API
Projects
None yet
Development

No branches or pull requests