Skip to content

LG-7817 Ensure that screen reader follows search page hierarchy#7459

Closed
eileen-nava wants to merge 8 commits intomainfrom
em/7817-screen-reader-order
Closed

LG-7817 Ensure that screen reader follows search page hierarchy#7459
eileen-nava wants to merge 8 commits intomainfrom
em/7817-screen-reader-order

Conversation

@eileen-nava
Copy link
Copy Markdown
Contributor

@eileen-nava eileen-nava commented Dec 8, 2022

This is a draft PR to share my work. This PR is not ready for review.
Currently, the screen reader correctly reads results and follows the hierarchy of the page. However, there's a bug where the screenreader will read "Back" twice. I included a video of the buggy behavior.

BackReadsTwice.mov

Comment on lines 204 to 214
<PageHeading>{t('in_person_proofing.headings.location')}</PageHeading>
{arcgisSearchEnabled && (
<AddressSearch onAddressFound={handleFoundAddress} registerField={registerField} />
)}
<p>{t('in_person_proofing.body.location.location_step_about')}</p>
{locationsContent}
<BackButton onClick={toPreviousStep} />
<div aria-live="polite">
{arcgisSearchEnabled && (
<AddressSearch onAddressFound={handleFoundAddress} registerField={registerField} />
)}
</div>
<div role="status">
<p>{t('in_person_proofing.body.location.location_step_about')}</p>
{locationsContent}
<BackButton onClick={toPreviousStep} />
</div>
</>
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.

Note that we should be using the in-person-location-post-office-search component for address-search related work.

Actually, the address-search conditional should be removed since this was merged...

@allthesignals
Copy link
Copy Markdown
Contributor

Need to know whether the back button bug is a problem introduced by this change...

@allthesignals allthesignals force-pushed the em/7817-screen-reader-order branch from dd5ec56 to 98fdf24 Compare December 9, 2022 16:33
<PageHeading>{t('in_person_proofing.headings.location')}</PageHeading>
{arcgisSearchEnabled && (
<div aria-live="polite">
<AddressSearch onAddressFound={handleFoundAddress} registerField={registerField} />
Copy link
Copy Markdown
Contributor

@svalexander svalexander Dec 9, 2022

Choose a reason for hiding this comment

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

this is really a thought for design - but wondering if the search field should clear the user's input after clicking the search button. And if the "no matches found" text has enough prominence on the page. Right a user see's the address they input 2x on the page and it's hard to notice the "no match" text.

Copy link
Copy Markdown
Contributor

@svalexander svalexander Dec 9, 2022

Choose a reason for hiding this comment

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

oh wait, is the 2nd address shown on the page the one that was actually used by the search function?/ the closest match to the input? that's unclear. I'll add these thoughts in slack so design can 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.

crossing out the portions that are likely not applicable since the 2nd address is for debugging

<div role="status">
<p>{t('in_person_proofing.body.location.location_step_about')}</p>
{locationsContent}
<BackButton onClick={toPreviousStep} />
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.

another thing happening with the screen reader...it's currently reading the text that appears alongside the search button after it reads out "back"

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.

if that address is removed then this screen reader issue should be ignored.

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.

i've been trying to test this page but voiceover +chrome are acting up (constantly freezing and not auto reading the page) So I'm not 100% sure if it's just a chrome issue or if removing the "<" would fix the double reading of back (since the 2 pieces of text are separate).

const button = (
<Button isUnstyled {...props}>
&#x2039; {t('forms.buttons.back')}
<span aria-hidden="true">&#x2039; </span>
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

As I said in slack, the screenreader read "back" twice because it read "back" for both the arrow "<" and the word "back"

I fixed this glitch by adding aria-hidden="true" to the arrow "<". Documentation notes that aria-hidden is a way to fix duplicated content.

@eileen-nava
Copy link
Copy Markdown
Contributor Author

I am closing this PR. The non-draft PR, which is ready for review, is #7487.

@eileen-nava eileen-nava deleted the em/7817-screen-reader-order branch December 20, 2022 16:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants