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.

Sign up for GitHub

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

Jump to bottom

Elevate tabIndex config from TextField to Field #8010

Closed
ghulamghousdev opened this issue Dec 7, 2023 · 5 comments
Closed

Elevate tabIndex config from TextField to Field #8010

ghulamghousdev opened this issue Dec 7, 2023 · 5 comments
Assignees
@dongryphon
Labels
forum Issues from forum large-account Reported by large customer OEM OEM customer resolved Fixed but not yet released (available in the nightly builds)
Milestone
6.0.0-alpha-1

Comments

@ghulamghousdev
Copy link
Member

Forum post

The tabIndex configuration must be in the Field class, not in the TextField class. Because all input elements have the tab index attribute!
@ghulamghousdev ghulamghousdev added forum Issues from forum large-account Reported by large customer OEM OEM customer labels Dec 7, 2023
@ExtAnimal
Copy link

DateTime field will have to use tabIndex and tabIndex+1 since it contains two nous fields.

@henriquewerlang
Copy link

Theoretically, the tabindex of DateTime field, must be of the container of the fields and the internal tabindex must be 0 and 1, respectively.

@henriquewerlang
Copy link

Reading the Mozzila documentation, any element can have the "TabIndex" attribute, with this info, I guess this configuration must be moved to the Widget class.

@dongryphon
Copy link

dongryphon commented Dec 30, 2023 •
edited

Moving this to Widget makes sense but will present some special case issues: imagine assigning it to a Calendar or a Scheduler or Grid. While these makes no sense, it is still correct for many cases.

As for compound widgets like DateTime, one must set the same tabIndex on all relevant focusable elements or you'll get surprising behavior. See CodePen:

<html>
  <body>
    <input type=text tabindex="0" value="First (tabIndex=0)"></input>
    <input type=text tabindex="1" value="Second (tabIndex=1)"></input>
    <input type=text tabindex="0" value="Third (tabIndex=0)"></input>
    <input type=text tabindex="0" value="Fourth (tabIndex=0)"></input>
    <input type=text tabindex="1" value="Fifth (tabIndex=1)"</input>
  </body>
</html>

Ignoring the browser chrome, the tab sequence for the above is: First, Third, Fourth, Second, Fifth (repeat).

tabIndex-behavior.mp4

There are good reasons why MDN warns against assigning tabIndex to anything other than 0 (to make non-tabbable elements participate w/naturally tabbable elements) and -1 (to remove tabbability from naturally tabbable elements or allow programmatic focus assignment to non-focusable elements). Anything else is a lot of complexity and likely an a11y problem.

@dongryphon dongryphon changed the title Move tabIndex config to Field.js Move tabIndex config to Widget Dec 30, 2023
@dongryphon dongryphon self-assigned this Dec 30, 2023
@dongryphon
Copy link

There are a lot of complexities in hoisting this to Widget due to the complex rendering in many advanced components and the fact that so many are not focusable themselves but contain a number of focusable elements. It is not clear what Widget can really offer as a baseline meaning for this config because it is not clear the element(s) to which it should be applied (if any).

Without a clear use case for when to ignore the general pattern of 0/-1, it seems best to hoist tabIndex to Field as originally requested and do minimal work propagating its value.

For example, DateTime could be configured like so:

    {
        type : 'datetimefield',
        tabIndex : 1
    }

and that would be equivalent to:

    {
        type : 'datetimefield',
        dateField : { tabIndex : 1 },
        timeField : { tabIndex : 1 }
    }

@dongryphon dongryphon changed the title Move tabIndex config to Widget Elevate tabIndex config to Field Dec 31, 2023
@dongryphon dongryphon changed the title Elevate tabIndex config to Field Elevate tabIndex config from TextField to Field Dec 31, 2023
@dongryphon dongryphon added the ready for review Issue is fixed, the pull request is being reviewed label Dec 31, 2023
@dongryphon dongryphon mentioned this issue Jan 3, 2024
Closed
@isglass isglass added resolved Fixed but not yet released (available in the nightly builds) and removed ready for review Issue is fixed, the pull request is being reviewed labels Jan 4, 2024
@isglass isglass modified the milestones: 6.0.0, 6.0.0-alpha-1 Jan 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@dongryphon dongryphon

Labels
forum Issues from forum large-account Reported by large customer OEM OEM customer resolved Fixed but not yet released (available in the nightly builds)
Projects
None yet
Milestone
6.0.0-alpha-1
Development

No branches or pull requests
6 participants
@ExtAnimal @dongryphon @isglass @henriquewerlang @ghulamghousdev @SergeyMaltsev