[ticket/16983] Accessibility - fix elements with no focus style in ACP and Prosilver

[lionel-rowe]

Fixes PHPBB3-16983: Accessibility — elements with no focus style in ACP and Prosilver

This PR simply removes all instances of outline: none and equivalent, per many a11y resources, such as the following:

• https://www.a11yproject.com/posts/never-remove-css-outlines/
• https://webaim.org/blog/plague-of-outline-0/
• https://www.outlinenone.com/
• https://betterprogramming.pub/a11y-never-remove-the-outlines-ee4efc7a9968?gi=5955f3df65d9

IMO this is the optimal fix — some (but not all) of the affected elements already independently define their own focus styles, but these are generally pretty subtle compared to the default outline, so re-adding the outline even for these elements is still an accessibility boost.

Checklist:

• Correct branch: master for new features; 3.3.x for fixes
• Tests pass
• Code follows coding guidelines: master and 3.3.x
• Commit follows commit message format

Tracker ticket (set the ticket ID to your ticket ID):

https://tracker.phpbb.com/browse/PHPBB3-16983

[hanakin]

Unfortunately we do this on purpose! We will never support default outlines. The recommend practice is to apply the same hover transition on focus for a number of reasons. The biggest being layout control.

If these are lacking a hover state on focus or if there is a case you feel its not apparent enough then by all means we will happily look at it.

You are also trying to change normalize.css is is not allowed ad its a third party library. The only thing we would look at fir this case is possibly upgrading it to a newer version if needed.

[lionel-rowe]

@hanakin Unfortunately we do this on purpose!

Yes, doing this on purpose is the problem — please refer to the links I posted.

We will never support default outlines

On the contrary, phpBB already supports default outlines on most focusable elements. Try Tab-ing around a fresh install with prosilver and you'll see more elements with the default outline than without it.

The problem is that a minority of elements don't support it, and worse yet, some of those elements don't have any alternative styles defined.

The recommend practice is to apply the same hover transition on focus for a number of reasons.

Do you have a source for this? I've never heard of this recommendation before. Hover and focus are two different types of states; an element can be hovered, focused, both, or neither.

The biggest being layout control.

Not sure what you mean by this. Unlike border, outline can never affect layout. Per MDN:

Unlike other areas of the box, outlines don't take up space, so they don't affect the layout of the document in any way.

---

You are also trying to change normalize.css is is not allowed ad its a third party library.

Fair enough, happy to revert that one as in practice it only affects :focus:active and :focus:hover. Even though their rationale of "to improve readability" makes no sense.

Edit: Actually not 100% happy — I think removing it on :focus:active is fine, as it only affects mouse users, but removing it on :focus:hover is confusing as you no longer get any focus indicator if your mouse happens to be over an element you Tab-ed into. But this can be fixed without modifying normalize.css (see latest changes).

[hanakin]

Honestly decades of experience and every open source framework i have ever worked with. But here is a quick link if you want that hints at my main points.

https://css-tricks.com/having-a-little-fun-with-custom-focus-styles/

By layout i mean two things.

1. given older browsers do not treat outline as you have mentioned and in fact if by error it did affect layout.

2. Given that modern browsers do treat it as not affecting layout like borders. It still has layout artifacts based on shape at times.

The last and probably biggest reason focus and hover matching is considered a best practice. which is hit upon in the first comment on that article is/was the lack of support for controlling the outlines rendering when using a mouse.

With all this said this all was based of older browser versions. If support is there in more current browsers for methods of only targeting display of outlines for these situations then I am open to re-looking at it.

As for the inconsistency this is prosilver its been hack upon by hundreds of individuals with no quality control from a design standpoint since its inception. Making it extremely difficult to try nowadays. Which is why one we are moving away from it, and two I have been trying to refactor whole components. The second hitting lots of barriers along the way. Mostly the template and theme structure being too verbose and not organized. As well as the worst selector scope ever

I would love to eventually overhaul all buttons to be uniform and flat with a simple contrasting bg change. As well as simplify all links in a similar way.

I would opt for this rather than adding an outline though since its purpose is meant as you said for accessibility. And instead fix those that do have outlines still.

My stance still remains that it should only exists in a situation that requires it as to not distract from the overall design. Which means using the hover styles on focus if it can not.

[lionel-rowe]

Honestly decades of experience and every open source framework i have ever worked with. But here is a quick link if you want that hints at my main points.
https://css-tricks.com/having-a-little-fun-with-custom-focus-styles/

I see nothing about "recommended" in there, it just mentions it's one thing you can do. CSS tricks is a fantastic resource, but I wouldn't treat it as authoritative when it comes to a11y.

given older browsers do not treat outline as you have mentioned and in fact if by error it did affect layout.

Which specific older browsers? Are we talking like IE6 or something?

Given that modern browsers do treat it as not affecting layout like borders. It still has layout artifacts based on shape at times.

True, though these can be fixed easily enough by changing margin/padding etc. Also I don't see any affected elements in Prosilver.

If support is there in more current browsers for methods of only targeting display of outlines for these situations then I am open to re-looking at it.
[...]
My stance still remains that it should only exists in a situation that requires it as to not distract from the overall design. Which means using the hover styles on focus if it can not.

I'm playing around with a combined CSS/JS solution as we speak — something like this:

.force-outline {
	outline: revert !important;
}

const focusClass = 'force-outline';

const addFocusClass = e => $(e.target).addClass(focusClass);
const removeFocusClass = e => $(e.target).removeClass(focusClass);

// we use `addEventListener` and `removeEventListener` rather than
// jQuery equivalents to allow access to event delegation via capture=true
document.addEventListener('focus', addFocusClass, true);
document.addEventListener('blur', removeFocusClass, true);

$(document).on('mousedown touchstart', () => {
	// fires before `focus` when event is clicked/touch
	document.removeEventListener('focus', addFocusClass, true);
});

$(document).on('mouseup touchend', () => {
	// re-add the listener after click/touch is finished
	document.addEventListener('focus', addFocusClass, true);
});