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

va-process-list-item: move status eyebrow into header so that it gets read by screen readers #1437

Merged
merged 5 commits into from
Dec 17, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,11 @@ describe('va-process-list-item', () => {
expect(element).toEqualHtml(`
<va-process-list-item class="hydrated usa-process-list__item" role="listitem" header="Heading">
<!---->
<h3 class="usa-process-list__heading" part="header">Heading</h3>
<h3 aria-label="Heading" class="usa-process-list__heading" part="header">
<span aria-hidden="true">
<div>Heading</div>
</span>
</h3>
<p>Some content</p>
</va-process-list-item>
`);
Expand All @@ -47,7 +51,11 @@ describe('va-process-list-item', () => {
expect(element).toEqualHtml(`
<va-process-list-item class="hydrated usa-process-list__item" role="listitem" header="Heading" level="1">
<!---->
<h1 class="usa-process-list__heading" part="header">Heading</h1>
<h1 aria-label="Heading" class="usa-process-list__heading" part="header">
<span aria-hidden="true">
<div>Heading</div>
</span>
</h1>
<p>Some content</p>
</va-process-list-item>
`);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,12 +49,20 @@ describe('va-process-list', () => {
</mock:shadow-root>
<va-process-list-item class="hydrated usa-process-list__item" header="Step one" role="listitem">
<!---->
<h3 class="usa-process-list__heading" part="header">Step one</h3>
<h3 aria-label="Step one" class="usa-process-list__heading" part="header">
<span aria-hidden="true">
<div>Step one</div>
</span>
</h3>
<p>Some content</p>
</va-process-list-item>
<va-process-list-item class="hydrated usa-process-list__item" header="Step two" role="listitem">
<!---->
<h3 class="usa-process-list__heading" part="header">Step two</h3>
<h3 aria-label="Step two" class="usa-process-list__heading" part="header">
<span aria-hidden="true">
<div>Step two</div>
</span>
</h3>
<p>Additional content</p>
<ul>
<li>Item one</li>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,11 @@
}

.usa-process-list__heading-eyebrow {
@extend .usa-process-list__heading; // needed for "source Sans Pro" font-family
color: var(--vads-color-base-darker);
text-transform: uppercase;
font-weight: normal;
font-size: var(--vads-font-size-source-sans-normalized);
}

va-process-list-item[pending='true'] .usa-process-list__heading {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,11 +57,15 @@ export class VaProcessListItem {

return (
<Host role="listitem" class='usa-process-list__item'>
{ status ?
<div class="usa-process-list__heading-eyebrow">{statusTextMap[status]}</div>
: null
}
{header ? <HeaderLevel part="header" class='usa-process-list__heading'>{header}</HeaderLevel> : null}
{header || status ? (
// aria-label hack to avoid header being read twice in voiceOver
<HeaderLevel part="header" class='usa-process-list__heading' aria-label={`${status ? `${statusTextMap[status]}: ` : ''}${header}`}>
Copy link
Contributor Author

@powellkerry powellkerry Dec 13, 2024

Choose a reason for hiding this comment

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

In VoiceOver when there are multiple elements in the header it will read it as "Header level 3; two items;" and repeats the heading text multiple times. The aria-label in the header and aria-hidden on the container fixes this issue, but it seems like there should be a better solution? @rsmithadhoc do you have a suggestion?

Copy link
Contributor

Choose a reason for hiding this comment

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

I tested in other screen readers, JAWS didn't recognize the headings at all. I'll take a look and see if there's a better solution.

Copy link
Contributor

Choose a reason for hiding this comment

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

@powellkerry
I think the original HTML may be the way to go here.

    <h2>
        <span></span>
        <span></span>
    </h2>

When reading the page, I see that VoiceOver reads the heading, then repeats the 2 items within the heading, like you said. But when using the command to read the next heading it only reads it once and using the headings menu it only reads it once, as expected.

While not ideal for VoiceOver, we may have to accept it for now. It may be confusing, but I don't think it would hinder the user in navigating the page. JAWS and NVDA only read it once when reading the page. I also tested them reading next heading or using their headings menu and it also functioned as intended with the original HTML.

I think the HTML is fine in the original way, so VoiceOver should be able to handle it. I think it also keeps things much simpler and should work in a wider range of places, as aria-hidden can cause some issues, like I saw with JAWS.

Thanks!

<span aria-hidden="true">
{status ? <div class="usa-process-list__heading-eyebrow">{statusTextMap[status]}</div> : null}
{header ? <div>{header}</div> : null}
</span>
</HeaderLevel>
): null}
<slot/>
</Host>
)
Expand Down
Loading