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

limel-list: keyboard navigation is broken for lists with checkboxes or radio buttons (affects typeahead too) #1468

Open
adrianschmidt opened this issue Nov 24, 2021 · 16 comments
Labels
bug Something isn't working

Comments

@adrianschmidt
Copy link
Contributor

adrianschmidt commented Nov 24, 2021

Current behavior

broken.checkbox.list.mov

Steps to reproduce the behavior:

  1. Go to the docs for limel-list and scroll to the example called "List with checkboxes".
  2. Focus the list in some way (clicking the disabled option "Charmander" or clicking on an item in the preceding list and then tabbing to the checkbox list both work).
  3. You can now move the focus between all list items using the up and down arrow keys.
  4. Move to "Pikachu".
  5. Press space or enter to deselect "Pikachu".
  • The list loses focus, which is the first bug here.
  1. Select "Pikachu" again, either by focusing the list and then using the keyboard, or simply by clicking on the checkbox, it doesn't matter.
  2. Try to navigate to the bottom of the list using the arrow keys.
  • You can't move past "Yoshi". The two last items in the list can no longer be focused. Other combinations of selecting and deselecting items result in other items becoming unreachable.

Expected behavior

The list should not lose focus when items are deselected, and all list items should always be reachable by keyboard.

Environment

  • lime-elements version: At least v34.0.2-next.10 and later, but I think the issue is older.
  • Framework used: Can be reproduced in the docs.
  • Logs:
@adrianschmidt adrianschmidt added the bug Something isn't working label Nov 24, 2021
@adrianschmidt adrianschmidt changed the title limel-list: keyboard navigation is broken for lists with checkboxes or radio buttons limel-list: keyboard navigation is broken for lists with checkboxes or radio buttons (affects typeahead too) Nov 24, 2021
@adrianschmidt
Copy link
Contributor Author

Check if this is related to using separators and/or disabled items.

@Kiarokh
Copy link
Contributor

Kiarokh commented Jan 12, 2022

This seems to be because the of how tabindex dynamically gets applied list items.

I can't find a solid pattern to describe exactly what happens. But as you tab throw the list items, the tab index number gets changed for all items. It seems the item that is having the keyboard focus gets tabindex="0", and others get "-1". Until you select an item and press enter (to close the menu or dropdown). Then when you press enter again to open it, some items lose the tabindex. Thus they become totally unavailable for keyboard navigation.

I actually don't think it has anything to do with having disabled items or dividers. I can reproduce it in limel-select, both with and without disabled items too.

Screen.Recording.2022-01-12.at.14.38.36.mov

in another example (limel-menu), when the keyboard navigation gets stuck, 👇 it seems to be possible somehow to go backwards and come in again from the other side of the list (press ⬇️ after being on the last item to jump to the top-most item again) of the list to force the keyboard navigation work again.

Screen.Recording.2022-01-12.at.15.10.55.mov

@adrianschmidt
Copy link
Contributor Author

adrianschmidt commented Jan 12, 2022

But as you tab throw the list items, the tab index number gets changed for all items.

That's because you're not meant to tab through the list. I think WCAG says that tab should take you to the selected item in a set, like a group of radio-buttons or a menu, or to the first item if none is selected, the next tab should move you on to the next control, not to the next item in the same set. While a set is focused, arrow-keys should move focus between items. The reason for this is that otherwise a user navigating by keyboard will have to tab their way through every single item in every table, every list, every menu, etc, etc, on the page.

Then when you press enter again to open it, some items lose the tabindex. Thus they become totally unavailable for keyboard navigation.

Ah! Ok, yeah, that might very well be the problem. I wonder why the tabindex attribute is lost then? 🤔

@Kiarokh
Copy link
Contributor

Kiarokh commented Jan 12, 2022

But as you tab throw the list items, the tab index number gets changed for all items.

That's because you're not meant to tab through the list.

Sorry, clumsy writing. I meant: as you go through (not tab, but arrow keys in this case).

Ah! Ok, yeah, that might very well be the problem. I wonder why the tabindex attribute is lost then? 🤔

it seems there is a logic for tabindex in limel-list. But me no understand it.
However, that might require some love

@Kiarokh
Copy link
Contributor

Kiarokh commented Nov 17, 2022

I had a quick look again at this one. I don't entirely understand the logic for setting tab indexes on limel-list items.

This is what I understand:

It seems the problems appear specially when we have a disabled list item. When there is a disabled item, we are getting positive tabindexes on some list items, and on others we are getting negative values.

That means the ones that have positive values are sequentially tab-able using keyboard. The focus will go to tabindex="1" first, then to tabindex="2", and so forth. However, those other ones with tabindex="-1" cannot be focused using keyboard.

The logic in lime-list is adding these tabindexes, and incrementally increases these numbers.

  1. First of all, this is not needed. All of them can simply have tabindex="0" and their logical order in the list in the DOM will make them sequentially tabable, automatically. https://developer.mozilla.org/en-US/docs/Web/HTML/Global_attributes/tabindex
  2. The logic also checks whether there is a disabled list item, and or a separator list item. They will also get a tabindex="-1". This is perhaps alright, and what we should have. But something messes up here. I guess every item before the disabled ones will also get a tabindex="-1".

@Kiarokh
Copy link
Contributor

Kiarokh commented Sep 13, 2023

The keyboard navigation bug in limel-list also affects menus. You cannot fully navigate the list of items using arrow keys. Sometimes, the outlined item gets stuck and you cannot go further than that. But perhaps because limel-menu has its own private list component (limel-menu-list), fixing this issue is easier there? Not sure.

Screen.Recording.2023-09-13.at.15.31.16.mov

☝️ This video is from an upcoming PR.

@adrianschmidt
Copy link
Contributor Author

The issue you are encountering is to do with putting multiple lists in the same menu (or so is my guess). The same happens in the "user menu" in the web client, where you can only arrow-navigate between the two items in the menu "header". To get to the second list you need to tab (if I recall correctly) and then you can arrow-navigate between those items instead.

I think we have the same problem in menus that have fixed items, like when you have a limel-picker, and there's a create-new option at the bottom.

I'm not sure if the problem described in this PR and the problem with having multiple lists in the same menu are related. My initial guess would be that they are not, but it could be that the solution for both is very closely related, even if the underlying causes are different… 🤔

@Kiarokh
Copy link
Contributor

Kiarokh commented Sep 14, 2023

I remember which one you mean. But this is not the same problem. There is only one list element in the menu here.

We have a script in limel-list (and an exact copy of it in limel-menu-list) that sets tabindexs on menu items.

https://github.com/Lundalogik/lime-elements/blob/663b36794335acbcf7a659736fe8c68c76bf1ecb/src/components/list/list-renderer.tsx#L81C33-L81C33

I think the idea behind it as explained here is that:

  • the first menu item should have tabindex="0" so that the user who is tabbing in the UI can find the first element using the tab key,
  • the currently focused/selected menu item (selected by click, or using arrow keys) should get a tabindex="0"
  • everything else should always have tabindex="-1", even the first item (if there is already an item that is focused/selected). This is because when users use the tab key again, they should leave the list element to other tabbable elements in the UI. And if they tab-back to the list (shift+tab), the last selected item should be focused.
  • additionally, separator lines in the list and disabled list items shouldn't have tabindex.

There is some more logic to it too, like when the items are radio buttons, or checkboxes etc… I don't understand how the script is set up to work. It just looks a too complicated for me. And it doesn't seem that the script works too well. In this case, the last item doesn't have a tabindex at all, which is probably why you cannot focus-select it suing the arrow keys.

Screenshot 2023-09-14 at 13 12 08

I guess the whatever script that is responsible for the arrow keys to work, require the tabindex attribute on the list items to exist and be -1.

I also noticed other issues with this script. It doesn't matter if a list item has disabled: true,. It's still focusable when you use the arrow keys (which is a bad accessibility practice it seems). The only reason we don't see the outline style of the focsed item on disabled items is that we have written the styles in a way that you don't see the outline. But in reality, it is focused.

@Kiarokh
Copy link
Contributor

Kiarokh commented Sep 14, 2023

@TommyLindh2 please check the script as we discussed 🙏

@adrianschmidt
Copy link
Contributor Author

I think that script might be part of MDC, and the reason it's not working so well could be that we're doing things MDC doesn't expect. If it is indeed from MDC, perhaps we could ditch that script and write our own instead.

I also noticed other issues with this script. It doesn't matter if a list item has disabled: true,. It's still focusable when you use the arrow keys (which is a bad accessibility practice it seems). The only reason we don't see the outline style of the focsed item on disabled items is that we have written the styles in a way that you don't see the outline. But in reality, it is focused.

Do you have any source for that (the part I bolded)? I would have thought that disabled items should still be focusable, so that a non-sighted user can get the same information that a sighted user gets (there's a menu choice here, label says "blah blah", it's currently disabled). But perhaps that information is already read by the screen reader when the menu is opened? And in that case, it totally makes sense that you'd want to skip past any disabled options.

Ugh… I feel like we really can't make good decisions in questions like these without at least trying things out with a screen reader 😞 I mean, even then we probably couldn't make good decisions, unless we also learned more (or at least more than I currently know) about how real users of screen readers use them. With any graphical UI, there can be things that seem great when you just click around a bit to try something out, but when you actually start using the feature, all of a sudden that behaviour that felt like a feature is now the most irritating thing in the world. I'm sure there are similar things with auditory interfaces too.

@Kiarokh
Copy link
Contributor

Kiarokh commented Sep 15, 2023

I also used to think like you. But I also knew when you have a disabled button on the page, you cannot select it using the tab key <button type="button" disabled>Click Me!</button>. Tabbing basically skips over the button, and this is the default browser behavior. So I thought this is the way it should be logically, right? Otherwise, why browsers default to that? Do they no consider screen-readers? 🤷

Then I asked my enemy (Chat GPT) if this was supposed to be like that. The stupid confident robot said:
image
So basically confirmed my hypothesis. Yesterday I was telling @TommyLindh2 and @BjornOstberg and @LucyChyzhova exactly what you said. "I have no idea how a non-sighted user is supposed to know that they skipped the button then". Anyhow, now after you asked too, I went back to my enemy and said:
image
This is exactly what the script is trying to do and exactly what MDC has explained for their list items. (Since list items aren't tabbable by default, they force them to be tabbable with the script).

Anyhow... let's ignore this part of the discussion for now.

@TommyLindh2 has figured out why this keyboard navigation problem happens 🥳 and he will soon make a PR with explanations about the source. He wanted to talk to you and Johan about it.

@adrianschmidt
Copy link
Contributor Author

What!? tabindex="-1" makes it possible to tab to the element when a screen reader is used, but not otherwise? Really!? If that's true, that's really cool! 😲

@adrianschmidt
Copy link
Contributor Author

I know you said to ignore this part of the discussion, but I just wanted to add a question I asked ChatGPT (with GPT-4), and the response I got:

Prompt:

I just now was told that if you give an element tabindex="-1", while that normally makes it so the element is skipped when tabbing between elements on a web page, if you are using a screen reader, such elements are actually possible to tab to. Is that true? Please work it out step by step, so we're sure we get this right.

Response:

Certainly! Let's break down the behavior of the tabindex attribute, especially when its value is set to -1.

tabindex Attribute

The tabindex attribute specifies the tab order of an element. It can take three types of values:

  1. Positive integer (e.g., tabindex="1")

    • Elements with a positive tabindex value are added to the natural tab order based on the value of the tabindex.
    • This can be problematic because it can disrupt the natural tab order, making navigation confusing for keyboard users.
  2. tabindex="0"

    • The element can be focused and is added to the natural tab order of the page.
    • This is useful for elements that are not natively focusable (like <div>, <span>) but need to be focusable due to interactive behavior added via JavaScript.
  3. tabindex="-1"

    • The element can be programmatically focused (using JavaScript's focus() method), but it is not reachable via sequential keyboard navigation (i.e., it's skipped when tabbing).
    • This is useful for elements that should not be tabbed to but might need to receive focus in certain scenarios, like when a modal is opened.

Screen Readers and tabindex="-1"

Now, let's address the behavior of elements with tabindex="-1" when using screen readers:

  • Without a screen reader: Elements with tabindex="-1" are skipped during normal tabbing. They can't be focused by pressing the Tab key.

  • With a screen reader: The behavior can vary based on the screen reader and its settings. Some screen readers, in their default settings, allow users to navigate to all focusable elements, including those with tabindex="-1". This is because screen readers provide mechanisms to navigate through all elements, not just those in the tab order. For example:

    • NVDA: By default, when navigating with the arrow keys, NVDA will read out elements with tabindex="-1". However, these elements are still skipped when tabbing.
    • VoiceOver: Similar to NVDA, VoiceOver will read out elements with tabindex="-1" when navigating with the arrow keys but will skip them when tabbing.
    • JAWS: The behavior is similar to NVDA and VoiceOver.

Conclusion

While elements with tabindex="-1" are skipped during normal tabbing, they can still be accessed and read out by screen readers when users navigate with mechanisms other than tabbing (e.g., arrow keys). This allows screen reader users to be aware of all elements on the page, even if they aren't in the natural tab order.

However, it's essential to test with actual screen readers to ensure the desired behavior, as there can be variations based on the screen reader, its version, and its settings.


Looks promising 👍

@Kiarokh
Copy link
Contributor

Kiarokh commented Sep 15, 2023

This is really cool, and I'm happy we went through this now. It opened up a bunch of closed doors in my head; and I realize how many accessibility problems we currently have in many of our components 🙈 I think we will open a bunch of new PRs for these.

We should also enable a way of navigating clickable elements within some of our current components, like the action bar, dock, or the upcoming breadcrumbs but toggling tabindexes. We should then also allow users to navigate them using arrow-keys instead of tabbing like a maniac between the internal buttons in these components.

@adrianschmidt
Copy link
Contributor Author

We should also enable a way of navigating clickable elements within some of our current components, like the action bar, dock, or the upcoming breadcrumbs but toggling tabindexes. We should then also allow users to navigate them using arrow-keys instead of tabbing like a maniac between the internal buttons in these components.

Yeah, that's what MDC typically does, or tries to do 😅. If there's a collection of focusable things of some kind, a list or something, it lets you tab to the selected one, if one is selected, or the first element if none is selected (not sure if it does the first element or the first selected element if many are selected, but anyway). You can then use the arrow keys to move between the different items. I'm not sure if that actually moves the focus or just MDC's own visual highlight. But that lets you both quickly tab past a giant list, and navigate through the list. And it's probably compatible with screen readers too then… 👍

@Kiarokh
Copy link
Contributor

Kiarokh commented Sep 18, 2023

Made this issue to follow this up #2486

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Status: All Open Issues
Development

No branches or pull requests

2 participants