search

18F / epa-notice

Web interface for viewing and commenting on proposed changes to federal regulations
Other
7 stars 17 forks source link

Update comment form markup. #278

Closed jmcarp closed 8 years ago

jmcarp commented 8 years ago

Goes with https://github.com/eregs/regulations-site/pull/343.

cmc333333 commented 8 years ago

gov_agency_text isn't hooked up to anything, right? It's not one of the regs.gov fields

jmcarp commented 8 years ago

Oh right. At least one of the inputs should probably have an id for so we can refer to it from the <label>, right?

cmc333333 commented 8 years ago

Sorry, I misunderstood the interplay between the fields, being disabled, and how the server would respond.

jmcarp commented 8 years ago

Anyway, the ids aren't right as they are--do you know what's best for accessibility here? We can keep the distinct ids and have for point to the input that's visible by default--but then do we want to change for in the js?

cmc333333 commented 8 years ago

It took me some time to understand, but having the selector default to disabled makes this work pretty neatly for non-JS users. Very smart!

Updating the for attribute in JS would be the best experience, but I worry that we're already in a pretty messy situation there. I wonder if we can use [aria-labelledby](https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/ARIA_Techniques/Using_the_aria-labelledby_attribute) and reference all of the potential labels. I don't know how screen readers would react to hidden labels. Maybe take a quick swing a JS implementation and if it look too messy, fall back to aria-labelledby?

I'm going to merge this now and we can follow up with a more accessible solution.