Provide an accessible variant

Created on 1 April 2020, over 4 years ago
Updated 19 January 2023, almost 2 years ago

Hello,

We want to use this module in several sites, but the current output doesn't comply to our accessibility checks.

This is how we arrived to the idea of providing an HTML structure variant (different from the <dl> <dt> <dd>) and also provide a - front-end - accessible JS.

Here is the idea:

  • Keep the back-end functionnality untouched
  • Provide a variant plugin system. In this way we can propose various flavors of the list/titles/descriptions combo with different HTML structures
  • Provide a text filter which will transform the default <dl> <dt> <dd> into the one defined in the selected variant
  • Provide an accessibility compliant JS that will apply to all variants

The coming patch will include a Default variant.

πŸ“Œ Task
Status

Needs review

Version

1.0

Component

Code

Created by

πŸ‡§πŸ‡ͺBelgium ludo.r Brussels

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • First commit to issue fork.
  • πŸ‡ΊπŸ‡ΈUnited States emptyvoid

    I have a working patch for this version of ckeditor_accordion providing aria compliant tags and events. Which was scanned, audited and approved by the US Federal Government.

    I rolled a Patch, built against 8.x-1.x-dev but I'm unsure where to merge or upload it?

  • πŸ‡ΊπŸ‡ΈUnited States emptyvoid

    Patch uploaded

    ckeditor_accordion-provide-accessible-variant.patch

    Demo is inspectable here: https://ncela.ed.gov/facilitating-online-learning

  • Status changed to Needs work over 1 year ago
  • πŸ‡¦πŸ‡ΊAustralia jannakha Brisbane!

    D10/CKEditor 5 returns an error (screenshot attached):

    CKEditor 5 only works with HTML-based text formats. The "CKEditor Accordion" (filter_ckeditor_accordion) filter implies this text format is not HTML anymore.

  • πŸ‡ΊπŸ‡ΈUnited States emptyvoid

    The Standards for the Accessible rules define attributes and values which may not be "generic html" entities. But without it the menu will never validate nor be accepted by the US Federal and State governments requiring 503 compliance.

    https://www.dol.gov/agencies/ofccp/section-503/compliance-assistance

    The current releases including Ckeditor 5 on Drupal 10 is not compliant either.
    But I'm not aware any other menu module is compliant at all either.

    The new architecture for Drupal 10 is significantly different form this build so upgrading the patch would be seriously challenging for those contributes who didn't write the new module version. If anything can we create a feature request for the new version detailing the requirements and referencing the standards it would need to comply with to pass both government and 3rd party auditors?

  • πŸ‡§πŸ‡ͺBelgium ludo.r Brussels

    Using patch #19 (didn't try more recent patches) is producing incorrect HTML.

    It ends up with <html><body>[processed text here]</body></html>.

  • πŸ‡§πŸ‡ͺBelgium ludo.r Brussels

    Here's an adapted version of #19 that fixes the <html><body> issue, however I'm sure there is a safer/better way to do this.
    This should still be improved IMO.

  • πŸ‡³πŸ‡±Netherlands llewellyn.dawson

    I have updated the patch #22 to fix the issues on issue 3368096 πŸ› Every page view generates 10-20 warnings per accordion Active

  • πŸ‡¦πŸ‡ΊAustralia jannakha Brisbane!

    Patch #30: Applied to D10: Getting an white screen of death on http://d10.local/admin/config/content/ckeditor-accordion:

    Uncaught PHP Exception Error: "Class "Drupal\ckeditor_accordion\Form\Html" not found" at /Users/XXX/work/d10.local/web/modules/contrib/ckeditor_accordion/src/Form/CkeditorAccordionSettingsForm.php line 66

    Patch #29:
    can't be applied to v2.0

    * Changing version to v2.0.x-dev as v8 is outdated

  • Status changed to Needs review about 1 year ago
  • πŸ‡΅πŸ‡ΉPortugal rutiolma

    Fixing the error reported at #31

  • First commit to issue fork.
  • πŸ‡©πŸ‡ͺGermany sascha_meissner Planet earth

    Hey, +1 for the issue and the work already done, to get some traction into this i just rebased origin/2.x into the issue fork, applied patch#32 on it (Had to reroll it because it didnt apply to CkeditorAccordionSettingsForm on tip of 2.x) and pushed into issue-fork, see https://git.drupalcode.org/issue/ckeditor_accordion-3124167/-/tree/31241... . Now we have the development in git i will start to review the work already done.

  • πŸ‡©πŸ‡ͺGermany sascha_meissner Planet earth

    So I just realized that the release and tag 2.1.0 was made from the 2.0.x branch and is not reflected within 2.x (which would make sense afais) So right now the issue fork should have the state of latest release tag 2.1.0 + the rerolled and applied patch #32

    Unfortunatly when trying it out it seem to not fully work for me. I followed the steps in updated readme.
    The new libraries css and js files are getting loaded in frontend, i can also choose default variant in ckeditor_accordion settingsform, and add the accordion button to the toolbar in text format without problems, i can create accordions in ckeditor with no problems, but the Filter that should replace the markup is not called at all, i just get the dl, dd, dt markup rendered in frontend. Am I missing something here? Does this have to do with the upstream changes to 2.1.0 ? I try to further investigate ...

  • πŸ‡©πŸ‡ͺGermany sascha_meissner Planet earth

    Did some further a11y testing with browser tools and WAVE, seems to work good, tried using screenreader "pericles" firefox extension which did not read the labels, only the contents but i dont know if thats a referrence.

    Personally i can just add that the focus styles are barely visible, so its hard to see where you are when using keyboard navigation, which is not a big problem for me because iΒ΄m heavily overwriting the css anyway.

  • πŸ‡©πŸ‡ͺGermany sascha_meissner Planet earth
  • πŸ‡©πŸ‡ͺGermany sascha_meissner Planet earth

    PS: I like the idea of the variant-plugin-system introduced here,
    but there needs to be some further work done to be able to actually use it.
    Right now it seems a little bit pointless because the WAI-ARIA-Patterns-And-Widgets library will always get loaded and has checks within itself for the element type, e.g "panel must be div", "heading must be H1-H6" so there is no point in making the element-types configurable when the accordion will not initialize then. In example, a customer of us dont want to use H-elements for the title because the headline-hierarchy differs between pages or places in pages where you can embed an accordion.

    IΒ΄m thinking of extending the VariantPlugin with associated libraries that will be loaded according to the chosen variant.

    Also it would be cool if the WAI-ARIA-Patterns-And-Widgets library would have an option to initialize more than just one row opened.

  • First commit to issue fork.
  • Status changed to Needs work 9 months ago
  • πŸ‡¦πŸ‡ΊAustralia jannakha Brisbane!

    to improve accessibility:

    Header link <a class="ckeditor-accordion-toggler" href="#Accordiontitle" id="Accordiontitle" onclick="return false;" ...> is not really a link and requires proper aria attributes, eg role="button" aria-expanded="true" aria-controls="[id of DD element]"
    to indicate this link controls a dd-element, it's a button and it's expanded

    - if dd is collapsed, a-link element should have the following aria-attributes:
    role="button" aria-expanded="false" aria-controls="[id of DD element]"

    - dd/dl/link ids maybe duplicated if accordion is used in multiple fields on multiple similar structures - so maybe add a random number to ID to make sure it's unique

  • πŸ‡­πŸ‡ΊHungary fox mulder

    Hi everybody!

    We use this patch on a hungarian website, but calling iconv() function with 'ISO-8859-1' value in the second parameter replaces special hungarian characters ( eg. 'Ε‘', 'Ε±' ) to question marks.
    I don't uderstand the code, but why is the iconv() call necessary?

  • πŸ‡³πŸ‡±Netherlands arantxio Dordrecht

    We had an issue where we got the following error:
    Notice: iconv(): Detected an illegal character in input string in /var/www/html/web/modules/contrib/ckeditor_accordion/src/Plugin/Filter/CKEditorAccordion.php on line 59

    This resulted in text not being shown.

    Adjusted it with examples from github and php.net

  • πŸ‡ΊπŸ‡ΈUnited States emptyvoid

    Drupal 10.2.7
    Ckeditor 5
    ckeditor_accordion 2.1.0

    The patch causes a white screen of death on my build.
    Especially once I enable the input filter.

    composer package snippet

    {
                "type": "package",
                "package": {
                    "name": "smillart/wai-aria-patterns-and-widgets",
                    "version": "1.0.6",
                    "type": "drupal-library",
                    "dist": {
                        "url": "https://github.com/digital-analytics-program/gov-wide-code/archive/refs/heads/master.zip",
                        "type": "zip"
                    },
                    "require": {
                        "composer/installers": "^2"
                    }
                }
            }
    

    Ideally the interface of the accordion html would enable simple keyboard control of each accordion.
    https://www.w3.org/WAI/ARIA/apg/patterns/accordion/

    Having built a add-on module for the views_accordion module, having the header be a actual button tag enables basic keyboard control on click behaviors for opening and closing the content panel. Also adding tabindex="0" ensures it can be targeted by keyboard and screen readers.

  • πŸ‡ΊπŸ‡ΈUnited States emptyvoid

    Why is the arrow icon not using a ascii character over multiple before/after styles?

    .ckeditor-accordion-container > dl dt > a > .ckeditor-accordion-toggle:before {
        content: "\2039";
        display: inline-block;
        position: absolute;
        left: 18px;
        -webkit-transform: rotateZ(180deg);  /* Chrome, Opera 15+, Safari 3.1+  */
        -ms-transform: rotateZ(180deg);  /* IE 9 */
        transform: rotateZ(180deg);
    }
    
    .ckeditor-accordion-container dl dt:hover .ckeditor-accordion-toggle:before,
    .ckeditor-accordion-container dl dt.active .ckeditor-accordion-toggle:before {
        left: 12px;
        -webkit-transform: rotateZ(-90deg);  /* Chrome, Opera 15+, Safari 3.1+  */
        -ms-transform: rotateZ(-90deg);  /* IE 9 */
        transform: rotateZ(-90deg);
    }
    
  • πŸ‡ΊπŸ‡ΈUnited States emptyvoid

    hmm,

    I rerolled the patch and had to debug the code as it just didn't seem to be written to actually work. Anyway I fixed the bugs and I have it running on my local build and it's rendering using the ARIA principles!!

    Wahoo!

    Now who can actually commit this as a pull request and or get it releasable?

  • πŸ‡ΊπŸ‡ΈUnited States emptyvoid

    Embraced and extended code design and created a issue for 2.2
    https://www.drupal.org/project/ckeditor_accordion/issues/3470583 πŸ› V2.2 - Accessibility ARIA/508 - D11 compliance patch Needs review

  • First commit to issue fork.
  • πŸ‡§πŸ‡ͺBelgium ludo.r Brussels

    I fixed a bug when text is empty, based on patch #46.

  • πŸ‡ΊπŸ‡ΈUnited States emptyvoid

    As of version 2.1 now I get the following errors in the JavaScript console

    accordion.min.js?v=1.x:2 Uncaught Error: Accordion constructor argument domNode has direct descendant elements that do not match with H2-H6 [data-aria-accordion-heading] or DIV [data-aria-accordion-panel] as required.
        at new o (accordion.min.js?v=1.x:2:7767)
        at accordion.min.js?v=1.x:2:10828
        at NodeList.forEach (<anonymous>)
        at accordion.min.js?v=1.x:2:10805
    

    It would appear that the rendering of the input is somehow being overriden and the attributes are being stripped from the rendered accordion. The accordion still renders and the user can interact via keyboard and mouse.

    But this error is posted to the console on page load.

    Anyone familar with this and or how the new code highjacks the input rendering?

  • πŸ‡ΊπŸ‡ΈUnited States majorrobot

    #49 works for me. Passes Axe Core automated testing. I also tested with keyboard and screenreader, and it all worked.

    I tested with version 2.2 of the module.

    I didn't see the issue in #50 πŸ“Œ Provide an accessible variant Needs review . Going to set this to Needs Review.

  • I'll note there is an open MR attached to this issue with commits from 9 months ago that has already fixed a lot of the issues from the last several comments/patches. I'd suggest anyone with issues test that MR and contribute back instead of creating separate patches.

Production build 0.71.5 2024