Skip to content
This repository was archived by the owner on Sep 5, 2024. It is now read-only.

Allow developers to disable attributes injected by ngAria #600

Closed
marcysutton opened this issue Nov 11, 2014 · 29 comments
Closed

Allow developers to disable attributes injected by ngAria #600

marcysutton opened this issue Nov 11, 2014 · 29 comments
Assignees
Labels
a11y This issue is related to accessibility
Milestone

Comments

@marcysutton
Copy link
Contributor

One nice feature of ngAria is that developers can disable specific attributes that conflict with their own implementation, for example, if they already manage tabindex. Because of our mix of ngAria and custom accessibility support, opting out of attributes managed both by us and by ngAria is not possible. For example, see these two code snippets:

var app = angular.module( 'app', ['ngAnimate','ngMaterial'], 
  function config($ariaProvider) {
      $ariaProvider.config({
        tabindex: false,
        ariaInvalid: false
   });
});
<md-radio-group ng-model="data.group1">
    <md-radio-button value="Apple" aria-label="Apple">Apple</md-radio-button>
    <md-radio-button value="Banana" aria-label="Banana">Banana</md-radio-button>
    <md-radio-button value="Mango" aria-label="Mango">Mango</md-radio-button>
</md-radio-group>

tabindex is added to <md-radio-group> by Angular Material so only aria-invalid is suppressed, not tabindex. ngAria leaves the existing tabindex intact, as it should.

As much as it seems like a bad idea to allow people to disable accessibility support for attributes, should we create a mechanism to support custom disabling of attributes in this way, since it is supported by ngAria?

Example on Plunkr.

Related: #583 (bug)

@marcysutton marcysutton added question a11y This issue is related to accessibility labels Nov 11, 2014
@ThomasBurleson
Copy link
Contributor

Can we not check the configuration of ngAria? Developers can use

$AriaProvider.config({ 'tabindex':false });

Then our $mdAria.expect( ) can get a reference to the configuration settings and internally check if that option is enabled.

var shouldCheckAriaAttribute =  $AriaProvider.$get().config;

@ThomasBurleson ThomasBurleson added this to the 0.7.0-rc1 milestone Nov 13, 2014
@marcysutton marcysutton self-assigned this Dec 4, 2014
@marcysutton marcysutton modified the milestones: 0.8.0-rc1, 0.7.0-rc1 Dec 11, 2014
@jarrodpayne
Copy link
Contributor

@marcysutton good find. I'm experiencing this issue now. 👍 in hopes that it doesn't get pushed back further than 0.8.0.

@marcysutton
Copy link
Contributor Author

@ThomasBurleson we don't actually expose the $ariaProvider in ngMaterial, we only pull in ngAria as a dependency (https://github.com/angular/material/blob/master/scripts/gulp-utils.js#L151). It's an unusual setup, so I can't quite figure out how to expose it. I'd love your input.

https://docs.angularjs.org/api/ngAria/provider/$ariaProvider

@ThomasBurleson
Copy link
Contributor

@marcysutton - I have some ideas. Let's talk after ng-conf.

@jarrodpayne
Copy link
Contributor

@ThomasBurleson @marcysutton Did you all have a chance to talk about this? :)

@marcysutton marcysutton modified the milestones: 0.10.0, 0.9.0 Apr 2, 2015
@marcysutton
Copy link
Contributor Author

@jpdesigndev can you elaborate on what issues you ran into? We build our components to be standalone-accessible, occasionally leveraging ngAria. I'm actually not so sure now that you should be able to override that.

@marcysutton
Copy link
Contributor Author

I don't think this is a good idea anymore. If it comes up again and we can make a good case for it, we'll reopen.

@jarrodpayne
Copy link
Contributor

@marcysutton Sorry, I missed the previous comment notification. So, I'm using ui-select which works fine without ngAria. With ngAria, tabindex is automatically added to a nested DOM node where it shouldn't therefore going into or out of the control requires an extra tab. I'm going off the top of my head here. It's been a while since I applied a workaround.

Why do you think this is no longer a good idea? Having a bit of control to override sensible defaults seems like a good idea to me.

@marcysutton
Copy link
Contributor Author

@jpdesigndev a demo would help, so I can dig in and look at the actual issue. What we don't want to enable is inaccessibility of the Angular Material components, which leverage ngAria–those instances of tabindex need to stay.

Tabindex dynamically added outside of components we control (like in a 3rd-party component) sounds like a byproduct of ngAria consumed in Angular Material. However, we do not support 3rd-party components. I'd recommend switching to the Angular Material select.

@benshope
Copy link

benshope commented Jun 5, 2015

Is it possible to remove ngAria from ngMaterial completely?

The extra bower requirement is a slightly annoying - and now ngAria appears to be printing warnings to the console - which is quite annoying.

Sorry if there is an easy "off" switch that I am missing.

@smashercosmo
Copy link

Yep, +1 for possibility to use angular.material without ngAria. In our app we don't need any accessibility stuff at all.

@marcysutton
Copy link
Contributor Author

No, ngAria is a required dependency. But it isn't printing warnings–Angular Material is. If you are seeing warnings in the console, you need to add aria-label attributes to your components. This is non-negotiable. It should at least point you to the specific nodes in question to help you locate the problem. If there are bugs with the ngMaterial logging utility, though, we want to know.

@patrickhlauke
Copy link

In our app we don't need any accessibility stuff at all.

"Our restaurant is up a steep flight of stairs...so we don't need a wheelchair-accessible toilet"

@benshope
Copy link

benshope commented Jun 5, 2015

I totally understand and respect that ngAria is included and active by default. But it would be really nice if there were a less-publicized fork of ngMaterial without ngAria, released by those who maintain ngMaterial. At the very least there should be a way to switch off all warnings in the app.config() (although... I'm not sure - is there?)

For example, my app does nothing but show visuals of data using d3.js. It is inherently inaccessible to people with impaired vision.

@patrickhlauke - If I were the owner of a restaurant - I would hope to be encouraged to choose accessibility, but allowed to make a pragmatic decision. Software developers are a nice bunch; everyone wants to do the right thing.

@marcysutton
Copy link
Contributor Author

@benshope if we added an option to disable warnings, it would effectively enable developers to create inaccessible web applications. It's just not something we are going to do. What we can do, however, is look at specific use cases where the warnings don't make sense. Can you provide a Codepen or Plunker that shows your use case?

There has to be a way to enhance your data visualization for visually impaired users, rather than completely throwing them under the bus. This is actually an opportunity to figure out what the best practices could be.

@benshope
Copy link

benshope commented Jun 5, 2015

@marcysutton - I am looking to remove tags like aria-label="Home" from my main menu.

Adding screen-reader accessibility to this app (even to the main menu) would be pointless because the data is already in it's most accessible format, as tables of data, on another website.

Perhaps the ideal behavior in my situation would be to have <body aria-redirect="Please visit this other website"> and then any content within aria-redirect would not throw an accessibility-related error.
The aria-redirect solution has several benefits:

  • People with screen readers would not experience confusion when hitting my site
  • Developers would be able to gracefully handle situations where all or part of their app is inaccessible
  • Developers would not have to write unnecessary lines of code
  • Developers would still be encouraged to make accessible apps (and forced to write an excuse when they do not)

I realize it is unlikely the idea of an aria-redirect will gain traction in this forum, but I will put it forward anyways.

@marcysutton
Copy link
Contributor Author

Shipping users off to another website is your decision, but it's not something we're going to recommend as a framework. Just add the labels and then you won't get the warnings...You can provide a text link on the page for people who want the other version. I'm still not convinced why you would advocate removing accessibility for an entire app when only part of it is inaccessible-a better approach would be to tell people about the alternative content somewhere, even a visually hidden link.

@MylesBorins
Copy link

Please let me know if I am missing something. Is the problem that you don't want to do a little extra work to avoid errors... and the extra bower dependency is giving you the jibbilies?

@benshope I hope you realize how entitled you are coming off here.

If you want a version without aria, which might i add goes completely against the fundamental ideology of the project, there is absolutely nothing stopping you from forking the project and making the changes yourself. Why you think the project maintainers would do this for you is beyond me.

The downside to this... all the time you will have to spend maintaining these changes and keeping things up to date. I wonder if just avoiding the errors to begin with would take less time.

@benshope
Copy link

benshope commented Jun 5, 2015

@marcysutton Perhaps a better name would be aria-inaccessible="My excuse for the situation". I am not advocating that the framework encourage magical redirects. Just that it allow totally inaccessible apps and parts-of-apps.

@thealphanerd Sorry, if I am coming across as insensitive. But yes, I feel strongly that the framework should take a stance that encourages, but does not require, insertion of unsemantic boilerplate code into an otherwise-beautiful and concise ui framework. I would fork the project, but what you said is absolutely correct - maintaining it would be a PITA.

A framework should be absolute in some ways, opinionated in some ways, and agnostic in some ways. Aria has been placed in the wrong basket.

Also, not meaning to argue, just to voice an opinion. I know sometimes I come across a bit strong 😄

@MylesBorins
Copy link

The argument that aria should be a nice to have is inherently flawed and files you in the noise category as far as I'm concerned

@patrickhlauke
Copy link

Perhaps a better name would be

before we try to bikeshed any more aria properties, might be worth pointing out that it's an actual standard, so not just something that can be made up... http://www.w3.org/TR/wai-aria/

unsemantic boilerplate code

the irony here is that adding aria is actually infusing the unsemantic tagsoup that's generally spat out by angular with some semblance of semantics that can then get conveyed to assistive technologies...

@ryanflorence
Copy link

If you are okay with doing something terrible like dismissing accessibility, then just console.warn = function() {} and go to bed at night feeling bad for at least something :P

@dylanb
Copy link

dylanb commented Jun 6, 2015

@benshope every single application can be made accessible, failure to want to do so is pure laziness. It is the sign of an inferior developer.. Do you also advocate for removing security features from frameworks? Perhaps we should remove the unicode support from JavaScript too. ASCII characters are so much more memory-friendly! Hey, I even have this Windows application that supports unicode, so keep those useless extra characters away from my web site!

@ryanflorence
Copy link

A framework should be absolute in some ways, opinionated in some ways, and agnostic in some ways. Aria has been placed in the wrong basket.

Material is a UI framework. UI does not mean "Visual Design" but "User Interface". The interface for some folks is how it sounds. So, making your UI not just look beautiful, but also sound beautiful is absolutely, without question, in the right basket.

Sorry, if I am coming across as insensitive

I wouldn't call it insensitive but ignorant to many users of the web. Use this thread and framework as an opportunity to level up on your craft!

At a previous job one of our front-end devs was completely blind. We'd get steaming piles of crap HR apps that he couldn't use. ngAria is doing you an enormous favor, you barely have to learn anything to not create another warm, steamy pile.

Level up! http://www.w3.org/TR/wai-aria-practices/

@benshope
Copy link

benshope commented Jun 6, 2015

@ryanflorence - Thank you! console.warn = function() {} is an admittedly-shady way to not-see console errors for the time being. I will very likely use ngAria eventually - what you say makes sense 👍

@dylanb I just like to write as few lines as humanly possible - and I don't think that should be a contentious philosophy. If my app does not need security features, and these features require me to write more code, then a good framework will allow me to disable them. Things like security, test coverage, and accessibility are all very nice, and very non-critical in certain situations. Writing less code is a gesture of respect towards other developers who will eventually read your code.

@ctoth
Copy link

ctoth commented Jun 6, 2015

@benshope It may be a gesture of respect to other developers reading your code (though I don't quite see how this could be), but it's a gesture of a totally different kind to the blind guy like me who comes across your inaccessible turd later. A gesture a lot more akin to an upraised middle finger.

@marcysutton
Copy link
Contributor Author

I would like to remind everyone of our Code of Conduct, which specifically forbids unprofessional behavior in communication channels. https://github.com/angular/code-of-conduct/blob/master/CODE_OF_CONDUCT.md

That said, @benshope, I can't believe you would seriously consider removing console output instead of just adding accessibility labels. If I had the ability to lock this issue, I would, as it has devolved into something that is not productive. What you are asking us to support goes against a core philosophy of the Angular Material project–the framework is intended to support a large user base, not a discriminatory subset. We are not going to disable accessibility features just so that you can write less lines of code. End of story.

@EladBezalel
Copy link
Member

This is absurd.. There are projects that don't need aria support.. You should supply a way to disable it and warn that this is disabled..

Adding additional attributes to large amount of elements is extra bytes that I can't allow to have in my project..

@yatil
Copy link

yatil commented Jun 28, 2015

@EladBezalel: Are your projects used by humans? If they do, they need ARIA support.

@angular angular locked and limited conversation to collaborators Jun 28, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
a11y This issue is related to accessibility
Projects
None yet
Development

No branches or pull requests