Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

Make the timeline less noisy for screen readers (mk II) #3019

Merged
merged 7 commits into from
May 23, 2019

Conversation

turt2live
Copy link
Member

There's some room for improvement here: we could be making the reader say "You sent a message at 18:46 [body]" or "TravisR sent a message at 18:46 [body]" to make it clearer. Currently it'll read out the sender profile if it is available, and if not just the timestamp + message. This improvement is best left to a future PR though.

Fixes element-hq/element-web#2716
Fixes element-hq/element-web#5697
See element-hq/element-web#9747

Original PR: #3007

To have less noise when they run over the sender profile.

See element-hq/element-web#9747
This reduces overall noise from the screen reader. It was reading the alt attribute from the sender avatar, which was just a mxid. The read receipts were just nonsensical noise.

Fixes element-hq/element-web#2716
Fixes element-hq/element-web#5697
See element-hq/element-web#9747
The short time is still read out (eg: 15:24), however by ignoring the anchor we prevent the reader from saying the title of the containing span. This prevents readers saying "Wed May 22, 2019 at 15:24 15:24".

See element-hq/element-web#9747
@turt2live turt2live marked this pull request as ready for review May 22, 2019 22:00
@turt2live turt2live requested a review from a team May 22, 2019 22:01
Copy link
Collaborator

@jryans jryans left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, this version seems better! 😁 Thanks for the big comment!

src/components/views/rooms/EventTile.js Outdated Show resolved Hide resolved
src/components/views/rooms/EventTile.js Outdated Show resolved Hide resolved
src/components/views/rooms/EventTile.js Outdated Show resolved Hide resolved
src/components/views/rooms/EventTile.js Outdated Show resolved Hide resolved
@turt2live
Copy link
Member Author

@jryans can I get a quick sanity check that the logic in a7d2309 is still the same?

@turt2live turt2live requested a review from jryans May 22, 2019 22:37
Copy link
Collaborator

@jryans jryans left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, the if block version looks equivalent and easier to read. 😁

@turt2live turt2live merged commit 9a1a982 into develop May 23, 2019
@turt2live turt2live deleted the travis/sr/fix-timeline branch May 23, 2019 01:18
@pvagner
Copy link
Contributor

pvagner commented May 23, 2019

Hello,

I am afraid using aria-hidden for filtering duplicate live region announcements is a dangerous thing to do.
My understanding is that aria-hidden=true is the same thing to screen readers what CSS display: none is to everyone.
So for testing this feature also apply display: none to the same nodes where you are setting aria-hidden=true. While testing you should see a lot of flickering, repositioning and similar as it rerenders, but at the end everything that is usefull should stay readable.
I can't figure this but currently I do have riot opened in Firefox 68 nightly on linux. I am typing into a room where I am the only user and newly appearing event tiles all have aria-hidden=true.
The messages are read by screen readers as if everything was fine. However if I wish to use screen reader functionality to reread what has been said most recent messages are not readable because aria-hidden has been set to true .
Have you looked at toggling aria-live=polite to aria-live=off instead of manipulating aria-hidden?

Also I feel hiding not very accessible parts is dangerous too if we have no better accessible counter parts.
For example looking at any room with the same screen reader vs browser combination I can see no timestamps what so ever. I would prefer to be able to read the same timestamps you can see when using the screen reader. Additionally event link is now also hidden from screen readers too and that's critical functionality.

The same goes for user name vs avatar combinations and flairs. They are no longer interactive for screen reader users after these changes. I agree they should not be focusable when navigating by using tab or shift+tab but should be focusable with the other means and react to keypresses so screen reader users can find and act on them by using screen reader specific features.

The same goes for the read receipts. Currently read receipts images are hidden, that's a good thing. +xxx number is not yet hidden and that will be easier to tweak. However it would be awesome to turn the parent element encapsulating all the RR images into accessible button and generate aria-label for that button saying something like "Matthew, Travis R +5 read" to bring that feature back in much polished way. Activating that button will cause RR list to expand. When expanded it should be rendered as list of items where one can highlight individual items when navigating with arrow keys similar to the profile menu in functionality not design.

There might be other features that are not accessible and I may even not be aware of them e.g. message encryption status images.

If we were only designing for screen reader users and not trying to apply universal design where possible I would suggest to present the time line the way you are currently doing to screen reader users by hiding all interactive features. For bringing those features back I would then add an off screen positioned button to each event tile. Pressing that button would popup a list with link to users profile, ability to mention the sender and other users mentioned in the content, flair, read receipts and other event tile specific actions such as copying or sharing the event. That would unfortunatelly mean a lot of duplication so this really needs much more thinking.

I apologize for such verbose message thinking aloud too much. I really appreciate your time and all the resources you are putting into this and I am extremelly happy you are considering screen reader accessibility almost from the start.
According to what I am seeing we need a bit more planning and testing.
I would appreciate even more if we can try to come up with an UX that will make as many Riot features accessible as possible however hiding those features to screen reader users does not sound that appealing to me if I am not assured this is just a work in progress and the plan is to eventually make it all work great if there are no other technical limitations.

jryans added a commit that referenced this pull request May 28, 2019
jryans added a commit that referenced this pull request May 28, 2019
Revert "Make the timeline less noisy for screen readers (mk II) #3019"
jryans added a commit that referenced this pull request May 29, 2019
jryans added a commit that referenced this pull request May 29, 2019
Revert "Make the timeline less noisy for screen readers (mk II) #3019" for release
@t3chguy t3chguy restored the travis/sr/fix-timeline branch October 28, 2019 15:38
@t3chguy t3chguy deleted the travis/sr/fix-timeline branch May 25, 2020 18:12
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Screen reader reads out my message twice when I send it Read receipts are noisy for screen readers
3 participants