Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(docs): Fix a11y issues in Documentation #2634

Merged
merged 6 commits into from Aug 9, 2019

Conversation

@jessiehuff
Copy link
Contributor

jessiehuff commented Aug 2, 2019

Fix accessibility issues in Accordion, ClipboardCopy, and DataList

Fixes patternfly/patternfly-org#1311
Fixes patternfly/patternfly-org#1310
Fixes patternfly/patternfly-org#1305

@patternfly-build

This comment has been minimized.

Copy link
Contributor

patternfly-build commented Aug 2, 2019

PatternFly-React preview: https://patternfly-react-pr-2634.surge.sh

Jessie
@tlabaj tlabaj requested review from redallen, jschuler and jgiardino Aug 6, 2019
Copy link
Contributor

jenny-s51 left a comment

Hi Jessie, your changes are looking good! There's a small thing that axe doesn't seem to like about the id of the first clipboard copy example, since it matches the title ... Looks like a small fix so I was going to try to find it myself and add it to your PR, but I looked around all three patternfly repos and I'm not sure where the ID for the first example is declared. Do you have any idea where this might be happening?

Screen Shot 2019-08-07 at 11 29 05 AM

@jessiehuff

This comment has been minimized.

Copy link
Contributor Author

jessiehuff commented Aug 8, 2019

Hey Jenny, thanks for taking a look! So from what I can tell, I believe in org we have AutoLinkHeaders that set the id. So in the mdxPF4Template.js and index.js there is an AutoLinkHeader that generates the size, className, id, content, etc. It does this from the AutoLinkHeader.js file. For the other PR I had on unique org ids, I had to pass a suffix to the AutoLinkHeader to add that to the id and differentiate them. With this issue, it looked like it was with what was being pulled from patternfly-react so I changed the aria-controls in ClipboardCopyToggle.tsx. That's been my thinking up until this point anyway. :)

@jenny-s51

This comment has been minimized.

Copy link
Contributor

jenny-s51 commented Aug 8, 2019

@jessiehuff That makes sense. Thank you for clarifying! Just out of curiosity (and for future reference), is aria-controls somehow modifying the value of the id there?

@jenny-s51 jenny-s51 self-requested a review Aug 8, 2019
@jessiehuff jessiehuff force-pushed the jessiehuff:fix/accessibilityDocs branch from 22f7a4e to a7530fb Aug 8, 2019
Jessie
@jessiehuff

This comment has been minimized.

Copy link
Contributor Author

jessiehuff commented Aug 8, 2019

@jenny-s51 aria-controls provide a relation between a parent element and a child element (the toggle's button in this case). The reason I think it needed to be changed here is because otherwise we get the issue: "ARIA attributes must conform to valid values" basically pointing out that we need to pass the correct information to screen readers. This article seems to give a good summary of aria-controls. :)

@jschuler jschuler merged commit b0d29a3 into patternfly:master Aug 9, 2019
8 checks passed
8 checks passed
ci/circleci: build Your tests passed on CircleCI!
Details
ci/circleci: build_integration Your tests passed on CircleCI!
Details
ci/circleci: build_pf3_docs Your tests passed on CircleCI!
Details
ci/circleci: build_pf4_docs Your tests passed on CircleCI!
Details
ci/circleci: lint Your tests passed on CircleCI!
Details
ci/circleci: test_jest_other Your tests passed on CircleCI!
Details
ci/circleci: test_jest_pf4 Your tests passed on CircleCI!
Details
ci/circleci: upload_docs Your tests passed on CircleCI!
Details
@patternfly-build

This comment has been minimized.

Copy link
Contributor

patternfly-build commented Aug 9, 2019

Your changes have been released in:

  • @patternfly/react-core@3.84.1
  • @patternfly/react-docs@4.9.27
  • @patternfly/react-inline-edit-extension@2.9.72
  • demo-app-ts@2.17.1
  • @patternfly/react-table@2.16.11
  • @patternfly/react-topology@2.7.20
  • @patternfly/react-virtualized-extension@1.1.106

Thanks for your contribution! 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.