WIP Jenweber a11y updates #121
Conversation
simpler input remove console.logs remove another console log fix role for options couple of more improvements
…ithub.com/toddjordan/super-rentals into toddjordan-global-accessibility-awareness-day
|
addressing #86 |
| @@ -1 +1 @@ | |||
| {{yield}} | |||
| <div class="map-container" aria-hidden="true" tabindex="-1"></div> | |||
There was a problem hiding this comment.
@jenweber question about this: we are also assigning the "map-container" class to the parent element (see location-map.js) and actually assigning the map element to the components base element. I don't understand why we need a second div with the "map-container" class. Should we rather assign the aria-hidden and tabindex attributes to the component element itself?
There was a problem hiding this comment.
Whichever you think is best! It was probably just an oversight to have duplicates.
app/components/location-map.js
Outdated
| @@ -3,6 +3,10 @@ import Component from '@ember/component'; | |||
|
|
|||
| export default Component.extend({ | |||
| classNames: ['map-container'], | |||
| attributeBindings: ['aria-hidden:ariaHidden', 'tabindex'], | |||
There was a problem hiding this comment.
For the sake of the tutorial I'm wondering if I should just add aria-hidden and tabindex as attributes to the component invocation. I might be easier to explain then introducing tutorial users to attributeBindings.
ex. {{location-map aria-hidden="true" tabindex="-1"}}
What do you think @jenweber
| @@ -23,6 +23,12 @@ module.exports = function(environment) { | |||
| } | |||
| }; | |||
|
|
|||
| ENV['ember-a11y-testing'] = { | |||
| componentOptions: { | |||
| turnAuditOff: true, // Change to run a11y audit on all dev build and serves | |||
There was a problem hiding this comment.
should we turn on instead of adding the explicit test cases. I don't have super intimate knowledge of the addon so unsure if its an equal substitute.
toddjordan
force-pushed
the
jenweber-a11y-updates
branch
from
8bec92e to
632506d
Compare
toddjordan
force-pushed
the
jenweber-a11y-updates
branch
from
7540442 to
e833cb5
Compare
toddjordan
force-pushed
the
jenweber-a11y-updates
branch
from
e833cb5 to
bbe4848
Compare
|
@toddjordan @jenweber I was wondering if this pull request can be closed now. Would this be okay with you? |
|
👍 I think we are good! If anything from here is still needed on the latest version let’s reopen this against ember-learn/super-rentals-tutorial |
@jenweber here's my updates that get things working + conflict resolution. Ready to start guides updates