Skip to content

Chore(DescriptionList): added unique aria-labels#11646

Merged
nicolethoen merged 5 commits intopatternfly:mainfrom
Mash707:add-aria-labels-description-list
Apr 21, 2025
Merged

Chore(DescriptionList): added unique aria-labels#11646
nicolethoen merged 5 commits intopatternfly:mainfrom
Mash707:add-aria-labels-description-list

Conversation

@Mash707
Copy link
Copy Markdown
Contributor

@Mash707 Mash707 commented Mar 11, 2025

Closes #11392

@Mash707
Copy link
Copy Markdown
Contributor Author

Mash707 commented Mar 11, 2025

Need to add aria-label to demos as well.

@patternfly-build
Copy link
Copy Markdown
Collaborator

patternfly-build commented Mar 11, 2025 •
edited
Loading

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

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

@Mash707
Copy link
Copy Markdown
Contributor Author

Mash707 commented Mar 11, 2025 •
edited
Loading

@thatblindgeye have a look once, I have added aria-labels to all DescriptionList components. Let me know I miss something.
I would also like to point out two things:

@thatblindgeye
Copy link
Copy Markdown
Contributor

thatblindgeye commented Mar 12, 2025

The "in drawer" demo should get published once our next release goes live, but thanks for mentioning that - had to double check it was showing up on our staging site.

For the Wizard bug, would you mind opening a bug report for that example?

@Mash707
Copy link
Copy Markdown
Contributor Author

Mash707 commented Mar 12, 2025

For the Wizard bug, would you mind opening a bug report for that example?

Sure , will open a issue.

thatblindgeye
thatblindgeye reviewed Mar 19, 2025
View reviewed changes
Copy link
Copy Markdown
Contributor

@thatblindgeye thatblindgeye left a comment

Choose a reason for hiding this comment

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

Lot of comments below, just tweaking labels a bit

Comment thread packages/react-core/src/components/DescriptionList/examples/DescriptionListBasic.tsx Outdated
Comment thread packages/react-core/src/components/DescriptionList/examples/DescriptionListColumnFill.tsx Outdated
Comment thread packages/react-core/src/components/DescriptionList/examples/DescriptionListCompact.tsx Outdated
Comment thread ...ages/react-core/src/components/DescriptionList/examples/DescriptionListCompactHorizontal.tsx Outdated
Comment thread ...act-core/src/components/DescriptionList/examples/DescriptionListDefaultResponsiveColumns.tsx Outdated
Comment thread packages/react-core/src/demos/DescriptionList/examples/DescriptionListInDrawer.tsx Outdated
Comment thread packages/react-core/src/demos/examples/Card/CardEventsView.tsx Outdated
Comment thread packages/react-core/src/demos/examples/Card/CardStatusTabbed.tsx Outdated

const tabContent = (
<DescriptionList isHorizontal columnModifier={{ lg: '2Col' }}>
<DescriptionList isHorizontal columnModifier={{ lg: '2Col' }} aria-label="Status list">
Copy link
Copy Markdown
Contributor

@thatblindgeye thatblindgeye Mar 19, 2025

Choose a reason for hiding this comment

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

Can we make this dynamic based on the activeTabKey state? So the end result is when the Object 1 tab is active, the aria-label is Object 1 status, when Object 2 is active Object 2 status and so on.

Copy link
Copy Markdown
Contributor Author

@Mash707 Mash707 Mar 19, 2025

Choose a reason for hiding this comment

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

Passing tabIndex as a parameter in tabContent would make this dynamic.

Comment thread ...tion/demo-app-ts/src/components/demos/DescriptionListDemo/DescriptionListBreakpointsDemo.tsx Outdated
Comment thread ...act-integration/demo-app-ts/src/components/demos/DescriptionListDemo/DescriptionListDemo.tsx Outdated
@Mash707 Mash707 force-pushed the add-aria-labels-description-list branch from d60ac8a to a1e76a6 Compare March 19, 2025 21:24
@Mash707
Copy link
Copy Markdown
Contributor Author

Mash707 commented Mar 19, 2025

I have addressed all the suggestions. Let me know if I miss something.

@Mash707 Mash707 requested a review from thatblindgeye March 19, 2025 21:38
thatblindgeye
thatblindgeye approved these changes Apr 8, 2025
View reviewed changes
Copy link
Copy Markdown
Contributor

@thatblindgeye thatblindgeye left a comment

Choose a reason for hiding this comment

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

Awesome job on this 🎉

@nicolethoen nicolethoen merged commit 14a7622 into patternfly:main Apr 21, 2025
13 checks passed
@Mash707 Mash707 deleted the add-aria-labels-description-list branch April 21, 2025 17:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@nicolethoen nicolethoen nicolethoen approved these changes

@thatblindgeye thatblindgeye thatblindgeye approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

DescriptionList - lists should have unique aria-labels

4 participants

@Mash707 @patternfly-build @thatblindgeye @nicolethoen