Skip to content

chore(page): updated examples to provide a11y labels #11740

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 8 commits into from
May 7, 2025
Merged

chore(page): updated examples to provide a11y labels #11740

merged 8 commits into from
May 7, 2025

Conversation

Mash707
Copy link
Contributor

@Mash707 Mash707 commented Apr 3, 2025

Closes #10173

@patternfly-build
Copy link
Contributor

patternfly-build commented Apr 3, 2025 •
edited
Loading

Preview: https://patternfly-react-pr-11740.surge.sh

A11y report: https://patternfly-react-pr-11740-a11y.surge.sh

@Mash707
Copy link
Contributor Author

Mash707 commented Apr 3, 2025

I have added a11y labels to only the page examples but the PageSection component is being used in more than 100 places. Should I update them too in this PR? Could become difficult to review.

@Mash707
Copy link
Contributor Author

Mash707 commented Apr 4, 2025

I'll create a new PR to update rest of the examples and link it to this one. Will make it easier to review.

@Mash707 Mash707 mentioned this pull request Apr 4, 2025
Merged
@nicolethoen nicolethoen self-requested a review April 21, 2025 14:39
@Mash707 Mash707 force-pushed the page-update-examples-to-provide-a11y-labels branch from eb837cb to 2009a4f Compare April 21, 2025 20:37
@Mash707
Copy link
Contributor Author

Mash707 commented Apr 21, 2025

@thatblindgeye made the necessary changes in reference to your comments.
One thing I like to mention is that many examples are currently using tags like h2 , p . We might consider replacing them with Title and Content. It is out of scope for this PR. Let me know what you think.

@Mash707 Mash707 requested a review from thatblindgeye April 21, 2025 20:54
@thatblindgeye
Copy link
Contributor

One thing I like to mention is that many examples are currently using tags like h2 , p . We might consider replacing them with Title and Content. It is out of scope for this PR. Let me know what you think.

Yeah definitely out of scope of this PR. May not be a bad thing to make updates for, but not an urgent thing to be tackled either.

thatblindgeye
thatblindgeye requested changes Apr 28, 2025
View reviewed changes
packages/react-core/src/demos/Banner.md Outdated
<p>
Body text should be Red Hat Text at 1rem(16px). It should have leading of 1.5rem(24px) because <br />
of it’s relative line height of 1.5.
</p>
</Content>
</PageSection>
<PageSection>
<PageSection aria-label = 'Cards gallery'>
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: can we just update cases of the whitepspace around the = and remove them? aria-label=' instead and such

@Mash707 Mash707 force-pushed the page-update-examples-to-provide-a11y-labels branch from fe4119b to 41da6ff Compare April 28, 2025 19:08
@Mash707 Mash707 requested a review from thatblindgeye April 28, 2025 20:00
rebeccaalpert
rebeccaalpert approved these changes May 7, 2025
View reviewed changes
Copy link
Member

@rebeccaalpert rebeccaalpert left a comment

Choose a reason for hiding this comment

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

This looks good to me.

@rebeccaalpert rebeccaalpert merged commit 61767ab into patternfly:main May 7, 2025
13 checks passed
@patternfly-build
Copy link
Contributor

Your changes have been released in:

  • @patternfly/react-code-editor@6.3.0-prerelease.7
  • @patternfly/react-core@6.3.0-prerelease.7
  • @patternfly/react-docs@7.3.0-prerelease.8
  • @patternfly/react-drag-drop@6.3.0-prerelease.7
  • demo-app-ts@6.0.0-prerelease.110
  • @patternfly/react-table@6.3.0-prerelease.7
  • @patternfly/react-templates@6.3.0-prerelease.7

Thanks for your contribution! 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rebeccaalpert rebeccaalpert rebeccaalpert approved these changes

@thatblindgeye thatblindgeye thatblindgeye approved these changes

@nicolethoen nicolethoen Awaiting requested review from nicolethoen

Assignees
No one assigned
Labels
A11y
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Page - update examples to provide aria-label/labelledby to page sections
5 participants
@Mash707 @patternfly-build @thatblindgeye @rebeccaalpert @nicolethoen