-
-
Notifications
You must be signed in to change notification settings - Fork 38
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
Keyboard navigation of the language picker needs improvement #189
Comments
Hey Tim, Thanks for the detailed bug report. Upon reviewing the issue and attempting some fixes, I'd like to discuss the constraints we have when it comes to resolving this without using JavaScript:
We are left with the question: Is it worth introducing JavaScript just for this feature? Implementing these specific keyboard interactions in compliance with ARIA standards would likely require a JavaScript solution like the one described here. However, that goes against my goal of minimising the use of JavaScript where possible. Given these constraints, I'd like to open the floor for discussion. Is the gain in accessibility by adhering strictly to the ARIA standards worth the trade-off of adding JavaScript to the project? Your feedback on this would be very valuable. |
You definitely won't be able to implement this pattern - and indeed many of the other desktop-UI like ARIA patterns - without JavaScript. This is a difficult call: The current implementation works and is accessible. However, it is non-standard, i.e., blind users won't expect his behaviour; whether they figure the language picker out is in part a matter of tech savviness, I reckon. On top of that, "a role is a promise" - by assigning the menu role, you sort of promise to your blind users that this widget behaves in a certain way - in this case Is navigatable with the up/down arrow keys. That said, I also understand your discomfort at adding even more JS. Two proposals:
I'm pretty busy at the moment, but when things calm down I can try to implement the latter approach for my website, and you can then decide if you want to merge it into Tabi or not; I won't judge either way, there's no clear right or wrong call here. |
Thanks for your input! If there was ever a good reason to use JavaScript, it's definitely to make a site more accessible.
I like this idea. I'll start working on it following the menu pattern info you shared and this article: https://codyhouse.co/blog/post/accessible-language-picker I'll be asking for your thoughts soon, hopefully. |
That article is good, I've used it as well when implementing a language picker. That's actually where I got that tip about the |
I've created a JavaScript solution that addresses the following (the HTML remains the same):
You can find the proposed changes in the I have a couple of questions:
The JS: document.addEventListener("DOMContentLoaded", function () {
initDropdownMenu();
});
function initDropdownMenu() {
const dropdown = document.querySelector('.dropdown');
const menuItems = document.querySelectorAll('[role="menuitem"]');
const menuButton = dropdown.querySelector('[role="button"]');
dropdown.addEventListener("toggle", handleToggle.bind(null, dropdown, menuItems, menuButton));
document.addEventListener("keydown", handleKeydown.bind(null, dropdown, menuItems, menuButton));
}
function handleToggle(dropdown, menuItems, menuButton) {
if (dropdown.hasAttribute('open')) {
focusMenuItem(menuItems, 0);
setAriaExpanded(menuButton, true);
} else {
menuButton.focus();
setAriaExpanded(menuButton, false);
}
}
function handleKeydown(dropdown, menuItems, menuButton, event) {
if (!dropdown.hasAttribute('open')) return;
let focusedItemIndex = Array.from(menuItems).indexOf(document.activeElement);
switch (event.key) {
case "ArrowDown":
event.preventDefault();
focusMenuItem(menuItems, (focusedItemIndex + 1) % menuItems.length);
break;
case "ArrowUp":
event.preventDefault();
focusMenuItem(menuItems, (focusedItemIndex - 1 + menuItems.length) % menuItems.length);
break;
case "Escape":
dropdown.removeAttribute('open');
setAriaExpanded(menuButton, false);
menuButton.focus();
break;
}
}
function focusMenuItem(menuItems, index) {
menuItems[index].focus();
}
function setAriaExpanded(element, state) {
element.setAttribute('aria-expanded', state ? 'true' : 'false');
} |
@Almost-Senseless-Coder hi! Have you had a chance to review these changes? I don't want to merge without your approval :) |
Bug Report
Environment
Zola version: 0.17.2
tabi commit: latest
Expected Behavior
As per definition of the menu pattern, the following hould happen:
Current Behavior
The menu only can get navigated using tab, which works, but is unusual.
Step to Reproduce
Navigate to the language picker by keyboard using the tab key and hit enter/space to activate it. Keyboard focus will remain in place. Press tab to move to the first menu item. Hit up/down arrow keys. Nothing will happen.
The text was updated successfully, but these errors were encountered: