Skip to content

Fix aria-labelledby issues#789

Merged
jessieay merged 4 commits intomasterfrom
jy-more-508
Dec 1, 2016
Merged

Fix aria-labelledby issues#789
jessieay merged 4 commits intomasterfrom
jy-more-508

Conversation

@jessieay
Copy link
Contributor

Why:

  • 508 compliance
  • For radio buttons, was getting error:
    "Radio inputs with the same name attribute value must be part of a
    group"
  • Ref: https://dequeuniversity.com/rules/axe/2.0/radiogroup
  • For other form, there was an invalid aria attr (did not correspond
    to an id in the markup)

@jessieay
Copy link
Contributor Author

Questions on this:

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should this be a fieldsetor legend instead? see https://dequeuniversity.com/rules/axe/2.0/radiogroup?application=axeAPI

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there is no label-ssn id on the page, so this is an invalid aria-labelledby attr

**Why**:
* 508 compliance
* For radio buttons, was getting error:
  "Radio inputs with the same name attribute value must be part of a
  group"
* Ref: https://dequeuniversity.com/rules/axe/2.0/radiogroup
* For other form, there was an invalid aria attr (did not correspond
  to an `id` in the markup)
.sm-right-align { text-align: right; }
}

.sr-only {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added this per recs in https://snook.ca/archives/html_and_css/hiding-content-for-accessibility

is this the best place to put this? cc @brendansudol

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jessieay basscss already has a similar class - checkout accessible hide here: http://basscss.com/#basscss-hide

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are correct! I assumed hide was not accessible but that was not a good assumption.

Screenshot of the CSS that hide generates:

screen shot 2016-12-01 at 10 06 05 am

@jessieay
Copy link
Contributor Author

jessieay commented Dec 1, 2016

@hursey013 updated at 2f1cece

@jessieay
Copy link
Contributor Author

jessieay commented Dec 1, 2016

@hursey013 re-review?

Copy link
Contributor

@zachmargolis zachmargolis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! yay a11y

@jessieay jessieay merged commit 3494341 into master Dec 1, 2016
@jessieay jessieay deleted the jy-more-508 branch December 1, 2016 19:57
amoose pushed a commit that referenced this pull request Feb 24, 2017
**Why**:
* 508 compliance
* For radio buttons, was getting error:
  "Radio inputs with the same name attribute value must be part of a
  group"
* Ref: https://dequeuniversity.com/rules/axe/2.0/radiogroup
* For other form, there was an invalid aria attr (did not correspond
  to an `id` in the markup)
amoose pushed a commit that referenced this pull request Feb 28, 2017
**Why**:
* 508 compliance
* For radio buttons, was getting error:
  "Radio inputs with the same name attribute value must be part of a
  group"
* Ref: https://dequeuniversity.com/rules/axe/2.0/radiogroup
* For other form, there was an invalid aria attr (did not correspond
  to an `id` in the markup)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants