Fix aria warnings in Chrome lighthouse, make aria spec compliant #197
Conversation
|
Hi @mattywong, Thanks for the PR. It looks like there's a fair number of unit tests that broke with this, though. Check the TravisCI job for the full list. I don't think this can be merged with that many. Could you work on those and update this PR? Also, you'll notice the files in |
src/typeahead/typeahead.js
Outdated
| @@ -333,6 +333,7 @@ var Typeahead = (function() { | |||
|
|
|||
| open: function open() { | |||
| if (!this.isOpen() && !this.eventBus.before('open')) { | |||
| this.input.$input.attr("aria-expanded", true); | |||
There was a problem hiding this comment.
I have noticed your tests are broken because $input is not available in this context, if possible create the function bellow at input.js and call it here to set aria-expanded. This will fix all the broken tests.
setAriaExpanded: function setAriaExpanded(value) {
this.$input.attr('aria-expanded', value);
},
src/typeahead/typeahead.js
Outdated
| @@ -343,6 +344,7 @@ var Typeahead = (function() { | |||
|
|
|||
| close: function close() { | |||
| if (this.isOpen() && !this.eventBus.before('close')) { | |||
| this.input.$input.attr("aria-expanded", false); | |||
There was a problem hiding this comment.
I have noticed your tests are broken because $input is not available in this context, if possible create the function bellow at input.js and call it here to set aria-expanded. This will fix all the broken tests.
setAriaExpanded: function setAriaExpanded(value) {
this.$input.attr('aria-expanded', value);
},
src/typeahead/typeahead.js
Outdated
| @@ -333,6 +333,7 @@ var Typeahead = (function() { | |||
|
|
|||
| open: function open() { | |||
| if (!this.isOpen() && !this.eventBus.before('open')) { | |||
| this.input.setAriaExpanded(false); | |||
There was a problem hiding this comment.
I think you should set to true here.
|
I think you have merged Also, it seems your updates are not in |
|
Hey @mattywong, I have created a new Pull Request (#203) to make some tests and I figured out that we just have to upgrade Node version in travis.yml to solve this. I have tested with v10.18.0 and it was just fine, can you apply this change? |
package.json
Outdated
| @@ -71,6 +71,6 @@ | |||
| "scripts": { | |||
| "test": "bower install && node ./node_modules/karma/bin/karma start --single-run --browsers PhantomJS" | |||
| }, | |||
| "version": "1.3.0", | |||
| "version": "1.2.3", | |||
There was a problem hiding this comment.
Can you update to the next minor and regenerate build files, please?
| "version": "1.2.3", | |
| "version": "1.3.1", |
package.json
Outdated
| "main": "dist/typeahead.bundle.js" | ||
| } | ||
| } |
There was a problem hiding this comment.
| } | |
| } | |
|
@jlbooker I think that we can merge this one when @mattywong resolve my last review. Is that possible? |
Co-Authored-By: Juliano Marques Nunes <[email protected]>
|
This has been released as part of v1.3.1. |
closes #195 + more additional fixes to get this lighthouse/spec compliant
aria-readonly attr on input has been removed - Not sure why it was added in the first place? According to w3 aria spec (http://w3c.github.io/html-aria/#index-aria-combobox) readonly and aria-readonly properties can't be used when role="combobox". This aria-readonly attr also shows an error in lighthouse aria validation.
remove initial empty aria-activedescendant (aria-activedescendant="") - This seems to go against the spec also (see w3c/aria#501 and validator/validator#557) and also shows an error in lighthouse aria validation.
add correct aria-owns id reference, and if no id is present on input, generate a guid from helpers and attach to menu - this fixes the issue outlined in #195, the previous change for this (line 45) never worked as it was referencing 'www.menu' which never exists, and even with the correct reference 'www.selectors.menu' also wouldn't have worked as it would've just changed all the menus ('.tt-menu') the the last input id that was initialised