Convert the Overlay close button from an anchor element to a button element.

I can write a patch tomorrow.

Here's the patch. I didn't turned the link in a button element, but I added role="button" to the link. This way nothing should change for sighted users! :D

I've also added aria-controls to the link so that we declare its relation with the overlay content.

Finally, just a question: could we move tabs into the div that the "overlay-content" ID is assigned to?

Committed 3161890 and pushed to 8.x. Thanks!

It doesn't seems correct
#1719640: Use 'button' element instead of empty links

True, it doesn't use a <button>, but they aren't empty links either.

We could switch it to a button, but it's not an accessibility problem with role="button":

<a id="overlay-close" class="overlay-close" aria-controls="overlay-content" role="button" href="#">
<span class="element-invisible">Close overlay</span>
</a>

@droplet, does that clarify things? Still want it switched to something like:

<button id="overlay-close" href="#" class="overlay-close" aria-controls="overlay-content" type="button"><span class="element-invisible"><?php print t('Close overlay'); ?></span></button>

Updated with #7 suggestions.

Also added some css styling to remove border added by element.

Your patch is more consistent with it being a <button>. Similar to #1993894: Contextual quick edit toggle should be a <a role="button"> not a <a> because it tracks on/off state

Applies nicely on SimplyTest.me

Fixed a few issues with #8:
- removed href attribute
- changed closing tag to button as well

Both of these patches though break accessibility as neither of them have any indication that the button has focus. Neither of them work without using a mouse.

So you can't see that you have focus and can't close the overlay without a mouse.

Yeah, it didn't even work with a mouse, because the JS specifically checked for Anchor tag. It only appeared to work, because the anchor was cached for me. Here's another attempt.

This works! I think it should get some JS review before it gets marked RTBC.

From an accessibility point of view, why did you add:
border: 0;

I could see the very faint outline when navigating to the close button, but am wary about removal of things like that and outlines:
http://www.outlinenone.co

As they are useful for sighted keyboard only users.

I got the "border: 0" from patch #8. It is because Anchor tags don't have border by default, and have outline on focus. Buttons don't have outline by default and have border by default. The idea is to change the markup, without any visual change. Without border: 0, there is only some displacement/misalignment of the button, without actual visible border.

I also want someone to see the JS change. There is some weirdness where the overlay can be closed with a right-click, but I know that I didn't introduce this, and it works like that with or without the patch. I am not sure if this was intended like that.

Ok, that satisfies me.. Thanks @MiroslavBanov.

So we'll wait for the review to mark it RTBC.

Need not to change JS code in my opinion.

and missing role="button" in button element

No need to add the role="button", see:

https://developer.mozilla.org/en-US/docs/Accessibility/ARIA/ARIA_Techniq...

Thanks @mgifford. You correct :) I looked at a wrong example:

#2120011: Remove extra role="button" in button element

Even the type="button" seems redundant. But I am not sure - there may be issues with old IE if that is removed.

It's not redundant, it you omit it, it defaults to type="submit" and you really don't want that. It's in the spec, IE isn't for anything in this one :)

Actually, there is some confusion on default type, and it is browser (read: IE) dependent. See this:
http://stackoverflow.com/questions/4667979/whats-the-standard-behavior-w...
So the moral is: without any evidence, if you assume there will be a problem with IE, you are probably correct.

We're in the core queue so a little biksheading won't hurt anyone, moreover we're friday :)

To me IE has the right position (same thing with how it handles padding IE 1 — W3C 0). Since we're talking about having buttons, not submits, looks to me that any problem we'd have would be because of everyone else except IE ; hence IE is not to blame. (I'm not even a fan of IE but you gotta give credit where it's due :)

Anyway, the JS is fine RTBC as per #16

Committed and pushed to 8.x. Thanks!

Committed and pushed to 8.x. Thanks!