Use .hidden instead of .screen-reader-text for toggling helper texts #4530
Conversation
Size Change: -102 B (0%) Total Size: 1.06 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.
Good catch with this issue @nielslange! The usage of the screen-reader-text
class here seems very weird and an important accessibility bug.
I'm approving, but suggesting one improvement in case you want to apply it before merging: do you think we should use the hidden
HTML attribute instead of the hidden
CSS class? The attribute seems to have more semantic meaning and it automatically applies display: none
.
Thanks for your review and approving the suggested change. I wasn't aware of the |
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.
Thanks for updating this PR @nielslange! LGTM 🚢
Fixes #4525
Accessibility
n/a
Screenshots
How to test the changes in this Pull Request:
Products by Tag block
Products by Category block
Products by Attribute block
Note
This PR only replaces
.screen-reader-text
with.hidden
, while it does not change the existing functionality. However, the current functionality seems to be implemented the other way around.Changelog