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

Improve reduced-motion case for spinners #30081

Closed
wants to merge 3 commits into from
Closed

Improve reduced-motion case for spinners #30081

wants to merge 3 commits into from

Conversation

ffoodd
Copy link
Member

@ffoodd ffoodd commented Jan 24, 2020

Last but not least, this is a real feature request and not another docs enhancement.

Basically, it ensures that spinners honor the prefers-reduced-motion feature by stopping the animation — which kind of defeat the need for a visual spinner. The idea I came up with is to display the visually hidden text instead.

So it then litterally displays the Loading… text instead of the animated thing.

@XhmikosR XhmikosR added the css label Jan 25, 2020
@MartijnCuppens
Copy link
Member

I don't really like to see the .sr-only is overridden, also .sr-only-focussable should be taken into account. On the other hand this construction makes it hard to theme the .sr-only element, even with utilities (specificity of 1,0,2,0 vs 1,0,1,0)

Also, I doubt this is really needed, quoting @patrickhlauke (#27986 (comment)):

I'd note that reduced motion is primarily about stuff that can cause trouble (read: feelings of nausea etc) to users with vestibular disorders, so mainly animated scrolling effect, accordion-type panels that animate open/close, etc.

The spinner animation is rather subtle.

@ffoodd
Copy link
Member Author

ffoodd commented Jan 27, 2020

I understand, and globally agree with your arguments.
This will probably land in our fork so I preferred to suggest this directly here :)

Let me know if it still sounds interesting and needs a rework, though.

@MartijnCuppens
Copy link
Member

Gonna close this for the reasons I mentioned above. Thanks though

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

Successfully merging this pull request may close these issues.

3 participants