Make .element-invisible work for focusable elements like links

I do think that @derjochenmeyer's point in #136 is quite interesting.

I'm not sure when there are instances where there is invisible text which has an embedded link associated with it. I don't know if there is an existing precedent for this though in the accessibility world.

I think that someone tabbing through the links would probably get quite confused either way.

I think this is likely going to have to be a D8 issue at this point. I'm definitely interested in hearing thoughts on how to address this though. User expectations are key.

If there is not currently a critical issue that is dependent on this issue then we should likely push this to D8.

#896364: Screen reader users and some keyboard only users need a clear, quick way to disable the overlay depends on this issue, so let's fix it. Quick. :)

So it seems like this was partially discussed in #718922-136: system class: .element-invisible does not work with VoiceOver on OS X Snow Leopard already. Furthermore, the patch in #896364-70: Screen reader users and some keyboard only users need a clear, quick way to disable the overlay contains the most recent proposal for styles that were proposed for Overlay only; can they serve as first kick-start template, perhaps?

Can someone summarize the last known findings?

Upfront, I guess this will turn into a new .element-invisible-focusable (perhaps shorter :P) base CSS class, so markup authors can cleanly decide whether they want links to be focusable in their invisible element or not. I figure that is needed, because Drupal can easily produce unexpected markup injected by other modules.

@sun - Thanks! That's a good enough argument for me.

Here's @casey's suggestion rolled up as a patch. It's also applied here - http://drupal7.dev.openconcept.ca/

I'm going to apply the latest patch from #896364: Screen reader users and some keyboard only users need a clear, quick way to disable the overlay so that we can have a working example to look at too.

I don't know if there is a best practice for how to do this. Certainly it would be nice to have a standard means to hide/display "Skip to main content link” as part of core.

However, the links we are talking about are a bit different than this. I'm wondering how something like:

<div class="element-invisible">Lies, damn lies & <a href="http://manyeyes.alphaworks.ibm.com">Many Eyes</a></div>

should be displayed. It's much different than something like:

<div class="element-invisible"><a href="#main">Skip to main content link</a></div>

A new name space is probably the right way to go so that we can avoid confusion. It's been hard on this issue to find agreement on names, but this works for me .element-invisible-focusable

In which case we'd probably just repeat the solution for .element-invisible:

.element-invisible-focusable {
  position: absolute !important;
  clip: rect(1px 1px 1px 1px); /* IE6, IE7 */
  clip: rect(1px, 1px, 1px, 1px);
}
.element-invisible-focusable:active,
.element-invisible-focusable:focus {
  position: static !important;
  clip: auto;
}

But we'd need to add a lot of inline docs as to why it is there.

+1 to the naming and concept by @mgifford.

I'm just spelling out what @sun laid out.

Everett if you are OK with the patch please RTBC.

I'd prefer the separate class as proposed by sun in #5, we can shorten this to just .focusable and chain the selectors. This would be applied like this...

<a href="#main-content" class="element-invisible focusable"><?php print t('Skip to main content'); ?></a>

#10: Multiple class selectors aren't supported by IE6 IIRC.

We don't care about IE6. They can still read the text and come to a conclusion.

@chx

I can't RTBC this patch because I can't ensure that it has no undesirable visual affect on the page.

@11 - there are certain situations where chained selectors are problematic in IE6, but certainly is this instance it works - I just tested it before I submitted the patch. The reason why this works is that IE6 will only read the last selector in the chain (in this case .focusable), which is actually all that any browser needs to read.

I've tested this in my sandbox against the requirement in #896364: Screen reader users and some keyboard only users need a clear, quick way to disable the overlay and this patch has been sufficiently reviewed.

It even looks right in RTL mode from my testing. I haven't tested this in IE 6 but Jeff has which is good for me.

I didn't test, but I agree with simply moving forward with what we have for now. If there'll happen to be a regression, then we can re-open this issue and discuss resolution attempts.

After all, this is a CSS class and we can fix the applied styles at any time.

http://drupal.org/node/896364 depends on this patch.

+++ modules/system/system-behavior.css	15 Sep 2010 17:39:53 -0000
@@ -313,3 +313,13 @@
+ * The .focusable class extends the .element-invisble class to allow the element

Invisible is spelled wrong.

Also, it seems the comment for .element-invisible in system-behaviors.css needs updating. Specifically this sentence:

Must not be used for focusable elements (such as links and form elements) as this causes issues for keyboard only or voice recognition users.

Aside from those minor issues, I've tested it, and it's not affecting the visual display. Once the comment issues are fixed, I'd say this is ready.

Powered by Dreditor.

Update to account for #18, updating comments and fixing the typo.

Looks good. New docs are a good improvement, thanks @jacine for catching this and @jeff for the re-roll.

This is actually going to need a reroll because of the awesome patch in #885228: CSS Files are in major need of clean up! just landed...

I actually have two more questions about this (sorry):

1. Why aren't we using .element-focusable for the class name? I realize there is some repetitiveness there, when used with .element-invisible, but it seems inconsistent with the others.
2. Is there any reason why you would ever want to use .focusable without .element-invisible?

Thanks :)

It would be futile to use the .focusable class without .element-invisible since it merely undoes what .element-invisible does.

I've rerolled but left it the same so we don't force a reroll on #896364: Screen reader users and some keyboard only users need a clear, quick way to disable the overlay

Congrats on #885228: CSS Files are in major need of clean up! - HOORAY!

And the patch...

@jeff

Thanks for the re-roll. I'll keep #896364: Screen reader users and some keyboard only users need a clear, quick way to disable the overlay in synch if you want to change the name on this element. We have yet to decide upon, let alone fully implement, the solution in the Overlay issue.

Thanks @Jeff! It's definitely a happy day in Drupal theming land! Woo hoo! :D

The naming inconsistency here is bugging me. .element-invisible.element-focusable:focus bugs me as well, but I prefer we keep these consistent. I think nixing the "element-" prefix would make me feel a lot better, but that's a bikeshed discussion for another issue.

@Everett thanks for offering to keep the overlay issue in sync for this.

Since, I feel bad to keep asking Jeff to re-roll, I just did it myself this time. If this is okay with you guys, I have no more questions and think this is RTBC :)

Just changing the name along with previous spelling / doc cleanup. RTBC.

Thanks for the re-rolls. @jacine, thanks for not re-opening the naming discussion re: element-, as I recall it took about 100 comments to get a name everyone was content with.

@Everett, yeah I wouldn't do that to you guys. ;)

These seems to be an oversight in an existing "API" (rather than a new feature), and allows movement on a critical bug. Has sign-off from the accessibility and markup teams.

Committed to HEAD. Thanks, folks!

Hooray, another critical bites the dust!