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

v4.2: Spinners! #22960

Merged
merged 5 commits into from
Nov 13, 2018
Merged

v4.2: Spinners! #22960

merged 5 commits into from
Nov 13, 2018

Conversation

mdo
Copy link
Member

@mdo mdo commented Jul 2, 2017

screen shot 2017-07-01 at 6 53 40 pm

Finally took a stab at coding these up and adding them to Bootstrap! Only took me like five years to do it after CSS support finally allowed a super straightforward, non-JS solution. This isn't 100% complete, and we're not adding it until v4.1, but the mood struck so I whipped it up tonight :).

Includes support for easy coloring, alignment, sizing, and more.

/cc @patrickhlauke for accessibility guidance

Fixes #19556, fixes #12598, fixes part of #7135, fixes #6807, fixes #6461, fixes #6454, fixes #5829, and fixes #1371. 😆

Preview: https://deploy-preview-22960--twbs-bootstrap4.netlify.com/docs/4.1/components/spinners/


Todos

@mdo mdo added this to the v4.1 ideas milestone Jul 2, 2017
@mdo mdo mentioned this pull request Jul 2, 2017
@eliashdezr
Copy link

@mdo
Just to suggest another designs, take a look at these:
http://beta.quasar-framework.org/components/spinner.html

I think the facebook, cube, dots, mats and puff would be nice to have them in Bootstrap :).

@gijsbotje
Copy link
Contributor

If you want to see them in action check this pen

@mdo mdo removed this from the v4.1 ideas milestone Jan 20, 2018
@mdo
Copy link
Member Author

mdo commented Jan 20, 2018

Pushed some updates here to bring the branch up to speed with v4-dev and our stable v4 release. Included are some new docs improvements for headings and copy, button examples, and a slightly reorganized page layout.

screen shot 2018-01-20 at 12 50 51 pm

@mdo
Copy link
Member Author

mdo commented Jan 21, 2018

@patrickhlauke Do we need something like role="alert" aria-busy="true" on all these?

@patrickhlauke
Copy link
Member

patrickhlauke commented Feb 18, 2018

apologies for the radio silence. will explore this in the coming days.

dropping some initial thoughts here:

  • aria-busy is usually added if an element is busy being loaded with new content. i.e. if the button (with spinner) was actually triggering the reload of, say, a table in the page, the table itself would be blanked/obscured and given aria-busy (and not the button that triggered this action). this probably falls outside of what we can explain at length in the docs, but i'll have a think - a demo would help clarify this / give an opportunity to mention it in passing without needing to go into great detail on the theory
  • likely the most appropriate attributes for a spinner is as an indeterminate role="progressbar" with an aria-valuetext="Loading..." - need to test how well this would perform

@patrickhlauke
Copy link
Member

further note:

  • in the button example, nothing will be announced by assistive technologies as any status/live region inside a disabled control is simply ignored, it seems.
  • i've started hacking away at a live prototype button to better understand what AT actually do. as is usually the case with live regions, it's a fragile mess...watch this space https://codepen.io/patrickhlauke/pen/3e86bd48eeb1309d0af9ad1b5a2dc1bd

@mdo
Copy link
Member Author

mdo commented Feb 20, 2018

If I'm reading your pen right, there are a few things to update:

  • role="progressbar" with aria-valuetext="Loading..." are the right a11y attributes to apply.
  • We need to implement some custom behaviors for the loading state since disabled resets focus.

Do we need custom JS added, too? Does this belong in the button JS, a docs snippet, or something else?

@patrickhlauke
Copy link
Member

patrickhlauke commented Feb 20, 2018

got more testing to do. but the main quandary is: the "live" aspect of live regions cannot properly be demonstrated unless it's in an actual live scenario. if your page already contains some with, say, role="status" or similar in the markup that's downloaded from the server, AT won't announce it in any special way on page load. only on any subsequent dynamic content change will they announce it. (if an AT user reads through the page though, once they stumble across the progressbar, it will at least announce it as "Progress", whereas for role="status", or even just a generic aria-live="...", they'll not provide any particular context. some other statuses, like role="alert", may also result in some prefixing/context, not tested yet...but probably not quite appropriate in most cases)

then, there's so many different scenarios where authors may use spinners. do we want to demonstrate them all?

or do we want to just say, basically: look, this is the raw markup for a spinner. depending on how you use it, you'll likely want to make sure that what these spinners convey visually is also conveyed to AT. there's many possibilities, but here's a few examples. maybe link out to a single demo/example page that shows various things in action, all with their specific approaches. perhaps an expanded version of what i've started (and will expand further) in my codepen.

not sure we want/need to enshrine specific variants of buttons that flip/flop between ready/busy, or special kinds of dialogs, or similar. those are perhaps too implementation-specific to be made generic enough.

@patrickhlauke
Copy link
Member

btw, as an aside, after ranting a bit on twitter, i updated https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/ARIA_Live_Regions to highlight the "you need to prime your live region first" thing about live regions, as it's a detail that's often overlooked by devs...

@patrickhlauke
Copy link
Member

@mdo see my question above #22960 (comment)

or do we want to just say, basically: look, this is the raw markup for a spinner. depending on how you use it, you'll likely want to make sure that what these spinners convey visually is also conveyed to AT. there's many possibilities, but here's a few examples. maybe link out to a single demo/example page that shows various things in action, all with their specific approaches. perhaps an expanded version of what i've started (and will expand further) in my codepen.

@mdo mdo requested a review from a team as a code owner November 2, 2018 11:09
@MartijnCuppens
Copy link
Member

We prevent transitions if prefers-reduced-motion: reduce is enabled (see transition mixin). Maybe its better to hide these spinners in that case?

@XhmikosR
Copy link
Member

XhmikosR commented Nov 2, 2018

Why even generate the CSS in that case? :)

EDIT:
Ah, you mean in run time. Still, I think we should wrap the mixin code in @if $enable-transitions although I don't like the extra indentation but we need some kind of a hack to return early :)

@mdo
Copy link
Member Author

mdo commented Nov 3, 2018

My gut reaction is to not hide spinners when reduced motion is enabled. A spinner is a more critical piece of user feedback than something sliding or fading in.

@patrickhlauke
Copy link
Member

agree, it's my understanding that issues relating to vestibular motion sensitivity are tied to large motion/movement/parallax effects. spinners should be relatively safe.

@midzer
Copy link
Contributor

midzer commented Nov 8, 2018

Yup, they are rather "static", meditativ pieces which should not arouse tension. Especially because of their ease animation it creates a softening visual effect for me.

Unless some1 else enjoys supporting bootstrap by refactoring this PR, I can do it once I got several other issues fixed.

@XhmikosR XhmikosR force-pushed the spinner branch 2 times, most recently from 65a5aeb to a480216 Compare November 11, 2018 09:13
@mdo mdo mentioned this pull request Nov 11, 2018
@sts-ryan-holton
Copy link
Contributor

It would be nice to have a class like Bulma which adds the spinner, e.g: .is-loading that can be added to a button and it would use the spinner

@midzer
Copy link
Contributor

midzer commented Nov 11, 2018

@sts-ryan-holton you can simply wrap the spinner in a <button> like

<button class="btn btn-secondary">
  <div class="spinner-border-sm text-primary">Loading...</div>
  MYBUTTON
</button>

for example

@sts-ryan-holton
Copy link
Contributor

@midzer Absolutely, but not with JS I can't. In the case of Bulma, adding a is-loading class to any button automatically inserts a spinner using a ::after element.

@XhmikosR
Copy link
Member

XhmikosR commented Nov 11, 2018 via email

@midzer midzer mentioned this pull request Nov 11, 2018
@mdo
Copy link
Member Author

mdo commented Nov 12, 2018

Thanks @XhmikosR, I think this is good to go now.

@XhmikosR XhmikosR merged commit dafb498 into v4-dev Nov 13, 2018
@XhmikosR XhmikosR deleted the spinner branch November 13, 2018 06:22
@twbs twbs deleted a comment Dec 21, 2018
@MamaikAn
Copy link

How about creating spinner-style progress bar? Something like this.
bootstrap

@ahtee
Copy link

ahtee commented Dec 23, 2019

How about creating spinner-style progress bar? Something like this.
bootstrap

This could probably be under a progress component feature instead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.