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

Fix Bug: .visuallyhidden on macOS VO #1986

Merged
merged 1 commit into from
Aug 12, 2017
Merged

Fix Bug: .visuallyhidden on macOS VO #1986

merged 1 commit into from
Aug 12, 2017

Conversation

scottaohara
Copy link
Contributor

This PR solves issue 1985: macOS - VoiceOver / Chrome announcing visually hidden text out of order

By removing the negative margin, and setting it to margin: 0;, the issue is resolved.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

This PR solves [issue 1985: macOS - VoiceOver / Chrome announcing visually hidden text out of order](https://github.com/h5bp/html5-boilerplate/issues/1985)

By removing the negative margin, and setting it to `margin: 0;`, the issue is resolved.
@roblarsen roblarsen merged commit 7bff68f into h5bp:master Aug 12, 2017
@scottaohara scottaohara deleted the bug-fix_visuallyhidden_vo branch August 12, 2017 01:01
@joe-watkins
Copy link

@scottaohara arg! I'm able to replicate the reading order bug with a line-height defined on the body. I was excited to put the new visually hidden CSS into action on a project and it still didn't help with reading order.

This makes a bit of sense as to why my initial 'fix' helped as it kept the text inline. It seems like if the text falls outside of some little box the browser is making for the type it affects reading order.

I'm continuing testing on this today. Check out this pen -- if you move the line-height below 1.3 it works. What the?

https://codepen.io/joe-watkins/pen/GvMyVR

@scottaohara
Copy link
Contributor Author

scottaohara commented Aug 14, 2017

well this is incredibly frustrating.

i'll spend some time hammering on this as well

also, it's not working for me w/the line-height below 1.3?

@scottaohara
Copy link
Contributor Author

scottaohara commented Aug 14, 2017

OK, @joe-watkins

Here's the markup I've been using (based on your markup, with extra text appearing after the VH-text)

<a href="/some-cool-page" target="_blank">
  I am a link with visually hidden text<span class="visuallyhidden2">, 
opens in a new window</span> more content to come after v.h.text
</a>

I've been able to modify the ruleset in the following ways to correct the reading order:

  1. remove the height property. Doing this will result in the correct reading order, but creates a "bump" in the focus indicator, as displayed in the following screen shot:
    screen shot of focus indicator with a 'bump' where the visually hidden text is located
.visuallyhidden-no-height {
    clip: rect(0 0 0 0);
    clip-path: inset(50%);
    overflow: hidden;
    width: 1px;
    position: absolute;
    white-space: nowrap;
}
  1. Alternatively, the height can be left in tact, and instead the position: absolute; can be removed, and a display: inline-block; can be added. This could work in situations where content ends with a block of visually hidden text, but in the example I made, where the vh-text appears in the middle of a sentence/link, there's a subtle gap of where the vh-text lives, caused by the removal of position: absolute;

screen shot of second solution, with gap in the link's underline, where the visually hidden text exists in the DOM

.visuallyhidden-no-absolute {
    clip: rect(0 0 0 0);
    clip-path: inset(50%);
    overflow: hidden;
    width: 1px;
    height: 1px;
    // position: absolute;
     display: inline-block;
    white-space: nowrap;
}
  1. Going back to the first option, if a larger height value is set (i got it to work with 5%, but I'd submit better safe than sorry, so at minimum 20%). and the parent of the visuallyhidden text has a position: relative; set, then the bump in the focus indicator won't appear.
.visuallyhidden-with-height {
    clip: rect(0 0 0 0);
    clip-path: inset(50%);
    overflow: hidden;
    width: 1px;
    height: 20%;
    top: 0;
    position: absolute;
    white-space: nowrap;
}

screen shot of no bump in focus indicator when rule set 3 is applied

Additionally, I found no issues with reading order when the original visually hidden text class was used with non-interactive text (e.g. paragraphs, lists and a heading, as in the above screen shot)

So those are the 'fixes' I've identified for now....personally I'm partial to number 3, but think number 1 would require the least amount of effort.

Thanks for continuing to test this, Joe.

@joe-watkins
Copy link

joe-watkins commented Aug 14, 2017

With the current CSS I also noticed that in Safari the focus ring appears a bit out-of-whack.. you can 'see' the visually hidden text in a way.

screenshot-blip

Oddly, I've had some success by adding display: flex; - continued visual weirdness with focus ring, only it moves to the bottom.

Adding top: 50%; actually worked a bit as well but created a giant focus box in VO/Safari.

@scottaohara I'm partial to #3 as well but feel like defining a percentage to anything may be brittle in certain scenarios.. who knows.

I'm more partial to my original recommendation to setting the positioning to relative, which will allow for text to be anywhere in the content, and display of inline block. I can't get this to break!

.visuallyhidden {
    border: 0;
    clip: rect(0 0 0 0);
    clip-path: inset(50%);
    display: inline-block; /* new */
    height: 1px;
    margin: 0;
    overflow: hidden;
    padding: 0;
    position: relative; /* relative instead of absolute */
    width: 1px;
    white-space: nowrap; /* 1 */
}

Code demo:
https://codepen.io/joe-watkins/pen/zdEWRv

@scottaohara
Copy link
Contributor Author

scottaohara commented Aug 14, 2017

@joe-watkins the original demo also has a break in an underline if you include additional text after the visuallyhidden text :(

subtle, but it'll drive all those 'pixel perfect' perfectionists crazy ;)

showcasing break in underline after the word 'text' and before the word 'adding'

shrug....maybe it's not that big of a deal for the break to be there? i'm more interested in the text reading correctly than i am with the link underline staying fully intact.

i guess another question to ask, is this actually a VO bug?

@joe-watkins
Copy link

joe-watkins commented Aug 14, 2017

@scottaohara Ha! Adding the old negative margin fixes the visual glitch on the underline. Leaving us with:

.visuallyhidden {
    border: 0;
    clip: rect(0 0 0 0);
    clip-path: inset(50%);
    display: inline-block; /* new */
    height: 1px;
    margin: -1px; /* back to life */
    overflow: hidden;
    padding: 0;
    position: relative; /* relative instead of absolute */
    width: 1px;
    white-space: nowrap; /* 1 */
}

I've been testing this pen with good results. I don't have an IE7/6 VM but holds up in 8/9 visually and JAWS/NVDA/VO doing well with modern browsers.

https://codepen.io/joe-watkins/pen/rzGrWX

Insert Family Guy CSS meme here :)

@scottaohara
Copy link
Contributor Author

nice @joe-watkins. not going to lie, i feel pretty silly right now since we're mostly back to your original suggestion. sorry to send us in a circle :(

I just tested your codepen link and saw that position: absolute; was still in there. instead of adding relative, i just took the property away, and didn't seem to have any adverse affects. is it needed for JAWS/NVDA IE? I only have voice over presently so i can't test those right now.

@joe-watkins
Copy link

@scottaohara Ha good catch.. copy/paste craziness got me there. You are correct,.. removing the position things still hold up. I believe the absolute positioning was for clip support in older browsers.

No worries with driving in circles -- that's all web development is :) This leaves us with:

.visuallyhidden {
    border: 0;
    clip: rect(0 0 0 0);
    clip-path: inset(50%);
    display: inline-block;
    height: 1px;
    margin: -1px;
    overflow: hidden;
    padding: 0;
    width: 1px;
    white-space: nowrap; /* 1 */
}

I'll plop this into a couple projects and see if I can break something.

@scottaohara
Copy link
Contributor Author

nice work @joe-watkins :)

@joe-watkins
Copy link

@scottaohara tnx! you too! Looks like this new CSS also works with .visuallyhidden.focusable:active,.visuallyhidden.focusable:focus {} which is good.

I'll take our findings back to the main issue to get more thoughts ;)

@tomasz1986
Copy link

There is no point in using clip: rect(0 0 0 0) without position: absolute.

@joe-watkins
Copy link

@tomasz1986 tnx! I wondered about that. I guess in super old browsers visually things may break w/out any targeting.

@tomasz1986
Copy link

It is not really about super old browsers. clip was supposed to be replaced by clip-path but support for it is still very spotty/experimental at best.

http://caniuse.com/css-clip-path

On the other hand, clip works in everything starting from IE 4.

roblarsen pushed a commit to roblarsen/html5-boilerplate that referenced this pull request Sep 25, 2017
This PR solves [issue 1985: macOS - VoiceOver / Chrome announcing visually hidden text out of order](https://github.com/h5bp/html5-boilerplate/issues/1985)

By removing the negative margin, and setting it to `margin: 0;`, the issue is resolved.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants