Bug - [Popover] - [accessibility issues with footer, header, and heading]
Describe the problem
Accessibility issues for Popover element:
- with
footerContentprop - with
headerContentprop - with default value
h6ofheaderComponentprop
Heading levels should only increase by one
https://dequeuniversity.com/rules/axe/4.9/heading-order
<h6 class="pf-v5-c-popover__title-text"><div>Popover header</div></h6>
Document should not have more than one banner landmark
https://dequeuniversity.com/rules/axe/4.9/landmark-no-duplicate-banner
<header class="pf-v5-c-masthead pf-m-display-inline ws-masthead">
Related node:
<header class="pf-v5-c-popover__header"><div class="pf-v5-c-popover__title" id="popover-pf-17234753664484trsgdfq2md-header"><h6 class="pf-v5-c-popover__title-text"><div>Popover header</div></h6></div></header>
Document should not have more than one contentinfo landmark
https://dequeuniversity.com/rules/axe/4.9/landmark-no-duplicate-contentinfo
<footer class="pf-v5-c-page__main-section ws-org-pfsite-l-footer">
Related node:
<footer class="pf-v5-c-popover__footer" id="popover-pf-17234753664484trsgdfq2md-footer">Popover footer</footer>
Ensures landmarks are unique
https://dequeuniversity.com/rules/axe/4.9/landmark-unique
<header class="pf-v5-c-masthead pf-m-display-inline ws-masthead">
Related node:
<header class="pf-v5-c-popover__header"><div class="pf-v5-c-popover__title" id="popover-pf-17234753664484trsgdfq2md-header"><h6 class="pf-v5-c-popover__title-text"><div>Popover header</div></h6></div></header>
<footer class="pf-v5-c-page__main-section ws-org-pfsite-l-footer">
Related node:
<footer class="pf-v5-c-popover__footer" id="popover-pf-17234753664484trsgdfq2md-footer">Popover footer</footer>
How do you reproduce the problem?
- Visit https://www.patternfly.org/components/popover/#basic
- Click Toggle popover button.
- Scan page with axe DevTools and ignore the last issue as irrelevant.
Expected behavior
Render markup that passes axe DevTools rules.
Is this issue blocking you?
No, we will fix 10 occurrences as follows:
- Move occurrences of
footerContent, orheaderContent, or both, to props of a pure presentation component that composes them withbodyContentand returns markup forbodyContentprop ofPopoverelement. That is, replacefooterandheaderelements withdivelements that have PatternFly classes for same visual appearance. - Render (instead of heading element)
pelement that has PatternFly class for same visual appearance, but takes out of heading hierarchy of page. Similar solution as our workaroundcomponent="p"prop forAlertelements that do not seem to fit heading hierarchy of page.
Screenshots
What is your environment?
- OS: macOS 14.6.1
- Browser: Chrome 127
- PatternFly: react-core 5.2.2
What is your product and what release date are you targeting?
Red Hat Advanced Cluster Security
Any other information?
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.
https://github.com/patternfly/patternfly-react/issues/11393 is related.
Part of this could be resolved by us implementing a prop to control the wrapper element in PopoverHeader and PopoverFooter (maybe either div, span, or the default header|footer elements). The issue arises when the Popover is appended to the document body and thus outside the main element; at that point header and footer retain their semantic roles, but already having a header and footer on the page itself conflicts. Inside a main element those elements have different semantics, so appending the Popover "inline" or somewhere else within the main element would also help resolve the issue.
The heading level issue requires more manual update as it's dependent on the context and up to the consumer to ensure the appropriate heading level is being used. I'd recommend rethinking the use of p elements that retain the "heading" styling, though. Either the heading element should be used and just update which heading level is used, or no heading should be included. It may lead to confusion to have text that looks like heading text in other views, but isn't semantically a heading element.
Thank you for commenting.
For your info, a more recent situation with info prop of Th element prevented even the work-around.
https://www.patternfly.org/components/table#custom-row-wrapper-header-tooltips--popovers
I'd recommend rethinking the use of p elements that retain the "heading" styling, though. Either the heading element should be used and just update which heading level is used, or no heading should be included. It may lead to confusion to have text that looks like heading text in other views, but isn't semantically a heading element.
Aha, for Popover element, I will review uses to omit heading whenever possible, like first example in guidelines:
https://www.patternfly.org/components/popover/design-guidelines#usage
For Alert element:
- The default
h6element confirms that consistent formatting is intuitive, independent of context on page. - Context-dependent heading element, but consistent formatting independent of level seems like similar problem to what you noted.
As you can imagine, a few components render reusable application-specific React elements (that have
Alertelements) in different heading hierarchies.
Is there a generic solution to heading-ish content independent of hierarchy?
During a past tour of duty in technical documentation, we used a specific style for equivalent to Alert elements.
When fixing issues from axe DevTools, I searched unsuccessfully for guidance (PatternFly or otherwise) on the subject.
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.
So sad 😰
Sorry for the delay in response @pedrottimark. FWIW our github actions now only labels things as stale when there's a lack of activity and simply gives us a reminder to look at those stale things (no more auto-closing of issues).
For your info, a more recent situation with info prop of Th element prevented even the work-around.
Yeah that would be expected. As part of adding the ability to override the wrapper of popover header/footer elements, being able to adjust them via that info prop in Th component should also work. If you'd be okay with it I can update this issue to be scoped to adding that customization for Popover headers/footers wrappers.
Is there a generic solution to heading-ish content independent of hierarchy?
Not sure if I 100% understand this question. Do you mean a solution for having content look heading-ish without being a part of the hierarchical heading structure of the rest of the page?
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.