sveltejs / svelte

Cybernetically enhanced web apps
https://svelte.dev
MIT License
78.66k stars 4.13k forks source link

A11y: noninteractive element cannot have positive tabIndex value when using tabindex="0" #7953

Open charbelnicolas opened 1 year ago

charbelnicolas commented 1 year ago

Describe the bug

I don't really know where to post this bug but I keep getting the same warning when using tabindex="0":

20221015_104155

20221015_104249

If you know where I should report this bug, let me know!

Reproduction

Add a tabindex="0" to any div element and see for yourself :)

Logs

No response

System Info

System:
    OS: Linux 6.0 Arch Linux
    CPU: (8) x64 Intel(R) Core(TM) i7-6700K CPU @ 4.00GHz
    Memory: 29.44 GB / 31.31 GB
    Container: Yes
    Shell: 5.1.16 - /bin/bash
  Binaries:
    Node: 18.11.0 - /usr/bin/node
    Yarn: 1.22.19 - /usr/bin/yarn
    npm: 8.19.2 - /usr/bin/npm
  Browsers:
    Chromium: 106.0.5249.119

Severity

annoyance

Conduitry commented 1 year ago

I can reproduce this in 3.52.0. It's coming from #6693. @tanhauhau since you merged this, do you have an opinion about it? The message says "positive tabindex values" but the check in the code is for >= 0. Do you know what we want to be checking for? There's also a comment on that PR from a couple days ago that would seem to indicate that this new rule is overzealous.

janosh commented 1 year ago

In case this rule is working as intended, I'd be curious how your supposed to make a list tabbable without using tabindex="0"?

<li tabindex="0">foo</li>
tanhauhau commented 1 year ago

@Conduitry all a11y rules are based on eslint-plugin-jsx-a11y, and this particular rule comes from https://github.com/jsx-eslint/eslint-plugin-jsx-a11y/blob/main/docs/rules/no-noninteractive-tabindex.md

adding an interactive role to the <div> element instead of a generic div would help resolve the issue.

@janosh also according to the eslint-plugin-jsx-a11y, It is not necessary to put a tabindex on an

, for instance or on
  • items; assistive technologies provide affordances to users to find and traverse these containers.

  • janosh commented 1 year ago

    @janosh also according to the eslint-plugin-jsx-a11y, It is not necessary to put a tabindex on an

    Yeah, I saw that. But my use case is non-assistive. I want to be able to tab-traverse list items myself. It's for this ToC component: https://svelte-toc.netlify.app.

    adding an interactive role to the

    element instead of a generic div would help resolve the issue.

    Thanks. role=link sounds like a good solution.

    <li tabindex="0" role="link">foo</li>
    charbelnicolas commented 1 year ago

    @Conduitry all a11y rules are based on eslint-plugin-jsx-a11y, and this particular rule comes from https://github.com/jsx-eslint/eslint-plugin-jsx-a11y/blob/main/docs/rules/no-noninteractive-tabindex.md

    adding an interactive role to the <div> element instead of a generic div would help resolve the issue.

    @janosh also according to the eslint-plugin-jsx-a11y, It is not necessary to put a tabindex on an

    , for instance or on

    • items; assistive technologies provide affordances to users to find and traverse these containers.

    Honestly, it doesn't matter what eslint-plugin-jsx-a11y says, it's a poorly worded, misleading warning... When did 0 become a positive number?

    janosh commented 1 year ago

    When did 0 become a positive number?

    @charbelnicolas It's debatable. I mostly agree with your stance but in mathematics, people often use positive to include zero and the term "strictly positive" when they want to exclude it.

    oatymart commented 1 year ago

    This rule is also triggered in a number of situations, for example when tabindex is set to -1, undefined, or the result of a ternary expression containing either of these.

    Repro: https://svelte.dev/repl/0bd4ce269ee64d48a51929a95be6d6e7?version=3.53.1

    I would add a role to indicate interactivity, but I don't know of one that represents a draggable or droppable area.

    benignant commented 1 year ago

    In my case, I have a noninteractive element with a tooltip. I want the tooltip to open when a sighted user tabs to the element. The element should not have an interactive role, as it is only informational. Screen reader users will get the same information that is provided in the tooltip. So it doesn't make sense to use an interactive element or role like button.

    Tabbing isn't only for screen reader users.

    ecker00 commented 1 year ago

    Also getting this warning, and good accessibility is important. Given a very long list of searchable items where the user can select one option, would the correct schematics be something like this?

    <ul role="radiogroup">
      <li role="radio"
        tabindex="0"
        aria-checked="false/true"
        on:click={select}
        on:keyup={select}>
        <!-- content -->
      </li>
    </ul>

    (Given select() handles mouse and keyboard event.key === 'Enter')

    Previously not been using role attribute, and role=button/link seems misleading if it's a list where one option can be selected.

    Was thinking the default role="listitem" of an <li> was acceptable, but this non-interactive element A11y warning thinks otherwise.