Conversation
Size Change: +413 B (0%) Total Size: 1.16 MB
ℹ️ View Unchanged
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've tested with Safari VoiceOver but I couldn't focus the element in normal use. Keen for some testing of this with various browsers/platforms and screen reader tech :)
If you were using the Tab key, I think that's to be expected. Screen readers usually provide a different navigation method which is more "line-by-line" so you can navigate to non-focusable items. With Orca it's done with the arrow keys, I guess it's probably the same in other screen readers?
I tested removing the screen-reader-text
styles from Storefront and I can confirm this PR adds a fallback so screen reader elements are hidden.
In order to test :focus
styles, I converted the <FormStep>
legend
element into an a
and it looked great:
My only concern is that we are introducing a new set of 'screen-reader' styles, which differs from the ones in other projects:
Not blocking merge, but do you think we could use one of those 👆 as the base instead of introducing a new one from underscores? 🙂
- tweak mixin to align with focus-reveal style - add new mixin for focus-reveal style, since it is coupled to visually-hidden mixin
Good thinking @Aljullu , I should have looked more carefully for prior art! I've switched to using our (most local) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice to see a new mixing for the focus state. I didn't re-test it but code changes look good.
Fixes #3265
This PR adds styling for screen-reader text elements. Usually this is handled by the theme. However if the theme doesn't have support the screen reader text may be rendered inline (which is not intended); these fallback styles ensure screen reader text is hidden by default, and provides
:focus
styles for when the text has focus and is being read.The specifics of the styles are pasted direct from underscores, with some colours changed. Feedback welcome on how to generalise these so they are a minimal, safe fallback (i.e. should not be opinionated).
Accessibility
prefers-reduced-motion
I've tested with Safari VoiceOver but I couldn't focus the element in normal use. Keen for some testing of this with various browsers/platforms and screen reader tech :)
Screenshots
Focused in chrome and safari:
Notes:
:focus
using browser dev tools.For comparison, here's a screenshot of the issue:
How to test the changes in this Pull Request:
Bad Theme
using Underscores and deleted the screen reader styles..screen-reader-text
is hidden by default and is helpful when using a screen reader.Changelog