Accessibility: Improve Voiceover

[akirk]

Improves Voiceover through a dedicated aria-label:

Checklist

• Tests written for new code (and old code if feasible)
• Linter and other CI checks pass
• Sign-off given on the changes (see CONTRIBUTING.md)

Signed-off-by: Alex Kirk <[email protected]>

---

Here's what your changelog entry will look like:

✨ Features

• Accessibility: Improve Voiceover (#12189). Contributed by @akirk.

[florianduros]

Hi!

Thanks for the contribution. Can you fix the typescript, lint and tests errors to make the CI happy?

[florianduros]

@akirk Hello! The CI is in failure. Do you still want to work on this PR? Thanks

[t3chguy]

@akirk any chance of splitting out the improve voiceover changes into a separate PR to simplify concerns for review?

[akirk]

I tried to address the lint problems, I am not sure it worked, npm run lint:fix started to work on code that I didn't touch so I tried to omit it.
For types npm run lint:types complains about lots of files that I didn't touch either. Could you give me a few pointers in how I could address these issues?

[t3chguy]

I tried to address the lint problems, I am not sure it worked, npm run lint:fix started to work on code that I didn't touch so I tried to omit it.
For types npm run lint:types complains about lots of files that I didn't touch either. Could you give me a few pointers in how I could address these issues?

Could you share examples of changes and errors respectively

[akirk]

any chance of splitting out the improve voiceover changes into a separate PR to simplify concerns for review?

Ok, I'll create a new PR and remove the keyboard nav changes from this PR.

[akirk]

The new PR for the keyboard navigation is #12328.

[t3chguy]

Previous comment re other Event renderers still stands

[t3chguy]

This comment doesn't make much sense, and points out that not all consumers of MemberAvatar may have any appended message so maybe another solution is desired which is more respective of the plethora of places a MemberAvatar may be used

[MidhunSureshR]

I've tried something in 90ea6c4

Basically the aria-label for the component itself is just the name (without dot).
In EventTile, we override the accessible name of the DIV that contains the avatar to be name + dot.

[MidhunSureshR]

That won't work because giving a div both a role and a negative tab index would be yet another accessibility violation. I've added a prop to conditionally add the . to the aria label.
I'm not entirely convinced that this is really useful. Testing with Orca, I don't really see a difference with/without the dot.

[MidhunSureshR]

Seems to break for events sent by the user

[MidhunSureshR]

getId() here will actually return the local id but will return the remote id from sync elsewhere:

Probably best to use mxEvent.getTxnId() ?? mxEvent.getId()

[MidhunSureshR]

I don't think this approach is viable:

1. Not sure if we should worry about the intrinsic behavior of screen readers i.e when and how much they pause. Testing with orca, I don't even get a pause.
2. An event can be rendered twice - once in the main timeline and then again in the thread panel. Assigning an id in this fashion would mean multiple elements have the same id.
3. Some message bodies already render with an id eg: MLocationBody

I think something like e30354b is probably what we want but we'd have a function that takes an event and spits out some accessible text:

"aria-label": this.props.mxEvent.sender.name + '. ' + getAccessibleTextForEvent(this.props.mxEvent),

I'm inclined to close this PR for now.

[MidhunSureshR]

Closing for the reasons mentioned in #12189 (comment)