Inline Help Search Accessibility

[jeryj]

Overview

The search input will operate independently of the results. There won't be any arrow key interactions on the results while focused on the search input.

Markup General Structure

The previous markup was one list with <li><h2>Section Title</h2></li> with a combo-box-like interaction using arrow keys to navigate results. This PR changes it to a list of links with titles outside of the lists, using TAB to navigate the search results.

<div aria-live="polite">
    <!-- Placeholders in search go here and have "Loading search results" announcement -->
    <!-- Error message would go here so it is announced -->
    <div aria-label="Search Results">
        <h3 id="section-1">Support articles</h3>
        <ul aria-labelleddby="section-1">
            <li><a href="#">Item 1</a></li>
            <li><a href="#">Item 1</a></li>
        </ul>
        <h3 id="section-2">Show me where to</h3>
        <ul role="grid" aria-labelledby="section-2">
            <li><a href="#">Item 3</a></li>
            <li><a href="#">Item 4</a></li>
        </ul>
    </div>
</div>

Keyboard interactions

Everything gets a tab! This does add more tabstops to the page, but overall it is a more consistent and, likely, usable and standardized experience. Using grid roles to implement arrowed results would likely be a confusing experience and, in the end, only benefit advanced sighted keyboard users.

Screen Reader Announcements

I would have liked to have used speak() to announce state changes on the search, such as:

• Loading/Searching
• New results loaded
• Errors

There isn't an a11y speak() utility in Calypso and I was hesitant to add one. I opted to wrap anything loaded from the search (error message, placeholder search bars, results) in an aria-live region. It works fairly well, but sometimes the results are not announced. This may be due to my inexperience with a screen reader though, as I believe the implementation is correct.

Testing instructions

Visually, nothing should have changed. Manually test searches (including searches that would not return results, like asdfadfasdfa) for:

• /home search section
• FAB ? in the bottom right of pages

All functionality should work correctly and feel the same as on production when using a mouse.

The HTML should look similar to the HTML section in this PR, with:

• aria-live="polite" wrapping the results
• <h3>Section Title</h3><ul>...</ul> for the sections
• Placeholder lines (when searching) should be unsemantic <div>s instead of a <ul> since it wasn't a real list, but a stylized thing to show the search.
• When searching (when placeholder search lines are showing), there should be a hidden "Loading search results" message

If you are comfortable with VoiceOver on Safari or another screen reader, you can test the announcements by performing a search and hopefully:

• "Loading search results" is announced during a long running search
• New results are announced when a search is complete. I've found this to be inconsistent in Safari, but mostly works (it's definitely better than current state)
• Error message is announced when no results are found

Fixes #43617

Translation String

The "Loading search results" string is hidden text only available to a screen reader. It is announced during this state when results are being fetched.

[matticbot]

Test live: https://calypso.live/?branch=update/inline-help-grid

[matticbot]

Here is how your PR affects size of JS and CSS bundles shipped to the user's browser:

Sections (~756 bytes removed 📉 [gzipped])

name      parsed_size           gzip_size
home          -1145 B  (-0.3%)     -276 B  (-0.3%)
preview        -659 B  (-0.4%)     -119 B  (-0.3%)
help           -659 B  (-0.1%)     -120 B  (-0.1%)
domains        -659 B  (-0.1%)     -119 B  (-0.0%)
checkout       -659 B  (-0.0%)     -122 B  (-0.0%)

Sections contain code specific for a given set of routes. Is downloaded and parsed only when a particular route is navigated to.

Async-loaded Components (~363 bytes removed 📉 [gzipped])

name                                         parsed_size           gzip_size
async-load-blocks-inline-help                     -659 B  (-0.9%)     -120 B  (-0.7%)
async-load-blocks-inline-help-popover             -473 B  (-0.2%)     -159 B  (-0.3%)
async-load-components-web-preview-component       -451 B  (-0.1%)      -84 B  (-0.1%)

React components that are loaded lazily, when a certain part of UI is displayed for the first time.

Legend

What is parsed and gzip size?

Parsed Size: Uncompressed size of the JS and CSS files. This much code needs to be parsed and stored in memory.
Gzip Size: Compressed size of the JS and CSS files. This much data needs to be downloaded over network.

Generated by performance advisor bot at iscalypsofastyet.com.

[obenland]

Are there tests that can be added to make sure the work you've done here is not going to be broken down the road?

[jeryj]

@obenland I'm honestly not sure what would be best to do regarding the tests. I don't think there are any integration or e2e tests for this. The tests I removed were all regarding the state, which I removed since we no longer needed those actions or states with the new keyboard interactions.

I do think adding some tests would be great, but I think it would likely need to be in the larger context of testing the component, not just this work. Maybe @getdave has some ideas on this?

[cpapazoglou]

Thanks @jeryj for wrangling this! Tabing through the results (instead of using arrow keys) is more intuitive in terms of accessibility. Also it seems that the code is cleaner now! Well done!

That said, it really works well in chrome but can't tab in results in safari the tabbed elements in sequence are input -> Close button -> Contact Us

With voice over, I got both the "Results are loading" and the actual results announcements!
In Chrome, I got the sections headers announced twice

Generally the Voice Over is a little hit and miss. I got very inconsistent results. I suppose we have to set it up properly.. To get solid results, I had to disable and enable before interacting with the Search component.

[jeryj]

Thank you for the review, @cpapazoglou! I don't get the Tab sequence issue in Safari 🤔. It sounds a little like it wasn't being built right, as that would have been the expected behavior previously to this PR. Could you try it again, and if Safari is doing that for you:

• Try using the up/down arrows while focused on the input and see if the virtual focus moves (it should not work, but that would explain the issue)
• Check the source HTML to make sure it's updated correctly

[cpapazoglou]

Thank you for the review, @cpapazoglou! I don't get the Tab sequence issue in Safari 🤔. It sounds a little like it wasn't being built right, as that would have been the expected behavior previously to this PR. Could you try it again, and if Safari is doing that for you:

• Try using the up/down arrows while focused on the input and see if the virtual focus moves (it should not work, but that would explain the issue)
• Check the source HTML to make sure it's updated correctly

Tried rebuilding, clearing cache etc. Could not make it work, tabbing from the input moves me to Close Button and then to Contact Us.

Checked my preferences though... And the following did the trick

maybe it is worth mentioning in the accessibility PRs description 😃 !

So let's decide what we should do with the tracking event and we should be good to merge it!

[jeryj]

Checked my preferences though... And the following did the trick

Oh yeah! I did that when I set-up my computer then forgot about it... It's such an odd preference. Nice find!

[jeryj]

@cpapazoglou Could you dismiss or approve your review? @retrofox said he'd give it another review in the morning. Thank you again for your testing and code review! 🙌

[retrofox]

LGTM. It works as expected. Also, confirmed that it keeps recording events either by clicking or pressing key.

Nice enhancement! !

[a8ci18n]

This Pull Request is now available for translation here: https://translate.wordpress.com/deliverables/4087237

Thank you @jeryj for including a screenshot in the description! This is really helpful for our translators.

[getdave]

@obenland I'm honestly not sure what would be best to do regarding the tests. I don't think there are any integration or e2e tests for this.
I do think adding some tests would be great, but I think it would likely need to be in the larger context of testing the component, not just this work. Maybe @getdave has some ideas on this?

In fact, we do have e2e tests for this component. I spent a good part of a previous Iteration adding these. I think perhaps you were away for this Iteration however @jeryj? Take a look in:

• test/e2e/specs/wp-my-home-support-search-spec.js
• test/e2e/specs/wp-inline-help-support-search-spec.js

I'm noticed that both this PR and @retrofox's PR to add the admin sections didn't make provision for updating the tests. In one way I'm pleased the tests were resilient enough to avoid breaking during your refactor - this is correct as the existing functionality was preserved. In another way however, this seems overly lenient and points towards the need to assert more closely on the behaviour of the components.

To ensure this necessary work isn't forgotten I've created an Issue to track the need to update the e2e tests.

I could easily do this myself. However, I believe that this would be a great opportunity for @jeryj and @retrofox to write their first(?) e2e tests in Calypso and become familiar with the process. This will be a good learning environment because you will be adjusting existing tests rather than writing from scratch would should make everything easier.

I'm more than happy to provide support with setup, reviews and implementation.

I know we have lots of things on our plate, but I would like to see this pulled into the next Iteration to ensure we have fully completed the tasks on the support search and left things in a good state for future teams.

[retrofox]

..., I believe that this would be a great opportunity for @jeryj and @retrofox to write their first(?) e2e tests in Calypso and become familiar with the process. This will be a good learning environment because you will be adjusting existing tests rather than writing from scratch would should make everything easier.

Indeed. Sorry about this, honestly I was aware these implementations needed tests, but I put them in a low priority among other tasks that we had to achieve. Going to take it as soon as I get the chance.
Also thank you for pushing for this. Definitely it's a good practice that we need to incorporate in any scenario where makes sense, as well as we add/should docs, jsdoc, unit tests, etc.

[a8ci18n]

Translation for this Pull Request has now been finished.