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

[4.0] which scss file? #19694

Closed
brianteeman opened this issue Feb 15, 2018 · 15 comments
Closed

[4.0] which scss file? #19694

brianteeman opened this issue Feb 15, 2018 · 15 comments

Comments

@brianteeman
Copy link
Contributor

brianteeman commented Feb 15, 2018

WCAG 2.1 introduces Success Criterion 2.2.9 Animation from Interactions (level AAA), it should be possible to disable the animation. Safari and ios have options that a user can set for this and we can support this with a media query for example.

Bootstrap has a class progress-bar-animated which we use in:

  • password strength meter
  • com_joomlaupdate
  • mod_sample_data
  • installation
  • maybe some other places like finder?

It is an easy fix to apply I just dont know which file to put it in so that it will be available in the installer, admin and front end templates. Advice please perhaps even a separate file for a11y hacks?

@media screen and (prefers-reduced-motion: reduce) {
  .progress-bar-animated {
    -webkit-animation: none;
    -moz-animation: none;
    /**
     * Some browsers do not recognise transition: none, so using
     * transition-duration instead for those browsers eg opera.
     */
    -webkit-transition: none;
    -o-transition: all 0 none;
    transition: none;
  }
@dgrammatiko
Copy link
Contributor

Same goes for bootstrap:

  • modals
  • tooltips
  • popovers
  • accordions
  • slider
  • tabs

Basically everything where bootstrap is using animation. I think this should be a PR upstream (the good part of using external library for our base css)...

@mbabker
Copy link
Contributor

mbabker commented Feb 15, 2018

+1 on fixing upstream. We should only patch if they reject it.

@dgrammatiko
Copy link
Contributor

And to add something more, since I spent some time reverse engineering Bootstrap js, if you patch the .close class to not have any animation if the os dictates that, then you have a very interesting effect in modals: they never close 😂. Same goes for some other components. This will be interesting for BS to implement, since the changes are affecting the js functionality!

@C-Lodder
Copy link
Member

C-Lodder commented Feb 15, 2018

I'm going to say due to the amount of animations that are part of or will be part of J4, that this should not be done. We dont support AAA so I do not want to justify the additional CSS.

There are browser extensions that those with disabilities can installed to apply for every site

@brianteeman
Copy link
Contributor Author

If we can support something (and I believe we can) then we should irrespective of it being A, AA or AAA

For now I will submit it upstream BUT if someone isnt using bootstrap there would still be an issue?

PS I remember seeing a doc about setting up and contributing to J4 scss etc but I cant find it

@brianteeman
Copy link
Contributor Author

@C-Lodder

There are browser extensions that those with disabilities can installed to apply for every site

Yes ios and safari have the motion stuff built in BUT they need the site to support it for it to work - thats what this change would do.

6 lines of scss is hardly a big amount of code to add in order to improve our accessibility.

@brianteeman
Copy link
Contributor Author

@MikeRogers0 of UK Gov Digital Services has already opened an issue upstream with bootstrap so hopefully it can/will be resolved upstream twbs/bootstrap#25249

I would like to keep this open until then

@MikeRogers0
Copy link

@brianteeman I've opened a PR for that issue now ( twbs/bootstrap#25641 ) 👍

Also, I don't work for UK Gov Digital Services - I just open issues / PRs to their codebase every now and then ;)

@brianteeman
Copy link
Contributor Author

@MikeRogers0 thanks I will test it

@brianteeman
Copy link
Contributor Author

Closing it here as it looks like it is going to be merged upstream

@dgrammatiko
Copy link
Contributor

@brianteeman just to let you know that all custom elements will implement this as well

@brianteeman
Copy link
Contributor Author

That's great. Awesome to see an issue start at a Drupal sprint and move to Joomla and bootstrap. This is what Opensource is all about.

@dgrammatiko
Copy link
Contributor

@fuzzbomb
Copy link

fuzzbomb commented Apr 24, 2018

@brianteeman

Advice please perhaps even a separate file for a11y hacks?

This wasn't answered yet. My view is it's better to keep accessibility as part of the main CSS. Don't relegate accessibility to a separate file, because...

  • Putting it in a separate file introduces a regression risk, where a CSS ruleset might be updated in the main stylesheet, but corresponding changes in the separate a11y file don't get carried out. Changes to class names are a particular risk. I've seen this happen before - if you don't follow up and change the class names in the separate a11y file, these rules get orphaned.
  • It encourages the mindset that accessibility is somehow an optional feature, a bolt-on patch, rather than an integral part of an inclusive design. You can hear this in the phrase "a11y hacks".
  • It's easier to understand what the code is doing, accessibility-wise, if it's presented alongside the rest of the style rules, with appropriate comments. Low awareness of accessibility is still a problem amongst developers generally, and keeping a11y-related code in the same file is a good way to promote it.
  • A precedent - I've never seen .fancy-button:focus styles relegated to a separate file. These generally accompany the non-interactive styles for .fancy-button. Why would prefers-reduced-motion be treated any differently?

This assumes it's code under your control, of course. In the case where Joomla is using Bootstrap, it makes sense to work on fixes upstream., and that entails following their policy on how code is organized.

I'm not sure about the part for making it available to the installer, because I don't know how Joomla's templates/themes work there. This might be an area where you end up with some duplicated code, but I'd guess that would be the case already, for installer CSS which isn't related to a11y...?

@C-Lodder,

I'm going to say due to the amount of animations that are part of or will be part of J4, that this should not be done. We dont support AAA so I do not want to justify the additional CSS.

"Should not be done" is very harsh to your users. Similar points have been made about supporting prefers-reduced-motion in Drupal, where the accessibility QA gate wants to satisfy level-AA. It's common to hear that something is level-AAA, so we don't need to address it. But just because something isn't a conformance issue for our chosen WCAG target, doesn't mean it isn't an issue for real users. The additional CSS to support prefers-reduced-motion is justified because it's quite easy to do, and it has a very high impact for affected users.

There are browser extensions that those with disabilities can installed to apply for every site

Which ones? I'm not aware of any extensions that let a user disable CSS animations easily. There are lots of browser extensions which stop animated GIFs, audio, and video. Extensions for managing user styles can do it, but you need to understand CSS.

@brianteeman
Copy link
Contributor Author

@fuzzbomb this specific issue was fixed upstream by bootstrap. Your other detailed comments are however greatly appreciated.

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

No branches or pull requests

7 participants