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

ARIA/accessibility fixes for mobile menu and search/typeahead #4456

Conversation

johnnyporkchops
Copy link
Contributor

@johnnyporkchops johnnyporkchops commented Mar 8, 2021

Summary (required)

The work for this PR was originally to address issue 4183 aria-hidden="true"] elements contain focusable descendants on the homepage.

Since the mobile menu and its child global search filed appear on every page of the site, I decided to combine the fixes for these into one PR. This also extended to datable search fields that use typeahead.

Also partially addressed:
#4185 [aria-*] attributes do not have valid values

[aria-*] attributes do not match their roles

[role]s do not have all required [aria-*] attributes


Failing elements and their fixes:

  1. The mobile menu failed aria-hidden="true"] elements contain focusable descendants (4183) because it maintains focusability on child elements when closed(aria-hidden='true')

    • Fix: Use the utility in static/js/accessibility.js to set tabindex to -1 on mobile menu links while ,menu is closed and add tab index to 0 when menu is expanded. ( site-nav.js)
  2. The global site search failed for ARIA IDs are not unique (4186) because there are two search boxes on the page, one being hidden/shown based on screen size. Followup issue to come.

    • Fix: Remove the second site search box that resides in the mobile menu and was hidden/shown based on screen size and instead use the same field for both mobile and desktop by moving the field to the correct location based on screen size. ( site-nav.js)
    • Note: To handle a scenario where a user resizes the screen from mobile to desktop when the menu is left open, I employ the followinng techniques that I hope would have a lower overhead than a window.resize listener:
      MediaQueryList
      matchMedia
      Testing media queries programmatically
      This allows us to handle the two locations of the global searchbox until we can redesign the header to persist one searchbox regardless of screen/device size. not supported inn IE, just giving the user a searchbox in different, but not ugly location the rare scenario that a user resizes with mobile menu open.
  3. The global site search field typeahead dropdown and datable search fields that use the typeahead failed for [aria-*] attributes do not have valid values (4185,) and two that did not have corresponding tickets [aria-*] attributes do not match their roles AND [role]s do not have all required [aria-*] attributes

    • Fix: Add or alter necessary Aria attributes to the site search field and datatable typeahead search fields and remove incorrect Aria attributes on this fields. (typeahead.js, filter-typeahead.js)
  4. Additional fix was also made based on documented ARIA requirements although they may not have triggered a Lighthouse failing flag. The typeahead fields have a role of combobox and require aria-expanded roles to be set based on the state of the typeahead suggestion popup.

    • Fix: Programmatically add aria-expanded tags based on the state of the search field and associated typeahead popup. (typeahead.js, filter-typeahead.js)
  5. Another accessibility issue that was brought to light by this work was the inconsistent tab navigation for the mobile menu which could translate to lack of navigability for screen readers.

    • Fix:This was fixed by moving the toggle to above the menu in the html template, allowing tabbed navigation through the mobile menu, once opened. (navigation.html)
  6. The "More" dropdown checkbox list for Report Time Period on datatables .dropdown__panel starts with aria-hidden=false, but has focusable elements with tabindex=0. This did not get flagged by Lighthouse but seems to violate aria-hidden="true"] elements contain focusable descendants.

  7. Remove tabindex on Mega Menu links to avoid [aria-hidden="true"] elements contain focusable descendants on div#site-menu.site-nav__container.
    Note: This worked in most browsers but made the dropdowns inoperable in Safari so this change was reverted in hotfix:
    [MERGE WITH GITFLOW] Fix Safari bug with desktop-size menu #4495


Impacted areas of the application

modified:   data/templates/layouts/main.jinja
modified:   fec/static/js/modules/dropdowns.js
modified:   fec/static/js/modules/filters/filter-typeahead.js
modified:   fec/static/js/modules/site-nav.js
modified:   fec/static/js/modules/typeahead.js
modified:   fec/static/scss/components/_nav.scss
modified:   fec/templates/base.html
modified:   fec/templates/home_base.html
modified:   fec/templates/partials/navigation/navigation.html

Screenshots

BEFORE:

Screen Shot 2021-03-08 at 11 33 33 AM

AFTER:

Screen Shot 2021-03-08 at 11 33 53 AM

How to test

  • Checkout feature/aria-fixes-for-search-typeahead-mobilemenu
  • npm run-build, ./manage.py runserver
  • Test that site search box gets aria-expanded=true when the typeahead suggestion box is visible and aria-expanded=false when it is collapsed
  • Test that you can tab to , open and tab through the mobile menu
  • Run a lighthouse scan on Home page and a datatable page and confirm that there are no failing ARIA flags on:
    • Site searchbox (.js-site-search.combo__input.tt-input)
    • RECIPIENT NAME OR ID (input#committee_id.combo__input.tt-input)
    • CONTRIBUTOR DETAILS (input#contributor_name.combo__input.tt-input)
  • Test that the global searchbox get appended to the mobile-menu on mobile view and appended back to site header in desktop view.

Note: The mobile toggle button may be missing at certain screen widths on your local because if the env specific banner -- you could resize until you see the button or remove the banner in the inspector


@johnnyporkchops johnnyporkchops requested a review from rfultz March 8, 2021 16:38
@johnnyporkchops johnnyporkchops changed the title ARIA/accessibility fixes for mobile menu and search/typeahead [DO NOT MERGE]ARIA/accessibility fixes for mobile menu and search/typeahead Mar 8, 2021
@johnnyporkchops johnnyporkchops requested review from patphongs and lbeaufort and removed request for patphongs March 10, 2021 00:43
@johnnyporkchops johnnyporkchops changed the title [DO NOT MERGE]ARIA/accessibility fixes for mobile menu and search/typeahead ARIA/accessibility fixes for mobile menu and search/typeahead Mar 10, 2021
@johnnyporkchops johnnyporkchops changed the base branch from develop to release/public-20210316 March 10, 2021 00:53
Copy link
Contributor

@rfultz rfultz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good work, John!
I'll open another accessibility ticket for tabbing through the desktop menu, which seems at least as important as the smallest widths

@rfultz rfultz merged commit f24025c into release/public-20210316 Mar 16, 2021
@rfultz rfultz deleted the feature/aria-fixes-for-search-typeahead-mobilemenu branch March 16, 2021 18:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants