Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[A11Y] Use Backdrop.announce() when using Block drag and drop rearrangement #5021

Closed
quicksketch opened this issue Mar 24, 2021 · 18 comments · Fixed by backdrop/backdrop#3670

Comments

@quicksketch
Copy link
Member

quicksketch commented Mar 24, 2021

Description of the need

In #4985 we added the new Backdrop.announce() method, but we are not yet using it. It was added to make the drag and drop of blocks accessible via screenreader, now that it is accessible via keyboard (see #4941).

Proposed solution

Backdrop.announce() should be used to describe the new position of a block and what region it is in after a block has changed position (whether by mouse or keyboard).

As an accessibility improvement, I this can qualify for any bugfix release.

@docwilmot
Copy link
Contributor

Maybe we need a META to list all the places we should be using this. E.g:

  • Layout block add pages
  • Dialogs launching ot closing
  • Tabledrag table rows being dragged
  • Batch API runs (to announce completion/errors etc)
  • Messages?
  • etc?

What say y'all? @klonos?

@indigoxela
Copy link
Member

@docwilmot IMO a meta issue makes sense. You already identified a bunch of places where the announce API will improve accessibility - there may be more.

@docwilmot
Copy link
Contributor

First attempt to continue this.

@indigoxela
Copy link
Member

@docwilmot many thanks for reviving this issue!

There are several test failures - I'd assume that some xpath patterns need an update? Didn't dig any deeper yet, though...

@docwilmot
Copy link
Contributor

Thanks @indigoxela. Will check ASAP
A brief explanation of the changes in the PR.

  • Added a function in JS file that announces
    • when a block is moved
    • if its moved to a new region
    • its position in the region
  • Added an aria-label to draggable blocks; this is because previously screenreaders on focus announced the entire block content (title, the block description, the dropbutton, the block conditions) immediately, and would do the announce last, which is too long to wait IMO. Once the aria label is added though, only the block title is said, then the announce.
  • Added a tabindex to the block content, so that now after the title and announce, if you want to hear the description etc, you can tab to it.

@indigoxela
Copy link
Member

@docwilmot the code per se looks good. I'm having trouble with testing, though. Things do get announced when a block got moved with page-up / page-down. There's some odd announcement "empty" whenever I move a block. Not sure if that's because of my screen reader (Orca) or if it's intended or not intended... 😕 Also, after moving a block a second time, the full block info gets announced (every form item inside that block and the position gets announced last)... that's probably as intended?

I have no idea, how to move a block to a different region... I know, that's not part of this issue. We probably didn't implement that? For testing we definitely need someone more familiar with browsing with screen readers. I'm not a big help here.

@MustafaDogan78
Copy link

MustafaDogan78 commented Jul 19, 2021 via email

@indigoxela
Copy link
Member

Hi @MustafaDogan78,

that's great!

Go to https://pr3670-w3xzxc22xssyd5ugdb4gb5mdsm56dlmg.tugboat.qa/ (you may have to wait for a few seconds, until the sandbox resumes).
The username is admin, the password is fa280d54d5f5

After logging in, go to https://pr3670-w3xzxc22xssyd5ugdb4gb5mdsm56dlmg.tugboat.qa/admin/structure/layouts/manage/default and move some blocks. After moving a block, you should get the information about where the block got moved.

To move a block up or down in a region, use page-up and page-down. To move it to the next or previous region use the right and left arrow keys.

@MustafaDogan78
Copy link

MustafaDogan78 commented Jul 20, 2021 via email

@indigoxela
Copy link
Member

@MustafaDogan78 thank you very much for verifying that it works as it should.

It's so important, that someone familiar with browsing with a screen reader tests these things. I could at most tell that it does something, but by no means if it does the right thing. Again, your testing is highly appreciated!

@docwilmot I'll do a final code review soon, I need a "quiet hour" for that, as Javascript isn't really in my comfort zone.

@MustafaDogan78
Copy link

MustafaDogan78 commented Jul 20, 2021 via email

@indigoxela
Copy link
Member

@docwilmot I've left a comment with a small suggestion - totally optional. The code's looking good to me.

@indigoxela
Copy link
Member

@docwilmot did you already find the time to read my comment?
If so, what's your conclusion?

@jenlampton jenlampton changed the title Use Backdrop.announce() when using Block drag and drop rearrangement [A11Y] Use Backdrop.announce() when using Block drag and drop rearrangement Aug 16, 2021
@laryn laryn added this to the 1.19.4 milestone Aug 27, 2021
@docwilmot
Copy link
Contributor

This has been tested and works, but there are many areas in Backdrop where we could implement the new announce functionality. In most cases this will require both PHP and JS changes. I'm wondering if need a standard formula for creating/formatting/announcing, instead of doing it piece by piece.

For example this PR we directly call announce() with concatenated strings built in JS. Seems fine to me TBH. But any JS expert or A11y expert think this could be done otherwise?

Having said, this is ready for code review since its already tagged WFM.

@indigoxela
Copy link
Member

we directly call announce() with concatenated strings built in JS. Seems fine to me TBH

I see no problem with this, too. I'm not sure if we could provide a generic "formula", that fits each and every use-case regarding label - if that's what you have in mind.

Having said, this is ready for code review since its already tagged WFM.

Looks good to me. Switching to backdrop_attributes() makes the code a lot easier to read. Many thanks for working on this, RTBC from my side.

@MustafaDogan78
Copy link

MustafaDogan78 commented Aug 31, 2021 via email

@indigoxela
Copy link
Member

@MustafaDogan78 that should be OK, no new functionality to test, the most recent change only improved code readability.

All the best for your mom, hopefully she gets well soon!

@quicksketch
Copy link
Member Author

I merged this into 1.x and 1.19.x. Thanks @docwilmot, @indigoxela, and @MustafaDogan78!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants