Skip to content
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

Open
Almost-Senseless-Coder opened this issue Oct 4, 2023 · 6 comments
Labels
bug Something isn't working help wanted Extra attention is needed

Comments

@Almost-Senseless-Coder
Copy link
Contributor

Bug Report

Environment

Zola version: 0.17.2
tabi commit: latest

Expected Behavior

As per definition of the menu pattern, the following hould happen:

  • Clicking the menu button should move keyboard focus to the first keybaord accessible menu item,
  • up/down arrow keys should move keyboard focus to the previous/next keyboard accessible menu item,
  • Keyboard focus should get trapped in the menu as long as it remains open,
  • Escape should move focus back to the menu button and hide the menu again.

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
Copy link
Owner

welpo commented Oct 16, 2023

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.

@welpo welpo added bug Something isn't working help wanted Extra attention is needed labels Oct 16, 2023
@Almost-Senseless-Coder
Copy link
Contributor Author

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:

  • We just leave it the way it is. If a tabi user wants to expand upon the language picker using JS, nothing is stopping them.
  • 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'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
Copy link
Owner

welpo commented Oct 17, 2023

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
Copy link
Contributor Author

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
Copy link
Owner

welpo commented Oct 17, 2023

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
Copy link
Owner

welpo commented Jan 4, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

2 participants