.dropdown-trigger class and keyboard navigation

[Gumboots]

Something I have noticed while testing recently: the .profile-contact drop menu has an anchor for the .dropdown-trigger class, so is accessible by keyboard. However other drop menus (like topic tools, jump to, etc.) use a span for the .dropdown-trigger class, and obviously this is not keyboard-accessible.

Is there any specific reason why this was done? It seems like an oversight to me, but I may be missing something.

[Mick]

Hmm, I see what you mean, but I suspect they may be accessible via [tab] or [ctrl+chr] (not tested) although not ideal. Does anyone navigate anything these days without a mouse or touch screen?

[warmweer]

Hmm, I see what you mean, but I suspect they may be accessible via [tab] or [ctrl+chr] (not tested) although not ideal. Does anyone navigate anything these days without a mouse or touch screen?

Speech: I say "Go to next tab" ...

[cabot]

Does anyone navigate anything these days without a mouse or touch screen?

Yes

https://www.w3.org/WAI/
https://en.wikipedia.org/wiki/Web_accessibility

[Mick]

Point taken.

[Gumboots]

A11y is the main reason for providing keyboard nav, but I also know some coders who tend to use keyboard nav quite heavily. Although admittedly in their case it is preference rather than necessity. I use it a bit myself sometimes, depending on mood and context.

Anyway, reason I asked is that I'm messing with a custom style again. Naturally it occurred to me that changing these critters to use anchors instead of spans isn't much hassle, so I thought I should double check to see if there was anything I hadn't considered. The only potential trap I have found so far is this, hiding in adm/style/permissons.js:

Code: Select all

	// Update the dropdown trigger to show the new value
	parent.querySelector('span.dropdown-trigger').innerText = text;
	parent.querySelector('input[data-name^=role]').value = roleId;

That would need to be changed to target an anchor instead of a span. Apart from that it just seems to be basic markup changes in the relevant templates (the rest of the js doesn't specify tag, only class, so should be no problem).

And arguably you don't need full a11y for every corner of admin anyway.

[Gumboots]

Hmm. Just took a look in that corner of admin. For some reason (presumably someone's ideas on styling) it's taking the content of a perfectly usable select and then using arcane trickery to turn it into a ul wrapped in umpteen divs, triggered by a span, while hiding the select from view.

Ok, it's prettier than a default Windows select presentation, but all the other selects on the page are bog basic anyway, and they are more accessible. In any case, you can style selects with CSS if you want them prettier (even in Windows). The way it's done at the moment seems like someone was determined to use three times as much code as was necessary to do the job, while simultaneously reducing accessibility.

Anyway, not an issue for a custom style, since the custom style won't be touching the ACP.

[Gumboots]

Bumping this, because I'm currently updating some templates anyway and there's a better way of doing it. Namely, buttons instead of anchors (or spans).

Advantages are:

1. Better semantics.
2. No need for role="button" (which the phpBB anchors and spans don't have, even though they really should).
3. No need for a href, which further simplifies markup (and also means no page jumping if anyone accidentally clicks when js is disabled).
4. Mouse activation will work exactly the same way with or without javascript.
5. Full keyboard accessibility, without javascript being required.
6. Automatic click outside to close, without javascript being required.
7. Automatic closing when you tab out of the drop menu (if using keyboard nav).

The trick is to swap the current no-js fallback from .dropdown-container: hover; to .dropdown-container: focus-within;

This is also used with js. IOW, one bit of CSS does the lot. This trick could not be used with ancient browsers, but has had full support for some time (except IE, but IE can have hover fallback). It works brilliantly. It can still be improved slightly when js is available*, but doesn't need it to give full functionality for everyone.

*The improvements are obviously:

1. to keep drop menu content hidden unless the button is actually selected by click/tap/Enter and
2. to alter the button behaviour to also allow closing the drop menu by by click/tap/Enter (which is not possible without js).

This probably won't get into phpBB 3.3.x at this stage, but IMO should be the preferred way of doing it for any future versions. I'd also suggest that anyone who is customising templates use this method. It really is a lot better, and very simple to implement.

[danieltj]

I tried to use keyboard based navigation around this topic to do some quick testing and the two search boxes are the only things that are navigable. Everything else isn't intractable which is quite surprising.

[Gumboots]

Using span for dropdown triggers isn’t ideal for accessibility since span elements aren’t focusable by default. It’s possible that the dropdown-trigger class was used inconsistently.

It was. I've checked. There are 12 instances of it in the 3.3.12 templates.

Code: Select all

prosilver\template\display_options.html (2 hits)
	Line  1: <div class="dropdown-container dropdown-container-left dropdown-button-control sort-tools">

prosilver\template\jumpbox.html (2 hits)
	Line 30: 	<div class="jumpbox dropdown-container dropdown-container-right<!-- IF not S_IN_MCP --> dropdown-up<!-- ENDIF --> dropdown-{S_CONTENT_FLOW_BEGIN} dropdown-button-control" id="jumpbox">

prosilver\template\memberlist_search.html (1 hit)
	Line 14: 			<!-- IF U_LIVE_SEARCH --><div class="dropdown-container dropdown-{S_CONTENT_FLOW_END}"><!-- ENDIF -->

prosilver\template\navbar_header.html (3 hits)
	Line   6: 		<li id="quick-links" class="quick-links dropdown-container responsive-menu<!-- IF not S_DISPLAY_QUICK_LINKS and not S_DISPLAY_SEARCH --> hidden<!-- ENDIF -->" data-skip-responsive="true">
	Line 106: 			<div class="header-profile dropdown-container">
	Line 155: 			<li class="dropdown-container dropdown-{S_CONTENT_FLOW_END} rightside" data-skip-responsive="true">

prosilver\template\pagination.html (1 hit)
	Line  3: 	<li class="dropdown-container dropdown-button-control dropdown-page-jump page-jump">

prosilver\template\ucp_pm_message_header.html (1 hit)
	Line 33: 		<div class="dropdown-container dropdown-button-control topic-tools">

prosilver\template\ucp_pm_viewmessage.html (1 hit)
	Line  59: 				<div class="dropdown-container dropdown-left">

prosilver\template\viewtopic_body.html (3 hits)
	Line 181: 				<div class="dropdown-container dropdown-left">
	Line 427: 	<div class="quickmod dropdown-container dropdown-container-left dropdown-up dropdown-{S_CONTENT_FLOW_END} dropdown-button-control" id="quickmod">

prosilver\template\viewtopic_topic_tools.html (1 hit)
	Line  2: 	<div class="dropdown-container dropdown-button-control topic-tools">

Out of those, display_options.html uses a span , and also duplicates the .dropdown-container class on the div. The duplicate class doesn't cause any issues in practice, but was obviously a mistake.

Edit: Just realised I wasn't looking closely enough, and the "duplicate" is actually .dropdown-container-left, so not a mistake. It's only used for setting a float though, and there are other classes that do that (.leftside and .dropdown-left) so there is still bloat in the CSS. And personally I'd be going with flex these days anyway, for the greater possibilities and the automatic RTL support, so I'd be deprecating the floats. Anyway...


The other thing is that in this instance .dropdown-container is a superfluous div inside the li, while some other instances have the .dropdown-container class on the parent li (which is all it needs to work). Really, the markup should be ul.whatever > li.dropdown-container > button.dropdown-trigger + div.dropdown in all cases.

jumpbox.html, ucp_pm_message_header.html, viewtopic_body.html, and viewtopic_topic_tools.html all use a span, and viewtopic_body.html has another case of the .dropdown-container class being duplicated on the same element.

The rest of the instances use anchors, except for memberlist_search.html, which is a really strange exception. And only three of the instances have the .dropdown-container on the li, where it should be. The rest of them have the superfluous div inside the li (God knows why, and if anyone suggests "making them consistent" by adding the superfluous div to all instances I will murder kittens).

It might be worth suggesting a change to ensure all dropdowns use anchors (<a>) or are made keyboard-accessible.

Buttons are better. Really. Anchors are for taking you somewhere else. Buttons do stuff on the same page. They are made for it.

[Gumboots]

I tried to use keyboard based navigation around this topic to do some quick testing and the two search boxes are the only things that are navigable. Everything else isn't intractable which is quite surprising.

No, its not that bad. It does have navigable anchors in lots of place (ie: pagination) and sensible things like h2's and h3's (which are not inherently focusable but are handy for screen readers). Most of the page is quite usable. It's just that a few aspects could be improved, and it wouldn't be that big a job.

Incidentally, I had a quick bash at the javascript refinements for a button-based system. This seems to cover all bases, and is less than 1 Kb of vanilla js.

Code: Select all

<script>
	const triggers = document.querySelectorAll('li.dropdown-container');
	for (const trigger of triggers) {
		//	Add .dropstop to class list.
		trigger.classList.add('dropstop');
		const button = trigger.firstElementChild
		document.addEventListener('click', (event) => {
			//	Toggle menu open/closed via click/Enter/tap.
			if (button.contains(event.target)) {trigger.classList.toggle('dropstop')}
			//	Menu stays open if target is child element.
			else if (trigger.contains(event.target)) {return}
			//	Click/tap anywhere outside to close.
			else {trigger.classList.add('dropstop')}
		});
		//	Reset to closed on tabbing out, or via Esc.
		document.addEventListener('keyup', (event) => {
			if (!trigger.contains(event.target) || (event.code == 'Escape')) {trigger.classList.add('dropstop')}
		});
	}
</script>

You can uglify it down to this (417 bytes) and it still works:

Code: Select all

const triggers=document.querySelectorAll("li.dropdown-container");for(const t of triggers){t.classList.add("dropstop");const s=t.firstElementChild;document.addEventListener("click",e=>{if(s.contains(e.target))t.classList.toggle("dropstop");else{if(t.contains(e.target))return;t.classList.add("dropstop")}}),document.addEventListener("keyup",s=>{t.contains(s.target)&&"Escape"!=s.code||t.classList.add("dropstop")})}

CSS is just this (anything else is only eye candy):

Code: Select all

.dropdown, .dropdown-container.dropstop:focus-within .dropdown {
	display: none;
}
.dropdown-container:focus-within .dropdown {
	display: block;
}

With the markup being ul.whatever > li.dropdown-container > button.dropdown-trigger + div.dropdown

Not tested on everything yet, but it works so far.

[Gumboots]

I've

I tried to use keyboard based navigation around this topic to do some quick testing and the two search boxes are the only things that are navigable. Everything else isn't intractable which is quite surprising.

I'm just finishing up implementing my menu idea* for my own use, and noticed something else. The default quickreply_editor.html is a disaster for keyboard nav. It has tabindex="2" on the subject input, tabindex="3" on the textarea, then tabindex="6" and tabindex="7" on the preview and submit buttons. It makes no logical sense in terms of numbering, because 1, 4 and 5 are missing, but that's not the real issue.

The real issue is that having those positive values set makes the bottom action-bar, and everything after it, inaccessible via keyboard nav when the quick reply is enabled. Totally screws it. The browser only recognises the two editor buttons as the last things on the page, then jumps back up to the start of the page.

If you remove the tabindex values for the quick reply suddenly everything is focusable, in the correct order (including all the editor components) right to the end of the page.

Now I realise my enthusiasm for changing code may not be regarded as suitable for a point release, but if you want to give the topic pages a quick and easy a11y boost the simplest way of doing it would be:

1. Get rid of all positive tabindex values (see https://www.tpgi.com/using-the-tabindex-attribute/)
2. Use tabindex="0" on the drop menu trigger spans to make them focusable.

2/ is frankly still not that great, because it would be better to use an inherently focusable element, but tabindex="0" will at least make the spans usable via keyboard without screwing anything else. It's a trivial markup change that won't conflict with anyone's CSS or javascript.

*And yes, my button-activated menus work perfectly with Firefox or Chrome-based browsers, on Windows desktop and on Android, with slide-out sidebars and with drop menus on the same style, while allowing animation eye candy if desired, and all on about 1 kB of vanilla js. Haven't tested with iThingz yet because I don't own any, but even Safari should find it hard to screw up.

The only caveat is that for uses like display_options.html it is necessary to set type="button" on the trigger, because otherwise it will adopt the default type="submit", which will conflict with the parent form. But as long as you do that it's bulletproof.

[Terceirense]

Yeah, it does seem like an oversight. Using a span for dropdown triggers isn’t accessible by default because spans don’t get focus like anchors do. Maybe it was just a mistake or an afterthought. Adding proper tabindex or using buttons could fix it. Good catch!

[danieltj]

To be honest I doubt it’s worth investing more n making prosilver accessibility-friendly. The code base is super old and is a bit of a mess. It would be much easier to start from scratch rather than trying to retroactively fox all of the problems with it.

Of course nothing is stopping you opening a pull request and fixing it yourself.

[Gumboots]

True, but if the general feeling on the team is "Meh, leave it for 4" there's no point me wasting everyone's time with a pull request. That's why I thought I'd float the idea here. I can understand if the PTB think they have enough on their plates just keeping the thing running.