Problem/Motivation

There are few acessibility issues with the accordion title:

  1. Screenreader - Accordion title is announced as link, when it is behaving as a button.
  2. Screenreader - Accordion title is missing aria-expanded to indicate visibility of content.
  3. Keyboard - Pressing spacebar while the title is active should toggle the visibility of the content.

There is a similar issue open #3124167: Provide an accessible variant, but don't believe this is a duplicate. The focus of that issue is adding a variant system, with an optional accessible variant. However, accessibility improvements should be the default and not an optional add-on. I opened this issue to add the accessibility improvements in a backwards-compatible way.

Steps to reproduce

Keyboard:

  1. Tab to an accordion title and press the spacebar. The content doesn't toggle.

VoiceOver in Safari:

  1. Navigate to the accordion title, it's announced as a link
  2. Navigate to the accordion title and activate it. There's no feedback that content is revealed.

Proposed resolution

  • Add role="button" to title link
  • Add aria-expanded to title link, and update it based on accordion state
  • Toggle accordion when space bar is pressed on link

For the next major version it should be updated to use the semantic <button> element for the title.

CommentFileSizeAuthor
#11 3424786-11--mr18.patch14.16 KBsonfd
#6 ckeditor_accordion_aria_expanded.patch16.94 KBtiagoz

Issue fork ckeditor_accordion-3424786

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

hbrokmeier created an issue. See original summary.

hbrokmeier’s picture

hbrokmeier commented
Version: 2.x-dev » 2.0.x-dev

hbrokmeier opened merge request !15

hbrokmeier’s picture

hbrokmeier commented
Status: Active » Needs review

Merge request submitted with the proposed changes:

  • Add role="button" to title link
  • Add aria-expanded to title link, and update it based on accordion state
  • Toggle accordion when space bar is pressed on link
tiagoz’s picture

tiagoz
Primary language Portuguese, Portugal
commented

There is already a related issue with patches https://www.drupal.org/project/ckeditor_accordion/issues/2869510

tiagoz’s picture

tiagoz
Primary language Portuguese, Portugal
commented
StatusFileSize
new ckeditor_accordion_aria_expanded.patch16.94 KB

sonfd made their first commit to this issue’s fork.

sonfd opened merge request !18

sonfd’s picture

sonfd
he/they
Location Portland, ME
commented
Version: 2.0.x-dev » 2.x-dev

I think the approach in the MR is more complete than the patch in #6. Also the patch contains a number of whitespace issues. MR !18 is the same as MR !15, but on top of the 2.x branch which seems to be the current development branch.

I'm concerned that the related issue referenced in #6 has been open for 8 years.

hipp2bsquare made their first commit to this issue’s fork.

sonfd’s picture

sonfd
he/they
Location Portland, ME
commented
StatusFileSize
new 3424786-11--mr18.patch14.16 KB

MR 18 updated to apply to the latest 2.x dev.

Attaching patch equivalent of MR 18 for use with composer.

sclsweb made their first commit to this issue’s fork.

sclsweb’s picture

sclsweb commented

#11 and #12 fix the following error (using the current version, 2.3.0, with #10) that I kept seeing in this scenario:

  • The first item would be open on page load by default
  • When another item was selected (leaving the first item still open), the toggle wouldn't work right, and this error would appear in the console: "Uncaught TypeError: can't access property "setAttribute", e.querySelector(...) is null"

What remains to get this merged and committed? My org is working to make all our sites WCAG 2.1 Level AA compliant. In order to do that we'll have to be using this patch in production sites, or abandoning CKEditor Accordion altogether, or switching to something like CKEditor Details Accordion. Would love to see the changes in this issue become a standard feature and part of the next commit.

whiplashomega’s picture

whiplashomega commented

applied this patch on our site to improve accessibility. It caused errors due to these lines (39 and 40 in the patch) whenever you click on another accordion title when one is already expanded.

+          activeChild.querySelector('.ckeditor-accordion-toggler').classList.remove('active');
+          activeChild.querySelector('.ckeditor-accordion-toggler').setAttribute('aria-expanded', false);

It appears to be because the forEach loop is hitting both the DT and DD tags, but the DD tags do not have any children with the class ckeditor-accordion-toggler, and thus the querySelector function returns null.

Altering those lines to use the conditional operator fixed the issue:

+          activeChild.querySelector('.ckeditor-accordion-toggler')?.classList.remove('active');
+          activeChild.querySelector('.ckeditor-accordion-toggler')?.setAttribute('aria-expanded', false);