Improve keyboard shortcuts & a11y. (Still a bunch remaining)

matrixbot commented 8 years ago

Created by @ matthew:matrix.org.

See scrollback in #vector with Peter Vagner from Nov 1

ara4n commented 8 years ago

This is needed for accessibility to work properly

pvagner commented 7 years ago

I've forked matrix-react-sdk and vector-web in order to test this approach adding a few changes like this. It mostly works although after the latest changes introduced in Riot v 0.9 I am starting to believe we do need more help with accessibility as room sublists are very difficult to navigate and right pannel holding tab pannels such as member lists, files and notifications are impossible to reach with further tweaks.

ndarilek commented 7 years ago

Would like to bump this. I was sad to see this regressed after returning to Matrix and attempting to use the non-vanilla React client. First I had to reject an invite, which involved routing my screen reader to the correct div and simulating a click. I then had to do the same to enter a room. Once inside rooms, the interface is painful to navigate since it no longer seems to include landmarks. I think I submitted some PRs to the vanilla React client to use more semantic HTML tags (I.e. section/nav/aside, and a <ul/ for the chat.) These appear to have been removed, and the result is that I can't really use the web interface anymore.

Additionally, I suspect but cannot confirm that some controls are image-only, and are missing textual representations at all. If these controls aren't links/buttons then they can't be interacted with via the keyboard, but if they don't include an alt attribute, implicit text, or an aria-label then they almost certainly won't appear to the screen reader at all.

I have lots of respect for what you're doing, and as much as I'd like to roll up my sleeves and fix this, I feel like I already addressed some of these and my changes were rolled back. I understand it was accidental, but these tweaks really do make the difference between an easy-to-use client and one that I avoid. If we can restore the missing functionality and commit to keeping it, I'd be happy to dig in and submit additional accessibility-related PRs to make Riot the most accessible chat client out there. Slack will likely never fix their accessibility issues, so I actively recommend Matrix/Rocket.chat wherever I can precisely because you all try to create a more accessible experience. :)

ara4n commented 7 years ago

bumped priority. agreed that the current situation sucks :(

pvagner commented 7 years ago

@ndarilek I started to be afraid that you have moved to rocketchat exclusivelly. I haven't taken care of missing aria landmarks and proper list items in the chat output area however I am trying to keep interactive controls focusable and labelled in my forks. I have it on a web server at https://matrix.pvagner.tk if you would like to see somewhat improved version in terms of accessibility. What I am afraid we need help is popup menus. We need to implement javascript focus handling and aria-active-descendants I assume. Similarly for the room sublists.

ndarilek commented 7 years ago

Hmm, any chance we can get your forks upstreamed as PRs?

pvagner commented 7 years ago

@ndarilek I think it needs more work. I would preferably like to have someone sighted to at least look at it whether changing divs to buttons does not ruin the overal design before subbmitting it as a PR. Additionally I've briefly chatted about my wish to make Riot more accessible with @ara4n so I assume people at the core team might be aware what we are trying to do here. If we have no one else to help with coding, then I wish more disabled people would join us by at least acknowledging we would be happy to have fully open decentralized and accessible messenger. Note I am also doing similar things with Riot Android but I am also not quite done with that either.

ndarilek commented 7 years ago

I understand that, but if you submit the PR then someone sighted can look over the changes as part of the review process, right? Also, if the element-based controls look different than their div counterparts, that's just a CSS tweak which designers could make on the Riot end.

Basically, I was about to give up and change all the divs, but I don't have a sighted person to look that over so I'd be in the same boat. I'd think that getting part of the job done would be appreciated, and might inspire someone else to finish it.

pvagner commented 7 years ago

Okay, I'll wait a bit to find out whether we have made enough noyse these days if someone will try to join us. If not I will update to the head revisions during one of the upcoming weekends and send it as a pull request.

ndarilek commented 7 years ago

Thanks! Do you also address the noisy read receipt icons? If not, I'll take a crack at those once your PR lands. Thanks for working on this.

pvagner commented 7 years ago

I haven't yet found how to make them less anoying but have access to that information. I don't want aria-labels on the messages as some of the screen readers are not rendering aria-labels as a content e.g. orca with Firefox on linux. Additionally there are more graphical only interactive controls associated with each message for example popup menu activation and encryption status which we do also need to be able to get properly accessible. Ideally I would like to avoid losing features as a trade of for better accessibility. I see accessible popup menus as a higher priority since this might become a building block for this in the future.

ara4n commented 7 years ago

@pvagner, @ndarilek: @JaniM did some work on this a few days ago, trying to replace all the divs with buttons. However, fighting against the default CSS for buttons seems to have been a nightmare.

Can anyone confirm whether <a role="button" onClick={...}/> is a good enough solution?

ndarilek commented 7 years ago

Two issues:

Please ensure that the button has tabindex="0" so it is keyboard-focusable.
Please ensure that the button responds to the Space and Enter keys for simulating clicks.

I think that, if you do those two things, it should work. Maybe package it up in a React <Button/> component using a <div/> underneath, but with the tabindex/keyboard behavior included so it doesn't get missed for future buttons. This would also let us make any additional modifications in one place and have them reflected across the codebase.

Thanks!

pvagner commented 7 years ago

Hello,

By saying role"button" you are saying this control behaves similar to button, let's present it accordingly. So if you cant change

this is a button

into you will have to also implement onKeyDown and in some cases some other things in order to implement proper keyboard controls.

ara4n commented 7 years ago

so @JaniM fixed this up. @pvagner, @ndarilek - thoughts on whether riot.im/develop are an improvement would be most welcome. Going to close for now and reopen if needed after comments.

ara4n commented 7 years ago

(thanks @JaniM!!)

pvagner commented 7 years ago

Indeed thanks @JaniM this is very functional. It's a pity I haven't discovered this earlier. It's almost a month since you have pushed this.

Unfortunately there are some edge case and follow issues to be addressed: I can point out some of them right now, however perhaps I have to fill them individually.

[x] When the left pane room lists are collapsed in the list of rooms perhaps individual items representing rooms need an aria-label because now only word DM is exposed to ATs.
[x] Buttons in the bottom left menu such as settings, room directory and all the others need aria-label or other technique to make them better accessible. Currently these can't be activated by using the keyboard alone. These buttons do have a label however the label is only shown on mouse over thus not shown at all for keyboard only users.
[x] There might be some left overs that are still divs. So far I was only able to notice Leave room button shown when clicking Settings button in the room header.
[x] When the member list is completelly collapsed room header has an image only button to show it. This perhaps needs an alt description or aria-label so people will understand what it does.
[x] Buttons next to the input field which are used for initiating voice / video call or file upload don't appear to be reachable via keyboard. I am not sure what they are missing my guess that they need either tabStop=0 or an alt text.
[x] We need to find out how to activate popup menu on an incoming message.
[x] We need to be able to switch side pannel from displaying member list to files and recents and / or the other way round. This is not accessible via keyboard.
[x] When the room search is showing we need to turn This room/All room toggles into radio buttons. I think similar technique can be used, however the role should be different.
[x] There is an image only button saying Search when the search is showing. This needs proper role as well.
[x] In the room statusbar Scroll to the bottom button appears when there are new messages in a room. This is a clickable image. It should also be turned into accessible button the same way most of the buttons have been tweaked recently.
[x] Text formatting toolbar is missing all the accessibility related awesomeness @JaniM has prepared as a general solution for accessible buttons.
[x] Both jump to last unread message bar and allow widget access dialog are confusing. I think there is a clickable button at the top, then there is an alert text and there is one or more buttons. Are those clickables at the top also actionable?
[ ] Tabbing to a hidden link for each event in the messages timeline is confusing. Should that link be there at all and what's its purpose? For copying there is permalink in the individual event's menu. If the link should be there perhaps we may consider setting its text to a hard space or some other invisible character and set its title attribute to a string that would hint screen reader users to its functionality there.
[x] There is an unlabelled button on the left pane next to the filter rooms entry. Apparently it's used for collapsing / expanding that pannel.
[x] There are still some texts that can't be translated. E.G. image alt texts on the homepage and warning icon on a You are not receiving desktop notifications. There might be more however I haven't recognized them so far.
[x] Expand button for multiple events such as X has joined and 2 others left is not accessible and its text is not translatable.
[x] Rewoke widget access dialog has its text exposed as alert. All the other Riot dialogs I have seen are missing that.
[x] We need to reconsider if setting alt text of all avatar images is helping accessibility. There are some places e.g. right pannel member list and messages timeline where this adds unecessary extra verbosity as the surrounding text holds the same information.
[x] When changing Riot language via Settings UI list of languages is not keyboard accessible.
[ ] It would be nice to consider marking typing indicator as an aria-alert so we can notice when someone is typing.
[x] It's not a very good practice to disable, remove or hide focused DOM elements e.g. after clicking. I have just noticed that for example when clicking the Settings button on the room header, the button goes away and focus stays nowhere. It would be nice if the new dialogs / windows / sub trees set their initial focus upon its creation.
[ ] Event tiles in a room timeline are missing accessibility related markup and functionality on the user avatar if there is no image and a default avatar is shown instead. The default avatar is still clickable so I'd say this needs adjusting.
[x] Buttons toggling right pannel content (Members, Files, Notifications) are missing alt text or aria-label as they appear unlabelled.
[x] When there is no image room avatars in the left pannel room list is missing aria-hidden as previously the autogenerated avatar containing first name is exposed.
[ ] When conference call is in progress it should be more noticeable for people using assistive tools. I think modifying window title would be more noticeable or modifying input text place holder to say something like send message (unencrypted, active conference call).
[x] When adding a phone number (including the registration form) the list of countries with their country codes (e.g. +49 for germany, +44 for UK) is not accessible. It is not working like a list it cant be controlled by using the keyboard and it is missing a markup so screen reader users can use and recognize it as interactive list.
[x] Issue sending out read receipts (#5679)
[ ] It is not possible to scrol right pannel content (e.g. notifications) using the keyboard - thanks @grahamperrin https://github.com/vector-im/riot-web/issues/302#issuecomment-297901971
[x] Room sublist items and message tiles in the main content timeline have some kind of dropdown arrow that displays a popup menu when hovering with the mouse. This is not useable with the keyboard nor it is accessible to screen reader users - thanks @pafcu https://github.com/vector-im/riot-web/issues/302#issuecomment-346933049

Again some highlights in no particular order: Incoming messages are turned into list items as @ndarilek proposed a while ago. Users in the member list are focusable and activatable using the keyboard, this was not possible earlier. User menu is also fully accessible it is possible to start new chat, change power levels and other moderation functions. Room header buttons are accessible, this is also new. Dialog buttons, buttons found in the settings and all the context buttons I have noticed so far are accessible. Notifications such as the desktop notifications toggle, Riot update are also accessible.

ara4n commented 7 years ago

@pvagner thanks for all the feedback - and sorry that this has been stuck out on /develop. we're doing a release today however. reopening to cover all the accessibility bulletpoints enumerated above.

pvagner commented 7 years ago

@ara4n Thank you and the whole team for working on this and find chance and expertise to make huge improvements. I was experimenting a bit with this also but this is more complete and polished than what I was hacking here.

pvagner commented 7 years ago

Sorry for the double posting.... @JaniM I'm just trying to understand how all this is working and I am curious how the commit JaniM/matrix-react-sdk@041196d7298d72381b66c7ce2aefa4ed924324ec relates to quick searching. Previously the user avatar seemed to be focusable by changing tabIndex properties the descending container is now focusable. For full treeview like experience we would most likelly need something like active descendants here. Or how this might all be improved?

JaniM commented 7 years ago

@pvagner The commit apparently is missing something and thus has a misleading name. :/ However, adding the tabindexes allows tabbing between rooms after inserting a search term. The tabindex="-1" on TintableSvg was meant to make this easier. It ws a huge oversight that I didn't notice its effect on user avatars.

EDIT: Ahah, the other half of the implementation is in my megacommit to riot-web. https://github.com/JaniM/riot-web/commit/5edb5f6487404b89da45f13f3d8444dd1f533d0f#diff-dc0dba0f86cd6bdf63884cc69d33431d

pvagner commented 7 years ago

Yeah @ara4n has already fixed the Show pannel button in matrix-org/matrix-react-sdk@d567b3bb6a4e8dcef01247794d658ab190d2d908

pvagner commented 7 years ago

Right pannel buttons Members, Files, Notifications are all accessible. It's my bad listing them as inaccessible last week. An enhancement might be turning these into toggle buttons conveying which of these parts is actually showing.

grahamperrin commented 7 years ago

… Notifications are … accessible. …

At, for example, https://riot.im/develop/#/room/#riot:matrix.org/$149292179537449sTbkn:kickass.systems with notifications displayed, in the column to the right, with a scroll bar and scroller present:

I can not find a way to make paging down or up occur in response to Page Down or Page Up keys

– should that be a separate issue?

pvagner commented 6 years ago

I've tried to mark fixed points in my recap comment however unfortunatelly I had to mention some more issues as I've discovered them. https://github.com/vector-im/riot-web/issues/302#issuecomment-276987671

pvagner commented 6 years ago

Huge thanks to @pafcu for #5661 and matrix-org/matrix-react-sdk#1628 I have checkmarked two items in my always updating recap comment.

pafcu commented 6 years ago

We need to reconsider if setting alt text of all avatar images is helping accessibility.

Agree 100%, it's super annoying. However, it's not clear to me what is the best way forward. It seems that the avatar images are usually unnecessary for screen readers, as the display name is almost always shown next to it. One exception is the collapsed left sidebar. It might make sense to hide the avatars from screen readers by default, and make the programmer explicitly make them visible for screen readers when needed.

pafcu commented 6 years ago

@pvagner: You say "all the context buttons I have noticed so far are accessible."

Does this also apply to the button that appears when you hover over a room in the room list (left sidebar)? It opens a menu that allows you to leave the room, set what type of notifications you want, and set the room type (e.g. low priority).

Also, there is a button that appears when you hover over message opens a menu that allows you to redact the message, view the source code, etc.

I did not see anything that would make them accessible now, and my test with a screen reader also did not show them to me, but perhaps I am just missing something.

pvagner commented 6 years ago

with notifications displayed, in the column to the right, with a scroll bar and scroller present:
I can not find a way to make paging down or up occur in response to Page Down or Page Up keys
– should that be a separate issue?

Ooops, I've missed this. I've now verified it and I think page up and page down keys are consumed globally so even if a notification is focused in the right pannel page up and page down keys are still scrolling the timeline within the main content.

Agree 100%, it's super annoying. However, it's not clear to me what is the best way forward. It seems that the avatar images are usually unnecessary for screen readers, as the display name is almost always shown next to it. One exception is the collapsed left sidebar. It might make sense to hide the avatars from screen readers by default, and make the programmer explicitly make them visible for screen readers when needed.

I think this might be my problem not knowing Riot-Web enough so I was not able to give proper suggestions in the past. Originally I have assumed people can just hover over the avatar or a display name and they will be able to find out corresponding mxid. So I was motivated to suggest avatars should have alt text set in order to convey this to assistive technologies. Now I am thinking the same way you are suggesting i.e. don't set an alt text attribute on the room avatars, user avatars unless:

Avatar is displayed alone with no display name next to it.
Clicking the avatar does something else than clicking the display name

I think when left pannel is collapsed the room sublists only display their avatars and at present these are missing alt text or a description or it's not properly wired together as I can't recognize individual rooms when using Riot-Web this way.

Does this also apply to the button that appears when you hover over a room in the room list (left sidebar)? It opens a menu that allows you to leave the room, set what type of notifications you want, and set the room type (e.g. low priority).

No this is not accessible. I know there are some actions associated with left pannel room sublist items however I don't know there is too much in there. I have difficulties coming up with clear straightforward suggestion here. It's awesome how we can currently navigate the room sublists using up and down arrow keys and making this accessible would require we have to add more tab stops for each room.

Also, there is a button that appears when you hover over message opens a menu that allows you to redact the message, view the source code, etc.

The same thing. I can't access that easily. In fact I've asked about this in the #riot-web:matrix.org.

pafcu commented 6 years ago

I can not find a way to make paging down or up occur in response to Page Down or Page Up keys

Personally I think it is easier to find issues when they are all separated but labeled correctly. It also makes it easier to see how common the problems actually are. On the other hand it is a lot of extra work to open separate issues for everything. Perhaps @lampholder can comment on the preferred way to collect issues like this?

I think the underlying issue for the context menus is that we tell users that there is a context menu by displaying a button (that you click to open the menu) when you hover over something. In a classic desktop GUI application you would simply right-click to open the context menu, but this is unfortunately not so common in web applications.

I wonder if there is some standard, accessible, way to tell that there is a context menu associated with an element and how to open it?

pvagner commented 6 years ago

Personally I think it is easier to find issues when they are all separated but labeled correctly. It also makes it easier to see how common the problems actually are. On the other hand it is a lot of extra work to open separate issues for everything. Perhaps @lampholder can comment on the preferred way to collect issues like this?

Originally this seemed just to be me speculating about this eventually trying out some minor modifications. It's why I have put this into a single issue. If we will decide on more collaborative approach with more people joining in then I can try to fill all those bullet points as individual issues.

I think the underlying issue for the context menus is that we tell users that there is a context menu by displaying a button (that you click to open the menu) when you hover over something.

Yes and this is the main problem as howering is either mouse specific or touch specific action thus not available when targetting keyboard navigation.

In a classic desktop GUI application you would simply right-click to open the context menu, but this is unfortunately not so common in web applications.

Analogy to the right click popping up a menu in the desktop apps is shift+F10 or applications key keyboard shortcut. This is not very common but can be seen implemented for mouse as well as keyboard for some web apps e.g. Google docs make extensive use of this.

I wonder if there is some standard, accessible, way to tell that there is a context menu associated with an element and how to open it?

aria-haspopup property can be used to tell users 'hey, interacting with this control pops up a menu or expands a hidden view'. However since the button is hidden until hovering with the mouse only adding this property to the hidden button is not sufficient. I've just found out interesting article on how this is implemented for some versions of Internet explorer. And now while trying to find suitable materials for you to look at I think I am possibly coming up with something that may become usefull at the end:

We should hide the button but not hide it to screen reader users. This will only require a change to the CSS styling.
We should turn that arrow into so called accessibleButton (introduced by @JaniM) and also apply aria-haspopup
We may need to make the menu that pops up react to keypresses and / or apply other markup. I don't know the details my-self yet but here is a working accessible popup menu component for react. How would that fit with matrix-org/matrix-react-sdk ?

pafcu commented 6 years ago

Yes and this is the main problem as howering is either mouse specific or touch specific action thus not available when targetting keyboard navigation.

Actually you can't hover with a touch screen either, which is why I'm not a fan of "reveal on hover" (excecpt IE10 in some cases, as your link shows). Riot might not be optimized for mobile devices, but there are PCs with touch screen too. Not a major issue, but just shows that having proper accessibility does not only help people with disabilities.

We should hide the button but not hide it to screen reader users.

A potential problem is that the screen reader will then announce every single time that there is a button that you can press. At some point a user is bound to go "Yeah, I get it already!". This might not be a huge problem if you are navigating through the room list because you can anyway jump to the next item before reaching that part, but when reading messages (which you usually listen to until the end) you'd end up hearing it a lot. That's why it might be a good idea to hide it, but still allow triggering it with the keyboard. Of course, this would make it difficult for a new user to actually know about this functionality. However, this is the same problem desktop apps have with context menus: usually there is nothing to show that you can right-click, yet people seem to find the menus, so I'm not sure how bad it is.

pvagner commented 6 years ago

A potential problem is that the screen reader will then announce every single time that there is a button that you can press. At some point a user is bound to go "Yeah, I get it already!". This might not be a huge problem if you are navigating through the room list because you can anyway jump to the next item before reaching that part, but when reading messages (which you usually listen to until the end) you'd end up hearing it a lot. That's why it might be a good idea to hide it, but still allow triggering it with the keyboard.

Oh yes, then when considering this the problem is where to register the keyboard handler as ideally you don't care what is focused. Is the focus on the link (i.e. collapsed timestamp), on a mention (avatar or the display name), on the message body ... ? I am not sure it is possible to forward all the children keypresses to the parent div of a timeline event tile. I am avoiding suggesting making the event tile it-self focusable as that will then break focusing its child elements. For providing additional descriptive hints there is an aria-described-by. Here is an article I was able to find on this. For the room sublists the hidden but visible to screen readers button would work, we just need to come up with a solution to the timeline. Edit: There must be a way I guess. For example here on github I can just select a text, disable the browse mode of my screen reader, hit the letter r and the quoted text is added to the comment input entry. Also as I am thinking more about it the button might have no text associated. That way it won't be read by most screen readers when reading text but it will act like a button when focused by using tab or shift+tab to navigate. The fact it has aria-haspopup will be enough as what it really does is that it brings up a popup menu. What do you think?

pafcu commented 6 years ago

ara4n commented 5 years ago

We're hopefully going to have some bandwidth to focus on screen-reader support in the nearish future.

I was wondering whether someone (@pvagner, @ndarilek, @pafcu) could recommend screen reader implementations to test against, so we're not just implementing this by wrote but actually dogfooding it?

ndarilek commented 5 years ago

Thanks, looking forward to that!

On Windows, NVDA is your best bet, while Orca is shipped with most Linux distributions, and Voiceover is built into every modern Mac. There are others, but those are a good representative sample.

ara4n commented 5 years ago

@ndarilek awesome, thanks.

pvagner commented 5 years ago

Hello,

I see some screen reader support improvements are being made these days. It's awesome and amazing, unfortunatelly I can see some high priority issues that might be related to this.

When using tab or shift+tab to navigate, focus is lost when chat input field is about to receive the focus. To reproduce, highlight the Show stickers button and press shift+tab.
In the Settings and Room settings dialogs newly implemented custom controls are not keyboard accessible i.e. it is not possible to navigate to them by pressing tab or shift+tab keys nor it's possible to activate them via the keyboard. I'm talking about the tabs at the top and about the toggle buttons.
When pressing Profile button at the top, nice menu with Settings and Logout entries pops up. It is not possible to activate these items by using the keyboard.
As part of fixing over verbosity of the timeline we have lost access to features such as copying link to selected event, reading event time, activating user details, mentioning an user and read receipts. Pretty much I can't figure out how to interact with timeline events now. I understand this needs fixing but I hope this is still work in progress.

By quickly looking at the timeline using firefox built-in DOM explorer I am afraid you may have missunderstood what aria-hidden does. Everything that has aria-hidden=true won't be visible to screen reader users at all.

I will try to check individual PRs and issues that exist to better understand how this is being implemented. I just would like you all to thank you for this work and point out things I feel are most severe.

turt2live commented 5 years ago

@pvagner thanks for trying it out!

The first two items are bugs which indeed haven't been addressed yet, however the profile dropdown should be keyboard accessible (arrow keys allow you to go up and down, and the menu itself is accessible with CTRL+I).

The timeline is still a work in progress, although the intention was to make it better. It sounds like it's not better. The timestamp and sender should still be accessible (how are you used to activating it?), however read receipts are intentionally disabled. Read receipts were causing problems for people where their screen reader would say things like "Seen by TravisR at ", but no be able to correlate that with the page or event. Fixing it is a lot more complicated, so I'm very curious as to how you may expect to interact with read receipts so we can move towards making them better.

There is heavy usage of aria-hidden in the app now, and a lot of this is to trick screen readers into saying the right thing at the right time. Several parts are read out in duplicate or triplicate normally, and the abuse of aria-hidden is supposed to fix it so it only says things once.

As mentioned, it's definitely a work in progress and the items you've highlighted as high priority are indeed the top of my list too. We're not screen reader users ourselves and can easily miss when we've made things worse for people, so please do continue to report areas which need improvement or simply aren't accessible.

ndarilek commented 5 years ago

Are read receipts visible on the view for an individual message?

I.e. if I click a message in the timeline, can I see a list of who has read it?

That might be an acceptable way to make read receipts visible without spamming screen readers. Not sure to what extent that is possible, but I can't necessarily use all of Riot, so I don't always know what we can currently do with it.

Thanks!

turt2live commented 5 years ago

@ndarilek that sounds like a thing we can do, probably. Not entirely sure but worth experimenting with.

pvagner commented 5 years ago

oh, I have added more thinking to the PR https://github.com/matrix-org/matrix-react-sdk/pull/3019 Okay now I can calm down a bit knowing this is a work in progress and we are free to make suggestions.

turt2live commented 5 years ago

Thank you for that detailed feedback (seriously - it's amazing). I'm not a screen reader user myself and have been trying to use riot by using audio cues only, which is where a bunch of the "fixes" have come from. Based on your feedback, I think I'm doing it wrong - do you have suggestions or hints on how you use a screen reader so I can put myself more in your shoes?

pvagner commented 5 years ago

Well this is very specific. Like normal people screen reader users are also very different. Most simple screen reader users or beginner screen reader users are only using basic keyboard navigation commands that work no matter if screen reader is running or not. i.e. they are using arrow keys, tab and shift+tab and some common keyboard shortcuts built into their operating system like alt+F4 for closing the window, ctrl+w for closing the current tab and similar. Then screen readers have traditionally allowed extra navigation on the web because accessibility was not a thing on the web from the start. For example when considering static content screen readers allow navigation by headings, lists, list items, by form fields in general, or by specific form fields such as text entries, checkboxes or comboboxes. It allows reporting if a particular element on the web is clickable and allows emulating clicks if the accessibility is not all great. That are features and paradigms dedicated screen reader users prefer. Similarly how you can react to a dialog window by interpreting its decoration screen reader power users tend to interupt speech and move on with the interaction if they recognize common spoken feetback. Having said this interactive and accessible controls should be prefered UX. However broken a11y is still better at least for the power users than missing features. So while I don't like to guide you to my screen reader usage patterns too much because I am afraid it might be too subjective there is something I think I might have identified you are doing and I'd like you to consider thinking a bit more about. It looks as if you were trying to kind of dictate to screen readers what they should speak. You are even considering rewording some parts in order to better fit to screen reader interaction for example tweaking the messages to saying something like "user XY has said whatever on whatever time". I see this as a bit unnecessary for example. We don't need to hear that someone has said something in a chat app as it's obvious. We just need to have the content to be there. We are often slower to react as compared to non screen reader users because we need to listen to the screen reader. So it's okay to try to find a good balance. For example if you have a diskette icon and no text on a button we just need a Save alt text for the image as the fact this is a button and the fact it is grayed out is already communicated through accessibility APIs. Also related to this the timeline has content so you should just try to present that content implementing exceptions for features and interactions that are naturally unavailable to screen reader users but you should not change other things. That's for example a hint with the time stamps. If it's displayed on the screen where it makes sense it should stay there for screen reader users too unless it's introducing different issues. Of course automatic reading of incoming text in a chat app is a good additional feature and screen reader users like that. You have started with this feature perhaps thinking it's very important. I think it's great but content presentation is a bit more important. However screen readers and browser combinations are really often causing quirks like you may already have discovered while working on that live region reporting i.e. automatic content presentation of incoming messages trying to elliminate duplicates. What you have found out might still turn very usefull we just need to find a way so the content will stay exposed to screen readers to query. I apologise for repeating some of the things. Please don't take it personally in this last message I am just trying to use what has been discussed earlier as an example to illustrate. I really don't want to sound like I was saying something is going wrong.

pvagner commented 5 years ago

I'm also in #a11y:matrix.org for a live discussion if that might be usefull.

ndarilek commented 5 years ago

One big tip: put away your mouse while using Riot. That won't catch every accessibility issue, but it will catch a few.

As screen reader users, we have shortcuts that don't require us to tab everywhere. But if you can't tab to a given control and activate it via the keyboard, then chances are that it will be more difficult for us to use it.

Thanks! I'll check out the PR as well.

pvagner commented 5 years ago

I have found another nifty example in the riot code to explain my thinking. In the TopLeftMenuButton component we render an AccessibleButton. That button has aria-label set to text "Your profile". However the button has a child element inside which includes user's display name. The fact you have added aria-label saying "Your profile" is not an accessibility issue but an UX issue in general. Now screen readers present this as "Your profile button" if the aria-label would get removed it would be presented like "%{displayname}s button" which is still accessible.

Related to the TopLeftMenu it self. It's a HTML list with individual list items set as focusable by adding tabIndex=0. Other than that it has no other accessibility features. So items can be focused by navigating using tab and shift+tab keys however it would be more natural if using up and down arrow keys was implemented to move from item to item if it should resemble a desktop style popup menu. For screen reader users there is no way to activate individual items in this menu other than emulating mouse clicks. To improve that we'd need similar component to AccessibleButton. The difference to AccessibleButton would be that the role is "menuitem" for the items and role="popup" for the menu container. TabIndex should be -1 and focus management has to be implemented if it has to behave like a desktop style menu. See this for more discussion on a popup menu. Also it should be dismissable by pressing the ESC key. https://stackoverflow.com/questions/41141247/aria-role-menuitem-for-a-or-li

turt2live commented 5 years ago

Thanks for the detailed information - I'm also hiding in #a11y if it's ever easier to send a message there. I'm personally on a different project for the next couple weeks, but will keep the keyboard accessibility in mind and try to come up with better solutions to our accessibility problem. Once I'm back in riot-web land, I'll get them on /develop and visit #a11y for review.

ara4n commented 4 years ago

so @t3chguy and @turt2live have just landed another wave of screenreader work following feedback by @jcsteh at mozilla. feedback would be very welcome as to whether the situation has improved by testing riot.im/develop

ndarilek commented 4 years ago

Nice!

I assume you know about the various buttons that still remain unlabeled, so I won't comment on those.

One minor nit: when announcing rooms, you don't necessarily need to say "It has 5 unread messages." Specifically, the "it has" can probably go, as a) it is fairly intuitive and b) it slows down interaction a bit. Think of it like presenting any other button or visual control. You don't necessarily have to explicitly spell everything out, as users generally quickly figure out intent after a few interactions.

Put differently, "5 unread messages", "5 unreads" or even simply "5" is pretty easy to figure out after an interaction or two, and the "it has" just takes up more audio space.

Thanks again, this is looking good.

element-hq / element-web

Improve keyboard shortcuts & a11y. (Still a bunch remaining) #302