Skip to content

chore(expandable section): add missing aria attributes to examples - #7754

Merged
thatblindgeye merged 2 commits into
patternfly:mainfrom
Mash707:expandable-section-examples-missing-aria-attributes
Sep 9, 2025
Merged

thatblindgeye merged 2 commits into
patternfly:mainfrom
Mash707:expandable-section-examples-missing-aria-attributes

Conversation

@Mash707

@Mash707 Mash707 commented Aug 22, 2025

Copy link
Copy Markdown
Contributor

Fixes #7280

@patternfly-build

patternfly-build commented Aug 22, 2025 •

Copy link
Copy Markdown
Collaborator

@@ -7,6 +7,7 @@
button--IsInline=expandable-section--IsTruncate
button--IsAriaExpanded=expandable-section--IsExpanded
button--aria-controls=(ternary expandable-section--IsDetached (concat expandable-section--id '-content') null)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This should be updated to render the aria-controls properly. Right now it's only rendering for detached variants, but we want it regardless of variant as long as an ID is passed.

Suggested change
button--aria-controls=(ternary expandable-section--IsDetached (concat expandable-section--id '-content') null)
button--aria-controls=(ternary expandable-section--id (concat expandable-section--id '-content') null)

@@ -7,6 +7,7 @@
button--IsInline=expandable-section--IsTruncate
button--IsAriaExpanded=expandable-section--IsExpanded

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@mcoker are you opposed to replacing instances of button--IsAriaExpanded in Core handlebars files with just button--IsExpanded? Only 5 instances of IsAriaExpanded in the codebase, and the logic in Button doesn't take that into account when setting the actual aria-expanded attribute (it checks if either IsExpanded or IsAriaExpanded is true, but then only sets aria-expanded based on IsExpanded). Having separate Disabled and AriaDisabled attr is fine since they're distinct things, but there's only an aria-expanded attr so just using IsExpanded is fine.

If you're okay with that, then for this PR we can just update this line to:

button--IsExpanded=(ternary expandable-section--IsExpanded true false)

(since we always want aria-expanded rendered whether true or false for this component), and open a followup for the other files (unless you want to update them here @Mash707, shouldn't take long at all).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@Mash707 ignore this comment for this PR. Coker and I met up to discuss this and I'll put a followup PR to update this across components.

@Mash707
Mash707 force-pushed the expandable-section-examples-missing-aria-attributes branch from 419ae98 to d19e9d0 Compare September 8, 2025 19:00

@mcoker mcoker left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM!

@thatblindgeye
thatblindgeye merged commit 91dcaee into patternfly:main Sep 9, 2025
4 checks passed
@patternfly-build

Copy link
Copy Markdown
Collaborator

🎉 This PR is included in version 6.3.0-prerelease.62 🎉

The release is available on:

Your semantic-release bot 📦🚀

@Mash707
Mash707 deleted the expandable-section-examples-missing-aria-attributes branch September 10, 2025 19:48
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.

ExpandableSection - examples missing aria attributes

4 participants