welpo / tabi

A modern Zola theme with search, multilingual support, optional JavaScript, a perfect Lighthouse score, and a focus on accessibility.
https://welpo.github.io/tabi/
MIT License
129 stars 38 forks source link

Keyboard navigation of the language picker needs improvement #189

Open Almost-Senseless-Coder opened 1 year ago

Almost-Senseless-Coder commented 1 year ago

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.

welpo commented 1 year ago

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:

  1. Native HTML Elements: While some HTML elements support specific keyboard interactions, none meet all the requirements outlined in the ARIA menu pattern. For example, radio buttons can be navigated using arrow keys, but the aesthetic constraints and semantic issues make them less ideal.

  2. HTML autofocus and tabindex: These attributes could be used to manipulate focus but lack the precision to cover all the keyboard interactions you've highlighted.

  3. <select> Element: A dropdown could be made using a <select> element within a <form>. This comes closest to the desired keyboard interactions, but it has its limitations in styling and potentially semantics.

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.

Almost-Senseless-Coder commented 1 year ago

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.

welpo commented 1 year ago

Thanks for your input!

If there was ever a good reason to use JavaScript, it's definitely to make a site more accessible.

We leave the structure of the language picker as it is right now, but implement the keyboard navigation in an optional JS file, which only gets loaded if the tabi user has enabled a setting - similar to the footnote backlinks.

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.

Almost-Senseless-Coder commented 1 year ago

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 lang attribute from...

welpo commented 1 year ago

I've created a JavaScript solution that addresses the following (the HTML remains the same):

  1. Clicking the menu button moves keyboard focus to the first accessible menu item.
  2. The up and down arrow keys move keyboard focus to the previous/next keyboard accessible menu item.
  3. Keyboard focus is trapped in the menu as long as it remains open.
  4. The Escape key moves focus back to the menu button and hides the menu again.

You can find the proposed changes in the fix/accessibility-js branch (or the JS snippet below). I would really appreciate your feedback on this.

I have a couple of questions:

  1. Should the screen reader read the currently selected language? At the moment, it's displayed as greyed out visually. I noticed that the macOS screen reader doesn't announce it.

  2. Speaking of the macOS screen reader, it also doesn't read out the first item upon opening the menu on my end, although it does select it. I'm not sure how to fix this.

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');
}
welpo commented 10 months ago

@Almost-Senseless-Coder hi! Have you had a chance to review these changes? I don't want to merge without your approval :)