WordPress / gutenberg

The Block Editor project for WordPress and beyond. Plugin is available from the official repository.
https://wordpress.org/gutenberg/
Other
10.52k stars 4.21k forks source link

Site editor: Fix the document title announcement for screen readers #56188

Closed afercia closed 11 months ago

afercia commented 1 year ago

Description

Alternative to https://github.com/WordPress/gutenberg/pull/56167

The Site Editor uses a 'single page app' navigation type, where navigating through the different views of the editor updates the content of the page but the HTML document is always the same. As such, like in all single page apps, the navigation needs to be announced bt screen readers to emulate the native beheavir of a traditional navigation across different pages, where the document title is announced as first thing.

The current implementation in the Site editor uses a speak() to announce the updated document title and the site title, also appending the string WordPress:

https://github.com/WordPress/gutenberg/blob/dc7e1aa202ee2605f0423ac0f48fa590b3027f60/packages/edit-site/src/components/routes/use-title.js#L38-L58

There's a few problems though:

Suggested remediation:

Step-by-step reproduction instructions

Screenshots, screen recording, code snippet

Wrong document title announced on the Templates view:

wrong document title

Wrong document title announced on the Navigation view:

Screenshot 2023-11-16 at 09 14 59

Example of too verbose announcement:

too verbose

Environment info

No response

Please confirm that you have searched existing issues in the repo.

Yes

Please confirm that you have tested with all plugins deactivated except Gutenberg.

Yes

alexstine commented 1 year ago

@afercia How would you feel about completely eliminating this message? My PR does just that. #56167. We now have progress bar to communicate loading state and the heading structure is much improved for context. I see this as being much less of an issue compared to the beginning of FSE.

afercia commented 12 months ago

@alexstine I'm not sure entirely removing these announcements would be good for all users.

We now have progress bar to communicate loading state

I had a look at the progressbar implementation and I doubt it can work correctly as it misses some important attributes like aria-valuenow. Also, when used as a loading indicator, the progressbar should be responsible to set aria-busy on the region that is being loaded. All this is well explained in the spec but I don't see any of that in the current code. Regardless, testing with Safari and VoiceOver, I don't get any progress announcement. We should create an issue to fix the progressbar.

and the heading structure is much improved for context.

Yes that is improved but I'm not sure it is sufficient. When testing https://github.com/WordPress/gutenberg/pull/56167 upon navigation the announcement would only tell something about the back button. Something like:

Back, button, Navigation, region

and nothing else, which isn't great, in my opinion.

alexstine commented 12 months ago

I'll make a follow-up to fix Progressbar. I thought we covered everything there but guess not. 😞 Testing in Storybook and site editor produced two different results I suppose.

alexstine commented 12 months ago

@kevin940726 or @youknowriad Can you offer up any ideas on what might be setting incorrect page titles in the site editor?

Make sure the document title always accurately reflects what the current view is.

I traced through the selectors and it looks like it tries to make a guess based off of template name and template type but the home page of the site editor should be neither of those. Need some custom fallback logic? I bet it's getting a default set somewhere and that's what makes it return the wrong page when the proper context can't be found.

youknowriad commented 12 months ago

I think at the moment the useTitle call is within the "Editor" component and is using the title from the useEditedEntityRecord hook. Which means it's showing the title of the parent template for the pages that show "pages within their templates". That's probably causing some issues.

The other thing is that as written today the "title" is guessed from the edited entity in the editor but in a lot of site editor pages, we always show the home page but the sidebar is different (templates, ...) so it makes more sense to tie the "title" to the "routing" instead.

To be honest, the routing as implemented today in the site editor is very basic and adhoc and not well organized. I know @kevin940726 has started some explorations to improve the situations, maybe just a poc for now. But we will solve this properly once we have a solid routing system in place I think.

alexstine commented 12 months ago

@youknowriad Yeah, being able to properly tie the title to the path is likely a better solution going forward. I will not attempt to fix that in my PR, I will simply shorten the title as requested and leave this issue open for reference. Looks like this is going to be a much bigger effort than accessibility alone.

Thanks.