hoodiehq / faq

Frequently asked questions about Hoodie
faq.hood.ie
11 stars 3 forks source link

fix(html5/accessibility): add aria roles to content divs #103

Closed varjmes closed 9 years ago

varjmes commented 9 years ago

This PR switches the <div>'s that hold the main page content to <main> elements for good HTML semantics. It also adds an aria role of main for accessibility purposes.

Does not seem to affect any visuals on the site, so should be save to merge!

gr2m commented 9 years ago

looks good to me :+1:

lewiscowper commented 9 years ago

Looks great. Our IE compatibility would suffer though - http://caniuse.com/#feat=html5semantic

None of the IEs support a <main> tag outright. Do we have a fallback strategy for IE/browser target list?

varjmes commented 9 years ago

Rookie mistake from me, I hadn't realised how far behind IE still was on these kinds of things.

http://stackoverflow.com/a/18088755/3512637 says we can do a few things. But I'm not sure if you want to take any of these approaches.

Not having this would be a shame, but accessible is also supporting all the browsers so we may have to leave this one :(

varjmes commented 9 years ago

http://weblog.west-wind.com/posts/2015/Jan/12/main-HTML5-Tag-not-working-in-Internet-Explorer-91011 is also useful. Maybe styling the main block would work for IE?

varjmes commented 9 years ago

So I realised that the main thing here is about accessibility, rather than my love of HTML semantics. To support both IE users and screenreader, I switched the <main>'s back to <div>s but left the aria role of main on them. Should be good now :)

boennemann commented 9 years ago

In my testing this PR breaks #101, because now the label is read on it's own, rather then when focusing on the search-field.

varjmes commented 9 years ago

Turns out this wasn't breaker #101 after all and can be merged. I have improved our search labelling separately in #110 :)