Use more descriptive accessible headings for search result pages

[sarayourfriend]

Fixes

Fixes #790 by @alexstine (by way of @zackkrida extracting the issue from a tracking issue of several required accessibility fixes)

Description

The large number of added lines come from snapshot tests, not actual code changes. See the testing instructions section for details about these.

The PR adds new sr-only text to the page, as suggested by Alex in the issue. I've chosen to use different variations based on the search type and count of media. In my first attempt, I used two separate h1s. However, this caused an issue with introducing unintended height. Rather than adding additional classes to the second one to prevent that, I decided to just put them inside the same h1, with each bit wrapped in a span so I could mark them as aria-hidden or sr-only as needed. This appears to work in all my testing (ORCA on Linux, NVDA on Windows and VoiceOver on macOS, the latter two in VMs), but please try to break this to make absolutely sure I've got it right.

• For "all-media" search types, the string is "All results for {query}, {imageResults}, {audioResults}" where the image and audio results are either "No images/audio", "{count} images/audio", or "Over 10000 images/audio". This means the title will inform that the query was for all media types, and the count of results for each media type. That might be too tedious, compared to a unified count, I'm not sure.
• For individual media types, the string is either "No image results for {query}", "1 image result for {query}", "{count} image results for {query}", or "Over 10000 image results for {query}" where "image" can be either image or audio depending on the search type.

Testing Instructions

Run the application locally using just p frontend dev from the root directory of the repository.

Execute a search from the homepage, and using a screen reader, observe the h1 of the page. Confirm it meets the requirements laid out in the issue.

I used snapshot tests rather than individual assertions for each possibility of the accessible text because otherwise I would have to duplicate the translation, which isn't the point. Ideally this would use a diff snapshot test, render a couple "baseline" variations, and then snapshot diff against those to confirm they are all different in code, but you can do this by reading the snapshots themselves. This is a small component, so the snapshots are not too cumbersome.

I wanted to use inline snapshots, because I've found those easier to work with in the past. However:

• You cannot use them in an each test
• Our trailing whitespace pre-commit fix kept trimming whitespace present in the one that isn't in the each block, and I was worried about how many different formatting issue's I'd need to fight against overall. We should disable that trailing whitespace check for anything linted by ruff, prettier, or eslint, though, as it's just duplicating those tools and is less configurable in cases like this... plus, in this case, it was modifying a string literal in the code, which is no good!

Anyway, that's out of scope for this issue. I just wanted to explain the decisions I made there.

Checklist

• My pull request has a descriptive title (not a vague title likeUpdate index.md).
• My pull request targets the default branch of the repository (main) or a parent feature branch.
• My commit messages follow best practices.
• My code follows the established code style of the repository.
• I added or updated tests for the changes I made (if applicable).
• I added or updated documentation (if applicable).
• I tried running the project locally and verified that there are no visible errors.
• [N/A] I ran the DAG documentation generator (if applicable).

Developer Certificate of Origin

Developer Certificate of Origin

Developer Certificate of Origin
Version 1.1

Copyright (C) 2004, 2006 The Linux Foundation and its contributors.
1 Letterman Drive
Suite D4700
San Francisco, CA, 94129

Everyone is permitted to copy and distribute verbatim copies of this
license document, but changing it is not allowed.


Developer's Certificate of Origin 1.1

By making a contribution to this project, I certify that:

(a) The contribution was created in whole or in part by me and I
    have the right to submit it under the open source license
    indicated in the file; or

(b) The contribution is based upon previous work that, to the best
    of my knowledge, is covered under an appropriate open source
    license and I have the right under that license to submit that
    work with modifications, whether created in whole or in part
    by me, under the same open source license (unless I am
    permitted to submit under a different license), as indicated
    in the file; or

(c) The contribution was provided directly to me by some other
    person who certified (a), (b) or (c) and I have not modified
    it.

(d) I understand and agree that this project and the contribution
    are public and that a record of the contribution (including all
    personal information I submit with it, including my sign-off) is
    maintained indefinitely and may be redistributed consistent with
    this project or the open source license(s) involved.

[dhruvkb]

Tried it locally and it works well. I have some subjective opinions but not worth blocking the PR over.

But, if all that's changed is a11y of search headings, the changes to the snapshots are unwarranted. Looking into the changes it seems like the changes are due to font size differences, different shades of grey in checkboxes, and difference in waveforms of audio results. Maybe they are flakes and the checks will still pass if they are reverted?

[dhruvkb]

Subjective opinion: Number with 'audio' felt wrong.

Suggested change

	count: "{localeCount} audio",
	countMore: "over {localeCount} audio",
	count: "{localeCount} audio tracks",
	countMore: "over {localeCount} audio tracks",

[sarayourfriend]

Done in db46a04

[zackkrida]

@sarayourfriend I believe "tracks" is still missing in the cases where the string is All results for "zack", 10 images and 10 audio.

[obulat]

@sarayourfriend, I'm drafting this PR to prevent pings while the comments on prettier and snapshot changes are addressed.

[sarayourfriend]

@obulat @dhruvkb This is ready for review.

[zackkrida]

Looks great. Requesting one small change for the last place where "tracks" is missing after "audio", and a non-blocking suggestion for code readability in the unit test.

[zackkrida]

@sarayourfriend I believe "tracks" is still missing in the cases where the string is All results for "zack", 10 images and 10 audio.

[zackkrida]

Great 👍 I appreciate the unit test improvements as well, like adding the translation job and excluding the tailwind prettier plugin

[obulat]

I tested this PR on NVDA and VoiceOver.

NVDA reads the heading, and you can get to the heading using the H shortcut.
In VoiceOver, you have to navigate inside the heading element to read the heading using the shortcut (VO+command+H). If you try the same shortcut when the VO focus area is in the content area (one level above), it says "Heading not found". I'm not sure if it's supposed to be like that, or if I'm using VO incorrectly, or if VO has a bug, but I couldn't find a way to navigate through the headings on the search page.

[obulat]

Great find!

[obulat]

Much better naming ✨

[zackkrida]

A few snapshots need updating after the merge of #3967

[sarayourfriend]

I had to squash this PR, the back-and-forth commits were making rebases a pain. I should be able to merge this now so long as CI passes, there are no changes other than updates to the snapshots that Zack mentioned.