matrix-react-sdk icon indicating copy to clipboard operation
matrix-react-sdk copied to clipboard

Published 20 hours ago • verified publisher icon matrix-org

Accessibility: Improve Voiceover

Open akirk opened this issue 1 year ago • 7 comments

Improves Voiceover through a dedicated aria-label:

message-navigation

Checklist

  • [ ] Tests written for new code (and old code if feasible)
  • [x] Linter and other CI checks pass
  • [x] 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.

akirk avatar Jan 30 '24 14:01 akirk

Hi!

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

florianduros avatar Jan 30 '24 16:01 florianduros

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

florianduros avatar Mar 05 '24 10:03 florianduros

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

t3chguy avatar Mar 08 '24 10:03 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?

akirk avatar Mar 08 '24 10:03 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?

Could you share examples of changes and errors respectively

t3chguy avatar Mar 08 '24 10:03 t3chguy

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 avatar Mar 08 '24 11:03 akirk

The new PR for the keyboard navigation is #12328.

akirk avatar Mar 08 '24 11:03 akirk

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 https://github.com/matrix-org/matrix-react-sdk/pull/12189/commits/e30354b57f7d4688401540c01b8ec585b3d758f6 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 avatar May 22 '24 08:05 MidhunSureshR

Closing for the reasons mentioned in https://github.com/matrix-org/matrix-react-sdk/pull/12189#issuecomment-2124168056

MidhunSureshR avatar Jun 13 '24 08:06 MidhunSureshR