search

backdrop / backdrop-issues

Issue tracker for Backdrop core.
144 stars 39 forks source link

[A11Y] Provide an API for aria-live region update announcements for accessibility improvements #4985

Closed docwilmot closed 8 months ago

docwilmot commented 3 years ago

Description of the need Sister issue on D7: https://www.drupal.org/project/drupal/issues/1915302 Triggering issue on Backdrop: https://github.com/backdrop/backdrop-issues/issues/4941 Read about aria-live here: https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/ARIA_Live_Regions

Specifically, we are looking for ways to make the Layout block add page accessible.

Proposed solution

An API to allow devs to trigger announcements on certain screen events and interactions PR upcoming; ported from D7.

PR by @docwilmot https://github.com/backdrop/backdrop/pull/3557

indigoxela commented 3 years ago

@docwilmot many thanks for porting this important change.

There currently seems to be a problem, though: the div with id "backdrop-live-announce" gets duplicated on events instead of updating it:

duplicate-ids

What seems to be missing is: if (!liveElement) {...} - only guessing, my js foo is limited. :wink:

docwilmot commented 3 years ago

Very true, thanks, fixed.

my js foo is limited

No less than mine I'm sure. My "PRs" are officially considered suggestions for @quicksketch to do the real work.

MustafaDogan78 commented 3 years ago

I don't know English. I'm building a dialect with translation. It's a Problem, but there's no other way. There is another problem with accessibility. Setting permissions. Problem reading roles first, then checkboxes. When using Drupal, the cue box and the roles would be read simultaneously. I figure out what role to allow by counting the tick boxes.

quicksketch commented 3 years ago

Thanks @docwilmot for filing this issue! And thank you @MustafaDogan78 for providing more reports on accessibility problems. Let's use this issue to discuss the new feature for aria-live announcements. I made a new issue at https://github.com/backdrop/backdrop-issues/issues/4987 that we can use to discuss this new problem that you have described regarding the permissions page.

docwilmot commented 3 years ago

The final Drupal version of this includes a debounce to allow multiple consecutive announcements to be queued up and read. I added the denounce function to Backdrop.js instead of a separate debounce file, since there's only one usage of this so far..

Also removed the tabledrag changes, so that we could commit this as a standalone API. We can add implementations as we go along later.

To test then will require pulling to local. Recommend adding a few lines of

    Backdrop.announce('hello world');
    Backdrop.announce('hello another world');
    Backdrop.announce('goodbye world');

to a JS file that listens for some user action (pasting the above in Backdrop.behaviors.layoutDisplayEditor.updateLayout() would fire anytime you drag a block in layout editor, for example) and listen with a screen reader.

Needs milestone and a review then please.

MustafaDogan78 commented 3 years ago 
I can not speak English. I could not understand with translation. I sent the article to my friend who speaks English. He will translate it into Turkish and send it to me. As soon as the answer comes, I will quickly do what you said. I will write to you as soon as possible.

From: docwilmot Sent: Tuesday, March 9, 2021 1:30 AM To: backdrop/backdrop-issues Cc: MustafaDogan78 ; Mention Subject: Re: [backdrop/backdrop-issues] Provide an API for aria-live region update announcements for accessibility improvements (#4985) The final Drupal version of this includes a debounce to allow multiple consecutive announcements to be queued up and read. I added the denounce function to Backdrop.js instead of a separate debounce file, since there's only one usage of this so far..

Also removed the tabledrag changes, so that we could commit this as a standalone API. We can add implementations as we go along later.

To test then will require pulling to local. Recommend adding a few lines of

Backdrop.announce('hello world');
Backdrop.announce('hello another world');
Backdrop.announce('goodbye world');

to a JS file that listens for some user action (pasting the above in Backdrop.behaviors.layoutDisplayEditor.updateLayout() would fire anytime you drag a block in layout editor, for example) and listen with a screen reader.

Needs milestone and a review then please.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

[ { "@context": "http://schema.org", "@type": "EmailMessage", "potentialAction": { "@type": "ViewAction", "target": "https://github.com/backdrop/backdrop-issues/issues/4985#issuecomment-793131194", "url": "https://github.com/backdrop/backdrop-issues/issues/4985#issuecomment-793131194", "name": "View Issue" }, "description": "View this Issue on GitHub", "publisher": { "@type": "Organization", "name": "GitHub", "url": "https://github.com" } } ]

indigoxela commented 3 years ago

@docwilmot I'll definitely need more detailed instructions on how to test the new API. As I already stated, my js foo is limited. the only thing I was able to produce is "Uncaught TypeError: Backdrop.behaviors is undefined".

MustafaDogan78 commented 3 years ago

I made a mistake. I am very sorry. after the misunderstanding, I got this mixed up. http://3531.backdrop.backdrop.qa.backdropcms.org/admin/structure/layouts/update Everything's gone. I hope he has a backup. again, I'm sorry. From: indigoxela Sent: Tuesday, March 9, 2021 10:13 AM To: backdrop/backdrop-issues Cc: MustafaDogan78 ; Mention Subject: Re: [backdrop/backdrop-issues] Provide an API for aria-live region update announcements for accessibility improvements (#4985) @docwilmot I'll definitely need more detailed instructions on how to test the new API. As I already stated, my js foo is limited.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

[ { "@context": "http://schema.org", "@type": "EmailMessage", "potentialAction": { "@type": "ViewAction", "target": "https://github.com/backdrop/backdrop-issues/issues/4985#issuecomment-793474163", "url": "https://github.com/backdrop/backdrop-issues/issues/4985#issuecomment-793474163", "name": "View Issue" }, "description": "View this Issue on GitHub", "publisher": { "@type": "Organization", "name": "GitHub", "url": "https://github.com" } } ]

indigoxela commented 3 years ago

@MustafaDogan78 No problem, no need to excuse. It's currently a bit confusing (also for me), because we have several issues that partly depend on each other. We'll hopefully sort out things soon, so it gets easier for testers to catch up.

MustafaDogan78 commented 3 years ago 
I made unintentional changes to the site. I've updated the layouts. There was a flashback. The developments there are gone. I'm talking about the Test site. the language problem caused it. I am very sorry. I hope he has a backup. I may have ruined your friends ' work.

From: indigoxela Sent: Tuesday, March 9, 2021 10:57 AM To: backdrop/backdrop-issues Cc: MustafaDogan78 ; Mention Subject: Re: [backdrop/backdrop-issues] Provide an API for aria-live region update announcements for accessibility improvements (#4985) @MustafaDogan78 No problem, no need to excuse. It's currently a bit confusing (also for me), because we have several issues that partly depend on each other. We'll hopefully sort out things soon, so it gets easier for testers to catch up.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

[ { "@context": "http://schema.org", "@type": "EmailMessage", "potentialAction": { "@type": "ViewAction", "target": "https://github.com/backdrop/backdrop-issues/issues/4985#issuecomment-793507048", "url": "https://github.com/backdrop/backdrop-issues/issues/4985#issuecomment-793507048", "name": "View Issue" }, "description": "View this Issue on GitHub", "publisher": { "@type": "Organization", "name": "GitHub", "url": "https://github.com" } } ]

indigoxela commented 3 years ago

I've updated the layouts. There was a flashback. The developments there are gone.

@MustafaDogan78 don't worry. Every time a pull request gets updated, the test box gets flushed. The developer reverted some of the initial changes, so this pull request only provides the API without any actual implementation. This means, that you can't test the new functionality here anymore. Nothing's broken. We'll inform you and other testers, as soon as there's some actual functionality to test on the issues, that will actually use this API.

docwilmot commented 3 years ago

I forgot to mention one important thing for testing: announce library has to be loaded. Add backdrop_add_library('system', 'announce'); in layout_content_form().

It may be reasonable to just add the Backdrop.announce function to Backdrop.js to prevent this requirement. I'll have to see what @quicksketch suggests as I'm not sure of the implications.

Reading the issue, D8/9 initially did have announce in Drupal.js but moved it out when they recognized it needed debounce, as debounce was already in its own file and it wouldn't do to have Drupal.js dependent on debounce.js. But if we're starting off creating both files, we could just add both to Backdrop.js from the start, I'd think.

indigoxela commented 3 years ago

I forgot to mention one important thing for testing: announce library has to be loaded.

@docwilmot I was aware of that. My testing setup is as such (I used Seven for a quick check):

The content of helper.js:

(function ($) {

  $(document).ready(function() {
      $('.form-type-checkbox input').on('change', function () {
        var notifytext = $(this).next('label').text() + ' has changed.';
        if ($(this).prop('checked') == true) {
          notifytext += ' New value is "checked".';
        }
        else {
          notifytext += ' New value is "unchecked".';
        }
        Backdrop.announce(notifytext);
      });
  });

})(jQuery);

This is not supposed to be a real-life example, but I expected it to work. But...

I opened /node/1/edit and checked/unchecked one of the checkboxes. And in the browser console I got:

Uncaught TypeError: Backdrop.behaviors is undefined
Uncaught TypeError: Backdrop.announce is not a function

Only if I edit announce.js and add on top: Backdrop.behaviors = {}; These errors go away and the div with id "backdrop-live-announce" finally gets added, but then when I check/uncheck a checkbox I get: Uncaught TypeError: debounce is not a function After changing return debounce(announce, 300)(); to return Backdrop.debounce(announce, 300)();, the announcements work.

What am I missing? (Obviously I'm not very smart with Javascript namespaces.)

indigoxela commented 3 years ago

What am I missing?

Now I know - it's the order in which the js files get loaded. The trouble I had was caused by the fact, that the file "announce.js" has been loaded even before "backdrop.js".

My sample code from above works without console nagging, if I slightly adapt the library definition to add the announce library to a different group (JS_DEFAULT) - which is loaded later. Not sure, if that's a valid solution.

Some other thoughts:

docwilmot commented 3 years ago

Wow, you certainly found a thorough way to test this. I've implemented your suggestions.

indigoxela commented 3 years ago

One last question: In function announce() the "aria-busy" attribute gets set to true and instantly to false again.

      liveElement.setAttribute('aria-busy', 'true');
      liveElement.setAttribute('aria-live', priority);
      liveElement.innerHTML = text.join('\n');
      liveElement.setAttribute('aria-busy', 'false');

Will screen readers pick this up quickly enough? I'm asking, because I can't properly test that yet (I didn't manage yet to get Orca working on my Linux and don't have Windows, so can't use NVDA.)

MustafaDogan78 commented 3 years ago

We can test it together. Send me the draft and I'll set up a yee. I'm also looking at the code you wrote. E-mail: @.*** From: indigoxela Sent: Thursday, March 11, 2021 9:22 AM To: backdrop/backdrop-issues Cc: MustafaDogan78 ; Mention Subject: Re: [backdrop/backdrop-issues] Provide an API for aria-live region update announcements for accessibility improvements (#4985) One last question: In function announce() the "aria-busy" attribute gets set to true and instantly to false again.

  liveElement.setAttribute('aria-busy', 'true');
  liveElement.setAttribute('aria-live', priority);
  liveElement.innerHTML = text.join('\n');
  liveElement.setAttribute('aria-busy', 'false');

Will screen readers pick this up quickly enough? I'm asking, because I can't properly test that yet (I didn't manage yet to get Orca working on my Linux and don't have Windows, so can't use NVDA.)

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

[ { @.": "http://schema.org", @.": "EmailMessage", "potentialAction": { @.": "ViewAction", "target": "https://github.com/backdrop/backdrop-issues/issues/4985#issuecomment-796491982", "url": "https://github.com/backdrop/backdrop-issues/issues/4985#issuecomment-796491982", "name": "View Issue" }, "description": "View this Issue on GitHub", "publisher": { @.": "Organization", "name": "GitHub", "url": "https://github.com" } } ]

indigoxela commented 3 years ago

We can test it together. Send me the draft and I'll set up a yee. I'm also looking at the code you wrote.

@MustafaDogan78 many thanks for your offer, but I'm afraid there's not much to actually test here. My example code, which I used locally, was only meant to verify that the new API will be working. That seems to be the case.

If you want to test locally, you could adapt my code example. Make sure that you have the latest code changes from the pull request on https://github.com/backdrop/backdrop/pull/3557.

Then you can load the library in a theme or module with, for instance, backdrop_add_library('system', 'backdrop.announce'). And then you should be able to use Backdrop.announce('Your announce text'); in your custom Javascript code. Does this help in any way?

MustafaDogan78 commented 3 years ago 
it'll be as you say. Screen reader works well with Aria. if a screen reader cannot work with aria, the problem is with the screen reader.

From: indigoxela Sent: Thursday, March 11, 2021 9:22 AM To: backdrop/backdrop-issues Cc: MustafaDogan78 ; Mention Subject: Re: [backdrop/backdrop-issues] Provide an API for aria-live region update announcements for accessibility improvements (#4985) One last question: In function announce() the "aria-busy" attribute gets set to true and instantly to false again.

  liveElement.setAttribute('aria-busy', 'true');
  liveElement.setAttribute('aria-live', priority);
  liveElement.innerHTML = text.join('\n');
  liveElement.setAttribute('aria-busy', 'false');

Will screen readers pick this up quickly enough? I'm asking, because I can't properly test that yet (I didn't manage yet to get Orca working on my Linux and don't have Windows, so can't use NVDA.)

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

[ { @.": "http://schema.org", @.": "EmailMessage", "potentialAction": { @.": "ViewAction", "target": "https://github.com/backdrop/backdrop-issues/issues/4985#issuecomment-796491982", "url": "https://github.com/backdrop/backdrop-issues/issues/4985#issuecomment-796491982", "name": "View Issue" }, "description": "View this Issue on GitHub", "publisher": { @.": "Organization", "name": "GitHub", "url": "https://github.com" } } ]

indigoxela commented 3 years ago

In the meantime I've been able to get Orca on my Linux (more or less) working. I could verify, that it properly picks up changes.

I'd say: works for me. I've left a suggestion to add some info to the doc block, about how to include the library. What should be our next steps?

quicksketch commented 3 years ago

@docwilmot has suggested that we include this in the next bugfix release (1.18.2 if possible) because it will help resolve the accessibility problems with Layout module (see https://github.com/backdrop/backdrop-issues/issues/4941).

Although this is a new API and feature, we make exceptions for UX improvements that they can be made at any time (as long as they don't have compatibility impacts). I think this falls under that umbrella of exceptions.

I think the next step here is to merge this issue, so that #4941 can utilize it. What do you folks think?

quicksketch commented 3 years ago

I added docblocks to both Backdrop.debounce() and Backdrop.announce(), and added @since 1.18.2 indicators to clarify when this API became available.

We're not yet using this functionality but it is now in place for further enhancements.

I have merged https://github.com/backdrop/backdrop/pull/3557 into 1.x and 1.18.x. Thanks @docwilmot, @indigoxela, and @MustafaDogan78!

quicksketch commented 3 years ago

I have made a follow-up to integrate this new feature with the block UI: https://github.com/backdrop/backdrop-issues/issues/5021

klonos commented 3 years ago

I'm re-opening this issue and adding the label for a change record, so that it's within our radar to do - but still keeping the "status - fixed" label 😉

herbdool commented 8 months ago

Change record https://docs.backdropcms.org/change-records/new-api-to-allow-for-aria-live-regions-for-announcing-ui-changes-aurally