Overlays are inaccessible

[Gittenburg]

Easy (good first issues):

• [ ] Use role="dialog". PR in progress: #16471
• [ ] Label them with aria-label or aria-labelledby. PR in progress: #16471
• [x] "Invite users" popup does not focus first element. there is a PR: #16084

More difficult:

• [ ] Prevent tab and shift + tab from focusing elements in the background, i.e. the focus should wrap around.
• [ ] Closing the overlay should return the focus to the previously focused element. So e.g. opening the drafts with d or the help with ? should remember document.activeElement and closing the overlay should focus it again. For overlays opened with the settings dropdown, changing the focus back should also work (which means opening the settings dropdown with g must already remember the focus and should restore it on Escape, no matter if you close the dropdown without selecting something or if you open the settings and close the overlay).

Fixing these problems is very much appreciated! Just reply to this issue on which part you want to work.

Resources:

• https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Roles/dialog_role
• https://bitsofco.de/accessible-modal-dialog/
• https://alastairc.uk/gists/7340946/PopOvers-Content.html

[abhijeetbodas2001]

• [ ] "Invite users" popup does not focus first element.

I would like to work on this part.

[abhijeetbodas2001]

I've made the emails field focus when user opens the "invite users" window. But what is to be done after the user clicks the invite button?

• If the data is valid, focus on the emails field or focus nowhere?
• If data is invalid, focus on the emails field or focus on the field which is invalid?

[Gittenburg]

I don't think we need to change the focus after submitting the form. Another accessibility problem I however just noticed is that we are forcing keyboard users to tab through every stream checkbox to reach the Invite button. We should instead support submitting the form by pressing Ctrl+Enter (like we do for compose).

I also just noticed a bug: Open the compose box and then the Invite more users popup. If you now click on Check all or Uncheck all the compose box textarea is focused (and you can type there).

[abhijeetbodas2001]

I've done Ctrl+Enter to submit part. One thing I noticed is that after the form is submitted, it does not scroll up to the top of the window. Is this to be done too? (Since this would directly show any error/success alerts which appear at the top after submitting, without the need to scroll)

[Gittenburg]

Good catch, that's indeed a UX problem. We don't want to just scroll without changing focus though (because then the focused element can be out of view), so I now like your initial proposal, i.e. after submitting the form we scroll to the top of the modal (so that you can see the success or error message) and focus the email input, assuming it is the only input that can lead to errors (so that you can invite more users or correct an error).

I created #16077 for further coordinating your work on the Invite users modal.

[sumit-kushwah]

@zulipbot claim

[zulipbot]

Welcome to Zulip, @sumit-kushwah! We just sent you an invite to collaborate on this repository at https://github.com/zulip/zulip/invitations. Please accept this invite in order to claim this issue and begin a fun, rewarding experience contributing to Zulip!

Here's some tips to get you off to a good start:

• Join me on the Zulip developers' server, to get help, chat about this issue, and meet the other developers.
• Unwatch this repository, so that you don't get 100 emails a day.

As you work on this issue, you'll also want to refer to the Zulip code contribution guide, as well as the rest of the developer documentation on that site.

See you on the other side (that is, the pull request side)!

[Gittenburg]

@sumit-kushwah which part of this issue do you want to work on?

[zulipbot]

Hello @sumit-kushwah, you have been unassigned from this issue because you have not updated this issue or any referenced pull requests for over 14 days.

You can reclaim this issue or claim any other issue by commenting @zulipbot claim on that issue.

Thanks for your contributions, and hope to see you again soon!

[tareq12345]

@zulipbot claim

[tareq12345]

@Gittenburg Hi I want to work on the easy part I'm a beginner and I want to contribute I just don't know which file to modify

[Gittenburg]

A good way to find the source code for some frontend element is to right-click on the element in your browser and choose Inspect in the context menu. Then in the inspector of the developer console look around the HTML for some unique id or class and then do a git grep "whatyoufound" in the cloned repository.

[Gittenburg]

Since we want to add the role="dialog" and aria-label to all overlays, it however probably makes more sense to do this in the open_overlay function in static/js/overlays.js, instead of editing each handlebars template individually.

[tareq12345]

@Gittenburg so basically what I have to do is opts.overlay.attr("role","dialog") andopts.overlay.attr("aria-labe") I want to test my code

[Gittenburg]

Yeah, that would work for role=dialog. The label however should be set to something descriptive. I initially thought we could just use opts.name for this but just realized that this wouldn't be ideal since it isn't translated and aria-label should be translated for non-English users. (In the Handlebars templates the translation is done with the t function e.g. {{t 'Streams' }}).

So I guess the simplest way is to manually add role=dialog and arial-label="{{t 'whatever the title is'}}" to our nine overlays (which you can find with git grep data-overlay=). For consistency I think it makes sense to always add both attributes to the div that has the data-overlay attribute.

[Gittenburg]

Actually I think we should prefer aria-labelledby as shown on MDN (for this you will need to add ids to the h1 tags in the overlays).

[tareq12345]

okay got you so one example would be aria-labelledby={{t 'Drafts' }}, I assigned it the value in h1 and add the attribute id={{t 'Drafts' }} toh1

[Gittenburg]

No the ids shouldn't be translated (since they are never shown to users). An example would be:

<div role="dialog" aria-labelledby="draftsTitle" data-overlay="drafts">
  <h1 id="draftsTitle">{{t 'Drafts'}}</h1>

[tareq12345]

@Gittenburg can you review if I need to add anything

[shubham00jain]

Hey @Gittenburg is the role=dialog issue fixed? If not I would like to collaborate with @tareq12345 on this.

[diwanshu8850]

@Gittenburg I would like to work on this issue Label them with aria-label or aria-labelledby. Please guide me if this is not fixed already.

[Gittenburg]

I think @tareq12345 is already doing both of these in the PR #16471, and there is not much room for collaboration there, so you should probably take on some other issue.