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.

Sign up for GitHub

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

Jump to bottom

Docs: screen reader helper/visibility utility tweaks #31133

Merged
merged 7 commits into from Jul 10, 2020
Merged

Docs: screen reader helper/visibility utility tweaks #31133

merged 7 commits into from Jul 10, 2020

Conversation

patrickhlauke
Copy link
Member

  • rename "Screen readers" helper to "Visually hidden" (in future, may consider actually renaming the classes as well? .visually-hidden / .visually-hidden-focusable or similar?)
  • reword description for visibility utility, emphasise/call out the fact that .invisible hides stuff from AT

@patrickhlauke patrickhlauke requested a review from a team as a code owner June 21, 2020 11:43
@patrickhlauke
Copy link
Member Author

meta-question: what's the cut-off between something being a "helper" and something being a "utility"?

@patrickhlauke patrickhlauke added this to Inbox in v5.0.0-alpha2 via automation Jun 21, 2020
@patrickhlauke patrickhlauke added this to Inbox in v4.5.1 via automation Jun 21, 2020
patrickhlauke added a commit that referenced this pull request Jun 21, 2020
@patrickhlauke
Rename sr-only/sr-only-focusable
8adb9d5 
To be more representative of the fact that these are not necessarily "screen reader" specific, but actually apply to assistive technologies in general (and also things like Alexa/Siri/etc). Goes hand-in-hand with #31133
@patrickhlauke patrickhlauke mentioned this pull request Jun 21, 2020
Merged
@patrickhlauke
Copy link
Member Author

Related, the proposed PR to actually rename the classes/mixins as well #31139

@XhmikosR
Copy link
Member

Note that we should focus to v5 and leave v4 in maintenance mode. We don't need to backport everything. Also, this needs to be handled carefully in v4 with a redirect.

@patrickhlauke
Copy link
Member Author

ok, there's probably little point in backporting just this bit. focusing on v5 makes sense.

@patrickhlauke
Copy link
Member Author

@XhmikosR should this also have an aliases?

@XhmikosR
Copy link
Member

@XhmikosR should this also have an aliases?

Doesn't hurt but maybe we don't need to, since it's still an alpha. I'll add a note to revisit the aliases later.

XhmikosR added a commit that referenced this pull request Jul 3, 2020
@patrickhlauke @XhmikosR
Rename sr-only/sr-only-focusable (#31139)
10690dd 
* Rename `sr-only`/`sr-only-focusable`

To be more representative of the fact that these are not necessarily "screen reader" specific, but actually apply to assistive technologies in general (and also things like Alexa/Siri/etc). Goes hand-in-hand with #31133

Co-authored-by: XhmikosR <xhmikosr@gmail.com>
@XhmikosR XhmikosR removed this from Inbox in v4.5.1 Jul 3, 2020
@patrickhlauke
Docs: rename "Screen readers" helper page to "Visually hidden"
729acd9 
"screen readers" is quite reductive, as there are other assistive technologies. content hidden this way is even announced by things like Alexa/Siri etc, so it's not so much just "screen readers".
in the long run, we may even consider changing the actual classnames (maybe `.visually-hidden` / `.visually-hidden-focusable`, though admittedly that's a bit verbose).

also includes a tiny tweak to layout.md to generalise the note about using `.sr-only`
@patrickhlauke
Emphasise/call out the note about .invisible and AT
52f5cd9
@patrickhlauke
Reword slightly awkward-sounding description
0fc1b36
@patrickhlauke
Add alias/redirect
f7053ae
@mdo
Copy link
Member

mdo commented Jul 6, 2020

meta-question: what's the cut-off between something being a "helper" and something being a "utility"?

Generally speaking, utilities are single property-value pairings and helpers are shortcuts or snippets.

@patrickhlauke
Copy link
Member Author

any chance of a review please? ;)

XhmikosR
XhmikosR approved these changes Jul 6, 2020
View reviewed changes
Copy link
Member

@XhmikosR XhmikosR left a comment

Choose a reason for hiding this comment

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

LGTM but I want @mdo to also review

v5.0.0-alpha2 automation moved this from Inbox to Approved Jul 6, 2020
ffoodd
ffoodd approved these changes Jul 6, 2020
View reviewed changes
Copy link
Member

@ffoodd ffoodd left a comment

Choose a reason for hiding this comment

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

LGTM

@patrickhlauke patrickhlauke requested a review from mdo July 6, 2020 11:27
@mdo mdo merged commit 69b3c46 into main Jul 10, 2020
v5.0.0-alpha2 automation moved this from Approved to Shipped Jul 10, 2020
@mdo mdo deleted the v5-docs-sr-visibility branch July 10, 2020 23:00
olsza pushed a commit to olsza/bootstrap that referenced this pull request Oct 3, 2020
@patrickhlauke @XhmikosR @olsza
Rename sr-only/sr-only-focusable (twbs#31139)
3327860 
* Rename `sr-only`/`sr-only-focusable`

To be more representative of the fact that these are not necessarily "screen reader" specific, but actually apply to assistive technologies in general (and also things like Alexa/Siri/etc). Goes hand-in-hand with twbs#31133

Co-authored-by: XhmikosR <xhmikosr@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@XhmikosR XhmikosR XhmikosR approved these changes

@ffoodd ffoodd ffoodd approved these changes

@mdo mdo Awaiting requested review from mdo

Assignees
No one assigned
Labels
docs v5
Projects
No open projects
v5.0.0-alpha2
  
Shipped
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@patrickhlauke @XhmikosR @mdo @ffoodd