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

UHF-8930: Announcement accessibility changes #777

Merged
merged 6 commits into from
Sep 26, 2023
Merged

UHF-8930: Announcement accessibility changes #777

merged 6 commits into from
Sep 26, 2023

Conversation

teroelonen
Copy link
Contributor

@teroelonen teroelonen commented Sep 21, 2023

UHF-8930

What was done

  • Adds javascript that moves focus to previous focusable element when announcement is closed
  • Adds announcement close button title for screen readers
  • Moves close button as first focusable element on announcement

How to install

  • Make sure your instance is up and running on latest dev branch.
    • git pull origin dev
    • make fresh
  • Update the HDBT theme
    • composer require drupal/hdbt:dev-UHF-8930
  • Update the helfi_platform_config module
    • composer require drupal/helfi_platform_config:dev-UHF-8930
  • Run make drush-cr drush-updb drush-cr

How to test

  • You should now have a new field on announcements called Assistive technology close button title and it should be required. Also the title of the node should now say Administrative title that describes the field better.
  • To test this you need of local and global announcements. So create at least two local announcements and make one without a link and one with a link.
  • Create global announcements in front page instance and publish them to what ever instance you are testing the feature on. How-to for the global announcements on local would make this PR long as a book so ask how that can be done if you don't know how.
  • Once you have the announcements set go check that they have now correct source order when focusing elements on keyboard. The close button should be first and if there is a link on the announcement then that should come after.
  • The Assistive technology close button title value should be on the close button span element that the screenreader reads separated with a dash from the Close-text. For example: Announcement about toxic spill - Close alert.
  • Now you need to start closing the announcements and then reopening them and closing again. To reopen closed announcements you need Chrome developer tools open and clear the local storage from the Application tab.
  • First close them in order from top to bottom and make sure the focus always moves to last breadcrumb link.
  • Next close them from bottom to top and make sure that the previous announcement to the closed announcement gets the focus - if it has link the link should get focus, if not the close button should get focus until finally the focus goes to breadcrumb link once again.
  • Next close them in a random order and make sure the above logic is used.
  • Now move to mobile and make sure that the focus moves the same way.
  • Now go to admin/structure/block and disable the breadcrumb block.
  • Now do the same tests again but make sure the focus now goes to last item on the menu on desktop or to the mobile menu toggle on mobile.
  • Now if your last item on the menu didn't have sub-items add it one or the other way around. Now do the desktop tests again and make sure that the correct element is focused (menu-item if no sub-items and menu-item-toggle if there is sub-items)
  • Check that code follows our standards

Designers review

  • This PR does not need designers review
  • This PR has been visually reviewed by a designer (Name of the designer)

Other PRs

…nt when annoucement is closed, add announcement close button title for screen readers, move close button as first focusable element on announcement
…ocus is moved correctly even when there is global announcements on the site
Copy link
Contributor

@Arkkimaagi Arkkimaagi left a comment

Choose a reason for hiding this comment

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

Looking good, works nicely, only a minor nitpick about the use of for-loop in code.

src/js/closable-announcements.js Outdated Show resolved Hide resolved
src/js/closable-announcements.js Outdated Show resolved Hide resolved
src/js/closable-announcements.js Outdated Show resolved Hide resolved
Copy link
Contributor

@Arkkimaagi Arkkimaagi left a comment

Choose a reason for hiding this comment

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

Good job! Approved!

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

Successfully merging this pull request may close these issues.

2 participants