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

Remove tabindex attribute from notification alert banner on blur #2014

Conversation

hannalaakso
Copy link
Member

@hannalaakso hannalaakso commented Nov 11, 2020

There is no need for the notification banner component to remain focusable after the page has loaded and the focus has moved away from it. This is following some feedback from the GDS Accessibility clinic: #1999 (comment).

This PR:

  • Removes the tabindex attribute from the notification alert banner on blur
  • Tests that the attribute gets removed correctly
  • Also tests that if the tabindex has been set as an attribute in the HTML, it doesn't get removed by the script.

Partially addresses #2011

This is following some feedback from the GDS Accessibility clinic: #1999 (comment)
There is no need for the component to be focusable after the page has loaded.

Only remove the tabindex attribute if it was set by JavaScript.
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-2014 November 11, 2020 16:53 Inactive
Copy link
Contributor

@36degrees 36degrees left a comment

Choose a reason for hiding this comment

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

Nice 👍🏻

@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-2014 November 16, 2020 10:36 Inactive
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-2014 November 16, 2020 10:40 Inactive
Also test that if the tabindex wasn't added with JavaScript, it doesn't get
removed by the script.
@hannalaakso hannalaakso force-pushed the notification-banner-remove-tabindex-on-blur branch from 345f919 to f7e9b8f Compare November 16, 2020 10:42
@hannalaakso hannalaakso merged commit 1c5b7e3 into feature/notification-banner Nov 16, 2020
@hannalaakso hannalaakso deleted the notification-banner-remove-tabindex-on-blur branch November 16, 2020 10:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

3 participants