Skip to content

Fixes issue #1120 by removing display: none from aria-hidden, doesn't…#1153

Closed
ducklet wants to merge 1 commit into
uswds:18f-pages-stagingfrom
ducklet:fix-aria-hidden
Closed

Fixes issue #1120 by removing display: none from aria-hidden, doesn't…#1153
ducklet wants to merge 1 commit into
uswds:18f-pages-stagingfrom
ducklet:fix-aria-hidden

Conversation

@ducklet

@ducklet ducklet commented May 9, 2016

Copy link
Copy Markdown

… impact anything but accordion component which should be addressed separately (see #1121, #1122, et al).

Regardless of how the accordion issue is solved, it is not appropriate for aria-hidden=true to have additional rules; instead, a class with display: none should be used where needed.

Thanks!

… impact anything but accordion component which should be addressed separately
@maya

maya commented May 9, 2016

Copy link
Copy Markdown
Contributor

Hey @ducklet! Thanks so much for submitting this. In order for us to not break the accordion component, we'll need to fix that before adding this in.

@ducklet ducklet mentioned this pull request May 10, 2016
@joshbruce

Copy link
Copy Markdown

@ducklet - Per the request on #1137 - I'm curious, as I don't know if it's possible.

  • Can I check out this branch and update this pull request directly?
  • Or, do I need to create another PR entirely?

I would prefer the former as it would be easiest. I've done some very brief Google-fu and saw that it was a feature request, but not finding instructions on doing it.

Maybe @maya knows?

@maya

maya commented May 10, 2016

Copy link
Copy Markdown
Contributor

@joshbruce opening a new PR would be best! I'm going to close this out since it doesn't have what we need in order to accept it, i.e. we can't fix one thing and break another.

But we agree with the intent of this 💯 .

@maya maya closed this May 10, 2016
@ducklet ducklet deleted the fix-aria-hidden branch May 11, 2016 15:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants