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-label class not properly hidden #606

Open
hanshillen opened this issue Nov 9, 2024 · 6 comments
Open

aria-label class not properly hidden #606

hanshillen opened this issue Nov 9, 2024 · 6 comments

Comments

@hanshillen
Copy link
Contributor

hanshillen commented Nov 9, 2024

Subject of the issue/enhancement/features

The .aria-label class is supposed to visually hide content while keeping it available to screen readers. But under certain conditions, that content is still:

  1. Selectable, and visible when selected.
  2. Blocking mouse clicks.

This seems to be specifically occurring on Windows, for example in Chrome. The issue is made worse if the user has set a minimum font-size in their browser settings, as that makes the hidden text bigger and more likely to have a negative effect.

Your environment

  • 5.41.12
  • Windows, Chrome (doesn't occur on MacOS)

Steps to reproduce

  • Create a new course with adapt create and open it in Chrome on Windows
  • Optionally, increase Chrome's minimum font size through Settings > Appearance > Customize fonts > Minimum Font size
  • Go to the 'Question Components' page
  • Select all page content with mouse or Ctrl + a

Expected behaviour

The visually hidden text should not be visible at any point

Actual behaviour

Visually hidden text is visble

Screenshots (if you can)

The screenshot below shows a question component with all text content selected. This makes the following aria-label text visible:

  • Completion status ("completed", .js-a11y-completion-description)
  • Component type ("Multiple choice question")
  • Feedback live region text, ("You answered incorrectly", .js-btn-marking-label). Since this text is positioned on top of the "show markings" button, it makes that part of the button unclickable with a mouse.

Notes

It might be easiest replacing the current aria-label class:

.aria-label {
position: absolute !important;
&.relative {
position: relative !important;
}
left: 0 !important;
width: auto !important;
height: auto !important;
overflow: auto !important;
color: rgba(0,0,0,0) !important;
background: rgba(0,0,0,0) !important;
font-size: 1px !important;
padding: 0 !important;
margin: 0 !important;
line-height: normal !important;
z-index: 1;
}

with something more inline with other frameworks, such as bootstrap's visually-hidden:

.aria-label{
  display: block;
  width: 1px !important;
  height: 1px !important;
  padding: 0 !important;
  margin: -1px !important;
  overflow: hidden !important;
  clip: rect(0, 0, 0, 0) !important;
  white-space: nowrap !important;
  border: 0 !important;
}

This is a commonly used and well tested approach for visually hiding content, and the results of many years of the accessibility community trying out different approaches. It does not have the reported side effect.

@oliverfoster
Copy link
Member

oliverfoster commented Nov 10, 2024

I don't know that that'll work on iPads with voiceover. Are you able to test? Otherwise looks good.

The reason we have absolute with the full height and width and opacity 0 etc is so that the text is all available for the screen reader on iOS (occluded/overflow text did not used to read) and in the correct location, but also so that it doesn't occupy block space and isn't visible to sighted users.

@hanshillen
Copy link
Contributor Author

Yeah Safari and VoiceOver have often caused issues for visually hiding content, which is why the technique has been revised over the years.

I've tested this on iOS with VoiceOver, and it's working correctly.

If we do end up using Bootstrap's styles, I suggest we also use their .visually-hidden-focusable class, to allow for visually hiding focusable elements that become visible on focus. This could then be used to fix #607.

If that sounds OK I can submit a PR.

@oliverfoster
Copy link
Member

oliverfoster commented Nov 12, 2024

Please don't include the backbone libraries, just use the resultant styles to mimic what they've done.

It should be two separate prs and issues, one for the aria-label and please contribute and reference the existing issue for the skip navigation button (you may need to open some others the skip to transcript button in the media, youtube and vimeo plugins?).

The display: block; width: 1px !important; height: 1px !important; padding: 0 !important; margin: -1px !important; seems to work nicely, at least with nvda, to allow the screen reader cursor to read the skip navigation button.

You seem to have three issues here with the .aria-label class.

  1. Selectable
  2. Visible when selected
  3. Blocking mouse clicks

Proposed code:

.aria-label{
  display: block;
  width: 1px !important;
  height: 1px !important;
  padding: 0 !important;
  margin: -1px !important;
  overflow: hidden !important;
  clip: rect(0, 0, 0, 0) !important;
  white-space: nowrap !important;
  border: 0 !important;
}

I can see that the backbone styling would stop the element from blocking mouse clicks and also stop text from showing when a font size is forced, because of how it sets the size of the element. There is however, no mechanism that stops these elements being selectable, so it's possible to select, copy and paste the text still.

It might be a good idea to include pointer-events: none; for issue 3 and to include user-select: none; for issue 1? The code as is works for issue 2.

@hanshillen
Copy link
Contributor Author

It might be a good idea to include pointer-events: none; for issue 3 and to include user-select: none; for issue 1? The code as is works for issue 2.

It's hard to predict whether this desirable to the user or not. If a screen reader user is selecting a section of course content that contains visually hidden text for their benefit, then that content would probably be useful to them after the copy as well. especially since this is how the aria-label class has worked up to now. For sighted users, the added text would probably be unexpected and potentially annoying, but again that's how the aria-label class has historically worked so there wouldn't be any change there.

I think the safest option is to leave the text selectable, but if you feel otherwise we can add those properties (a screen reader user could always copy the complete contents from their screen reader's speech history).

It should be two separate prs and issues, one for the aria-label and please contribute and reference the existing issue for the skip navigation button (you may need to open some others the skip to transcript button in the media, youtube and vimeo plugins?).

Core still needs a dedicated class for visually hidden focusable elements that become visible on focus, as this is a pretty common use-case that would benefit from consistency across the framework. This class could then be used by plugins, either by adding the class explicitly or by using it as a mixin.

Finally, I think it would be good to add a visually-hidden alias for aria-label. Bootstrap originally called their class 'sr-only', but changed it to 'visually-hidden' because the class is relevant to other (assistive) technologies besides just screen readers. 'aria-label' is even more of a misnomer, as it has nothing to do with wai-aria and it's not immediately clear what the class does. Calling it 'visually-hidden' clearly conveys what the class does, and is in line with general recommendations from the a11y community. We could keep aria-label as a deprecated class for backwards compatibility, and recommend visually-hidden moving forward.

With all of the above in mind, my proposed code to core would be:

.aria-label, .visually-hidden {
  display: block;
  width: 1px !important;
  height: 1px !important;
  padding: 0 !important;
  margin: -1px !important;
  overflow: hidden !important;
  clip: rect(0, 0, 0, 0) !important;
  white-space: nowrap !important;
  border: 0 !important;
}

.aria-label-focusable, .visually-hidden-focusable {
 &:not(:focus):not(:focus-within) {
    .visually-hidden
  }
}

@oliverfoster Let me know if this is OK or if you disagree.

Then I can go ahead and submit a PR, a separate fix for #607, and separate issues for the media plugins.

@oliverfoster
Copy link
Member

I'm happy with not including user-select: none;.

I'm also happy with the other suggestions, but I'll pass agreement to @kirsty-hames

@kirsty-hames
Copy link
Contributor

I'm happy with not including user-select: none;.

I'm also happy with the other suggestions, but I'll pass agreement to @kirsty-hames

I agree. The suggestions above sound good to me thanks and I don't think we need to include user-select: none;. Let me know if you would like any support with the proposed PRs @hanshillen.

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

No branches or pull requests

3 participants