Git Product home page Git Product logo

tabbable's Issues

Seeking co-maintainers!

I've been shifting my focus away from UI development, so don't plan on addressing new issues myself. If you use this library and want to see development continue, you can make that happen by becoming a co-maintainer — with permissions to triage issues, merge PRs, cut releases, etc.

Please comment below if you're interested!

Another possibility is for a dedicated owner to fork this code and create a new package. I'd be happy to link to that library from the README of this one.

Make this package tree-shakeable

I'm building a UI library which exports a bunch of stuff, only some of my components rely on this package so it would be desirable that if a consumer of my package doesn't use component relying on tabbable it would be tree shaken out of the final bundle. However it's not the case right now.

Let's focus on 2 common tree-shaking deopts:

  • no ESM bundle (this package) exports only CJS format
  • top-level static properties, as in module.exports = tabbable; tabbable.isTabbable = isTabbable;

Action items:

  • migrate source to ESM + build CJS format from it, point to CJS from package.json#main and to ESM from package.json#module so bundlers can pick appropriate file
  • change exports shape to export default tabbable; export { isTabbable, isFocusable };

Intent to implement - yes (was already implemented in #38 )

Update all dependencies

Dependencies haven't been updated in a while, and focus-trap, which depends on this module, uses Webpack now after focus-trap/focus-trap#124, so I'm also going to change the build system to Webpack.

I hope to address #51 while I'm at it.

Also along with focus-trap/focus-trap#124, and along with focus-trap/focus-trap-react#78, I'll be updating the ESLint and Prettier configuration to match. This way, all 3 repos now number the https://github.com/focus-trap "roof" will use the same stack and the same linting/formatting rules.

The project will move to Yarn since the other 2 repos use Yarn also.

Finally, I'll switch the CI to a GitHub Workflow, just like the other 2 repos are using now.

Restore actual browser testing with Cypress

We removed Karma in #226 but that removed browser-based testing. tabbable, focus-trap, and focus-trap-react now all use the @testing-library and focus-trap/react use Cypress. Let's add Cypress tests to tabbable.

tabbable breaks when processing an element with a "scope" attribute

I have a situation where a TH element has both scope="col" and tabindex="0" attributes, which is causing Tabbable to break with "Cannot read properties of undefined (reading 'forEach').

index.js:425 Uncaught TypeError: Cannot read properties of undefined (reading 'forEach')
at e (index.js:425:14)
at index.js:429:32
at Array.forEach ()
at e (index.js:425:14)
at e.tabbable (index.js:473:10)

Tabbable appears to be evaluating !! item.scope as true because the attribute is present, and assumes there is a ShadowRoot to process.

support inert attribute

Elements that are marked with inert or are nested in another inert element shouldn't be tabbable/focusable. Since this is just supported behind browsers flags at the moment, maybe tabbable can add support with a default false flag for now.

Incorrect README import instructions

In the README it states to import this module you need to use a named import instead of a default import.

import { tabbable } from 'tabbable';

tabbable is undefined.

You my want to update it to reflect the need for a default import.

Provide TS typings

While it's possible to type callable signature with static properties in TS it would be better to change exports shape to default export + 2 named exports (which I mention also in #39 ).

It would also be better to use mixed exports, even if it means having to use require('tabbable').default by CJS consumers - because then TS typings would be accurate. Faking default with module.exports = _default is a can of worms and might cause issues in webpack.

Radios in a set are all tabbable but only one at a time should be

When you tab through a radio set (radio inputs sharing a name), only one of them is actually tabbable — one of the following:

  • The one that has been selected.
  • If none are selected and you're moving forward with tab, the first one.
  • If none are selected and you're moving backward with shift-tab, the last one.

This library doesn't take that into account: it treats radio inputs like other inputs, assuming they're all tabbable.

This would be a tricky problem to fix without adding a fair amount of delicate complexity. The best solution might be similar to the iframe problem: #23 (comment) — only hijack tab and shift+tab if you're leaving the focus trap. However, the big blocking failing of this solution remains to be solved: I think the approach will only work if the first and last tabbable elements in the container are recognizable by the library. For example, if the first element that's actually tabbable (according to the browser) is in an iframe or the shadow dom or is a selected radio button that's not the first one of it's group — then how is this library going to know that that's what should receive focus?

Open to solutions.

cc @doodirock

jsdom issue: 'slot):not([inert]' is not a valid selector

If you regenerate package-lock and run tabbable's unit tests, you get the error SyntaxError: 'slot):not([inert]' is not a valid selector
I've narrowed this down to the update of one of jsdom's dependencies: [email protected] to [email protected]
This breaks unit tests for any consumers of tabbable who install latest and use jsdom.

I'm not sure if the fix is re-writing the [tabindex]:not(slot):not([inert]) selector, or if this is a bug that nwsapi needs to fix. Please advise.

Tab into iframe

Tabbable does not select iframes it as candidate, and thus it is impossible to tab into them. This is needed in some scenarios, for instance when you have a reCaptcha (which is constructed as an iframe) in a form. One solution could be to just add 'iframe' to the candidateSelectors, but perhaps there is some issues with that?

Add code coverage badge to the README

Now that the test suite is converted over to use Jest and DOM Testing Library, what do you think about adding a code coverage badge to the README to show off the code coverage?

I see in the focus-trap-react repo we used Codecov along with GitHub Actions for the CI. In this tabbable repo it's also using GitHub Actions for the CI, so I assume we can generate the code coverage report there and then feed the results to Codecov, right? Or maybe even generate the badge without using Codecov if that's not something you want to do, and we could just use GitHub Actions directly?

Thoughts?

Accept more than one container in which to look for tabbable nodes

It'd be nice to be able to pass more than one element to tabbable. The current tabbable doesn't allow us to easily find the relative tab orders of elements in multiple containers, if we were to call tabbable on each container.

Would it be reasonable to add this feature to tabbable? Happy to send a pull request if that's okay.

Example:

Suppose the numbers following "el" represent the tab orders of focusable the elements...

container1: el1, el4, el3
container2: el2, el5, el6

Calling tabbable on container1 and container2 would give us...

container1: el1, el3, el4
container2: el2, el5, el6

However, we wouldn't know how to combine the two lists of els, or know that el2 from container2 should be tabbed to before el3 from container1.

Proposal

Allow tabbable to accept multiple elements: tabbable(el1, el2, ...). tabbable returns an array of tabbable nodes in tab order from all of the passed elements/nodes.

Use cases

  • Composite modals (ie. modals with multiple "dialogs").
  • Non-fullscreen modals/overlays (e.g. overlay does not cover a top nav bar).

couple question

First of all, thank you.

A couple questions; Is it possible to add <summary>/<details> ? What about hidden html attribute ?

getComputedStyle fails with shadow dom

I'm trying to use focus-trap in a StencilJS component and the various checks that rely on getComputedStyle fail when it hits a shadow root.

Pretty sure this could be fixed with a simple check, I'll take a crack at a PR. Thanks!

7vfqzmxv.entry.js:formatted:2 Uncaught TypeError: Failed to execute 'getComputedStyle' on 'Window': parameter 1 is not of type 'Element'.
    at p.hasDisplayNone (7vfqzmxv.entry.js:formatted:2)
    at p.hasDisplayNone (7vfqzmxv.entry.js:formatted:2)
    at p.hasDisplayNone (7vfqzmxv.entry.js:formatted:2)
    at p.hasDisplayNone (7vfqzmxv.entry.js:formatted:2)
    at p.isUntouchable (7vfqzmxv.entry.js:formatted:2)
    at l (7vfqzmxv.entry.js:formatted:2)
    at s (7vfqzmxv.entry.js:formatted:2)
    at i (7vfqzmxv.entry.js:formatted:2)
    at w (7vfqzmxv.entry.js:formatted:2)
    at Object.activate (7vfqzmxv.entry.js:formatted:2)

Can't make it work testing with tabbable mocked

Hi guys, sorry for leaving this here but I can't find any solution. I'm trying to mock tabbable to use with jest and I followed the instructions that you leaved but I'm getting an error "TypeError: tabbable.tabbable is not a function" when test runs.

My root for jest is defined in '/packages/core', so that is why I placed mock folder there. Any ideas where could be the issue?
image

isTabbableRadio doesn't properly escape query selectors

It's possible for radio button names to contain characters that are invalid when used in a CSS selector (e.g. "). Below is an example that causes the tabbable function to throw an error due to an invalid selector constructed at:

'input[type="radio"][name="' + node.name + '"]'

HTML:

<div>
  <input type="radio" id="huey" name="&quot;duck&quot;" value="huey"
         checked>
  <label for="huey">Huey</label>
</div>

<div>
  <input type="radio" id="dewey" name="&quot;duck&quot;" value="dewey">
  <label for="dewey">Dewey</label>
</div>

JS:

import { tabbable } from 'tabbable';
// Throws DOMException: Document.querySelectorAll: 'input[type="radio"][name=""duck""]' is not a valid selector
tabbable(document.body);

It's also possible to inject values into the selector with a name like:

<input type="radio" id="dewey" name="&quot;] .some-class[data-attr=&quot;some-attribute" value="dewey">

A potential solution is CSS.escape, but it does require a polyfill for IE.

Remove nwsapi v2.2.2 override once bug is fixed

Blocked by dperini/nwsapi#83

Relates to #982

Would revert #995

Might want to make sure package-lock.json is also now using the nwsapi version that has the fix instead of being still configured for v2.2.2 (which is why #982 isn't visible unless you regenerate the lock file and NPM picks the current-latest problematic version of nwsapi).

IE 9+ Support

Hi David,
just a brief remark: According to your README file you are supporting IE9+. I noticed that since version 1.0.3 you are now using the Array.prototype.find Method in [1]. This will break in IE <= 11.

[1] index.js#L53

Security policy questions

The Drupal project is considering adding this library as one of our dependencies and so we're performing a standard stability review. We're looking into adopting this in https://www.drupal.org/project/drupal/issues/3113649. I fully acknowledge the likelihood of security issues tabbable are very low, so I particularly appreciate the time taken to answer these.

Since there isn't a policy at https://github.com/focus-trap/tabbable/security I'm curious if you have any official policies documented somewhere regarding:

Security releases
For example, does more than one version receive security fixes, or only the current version? It looks like 5.x is the first after the maintainer switch, so this may not apply currently, but would like to know if it would be applicable with a >=6.x release.
Release windows/cadence
For example, do they happen as necessary on any given day, or on a set schedule after a certain passage of time (e.g. once a month)? Looking at all the recent releases (which is great to see!), I can probably make some assumptions, but would like to confirm.
Backwards compatibility guarantees
Tabbable uses semver, so I assume the major version promises not to break BC. Are there any guarantees that a geven version will be supported for some period of time (an LTS version, for example), also with the understanding that things possibly changed between 4 and 5?

Thanks, I'm pleased to see all the recent activity on this library!

Contenteditable

jQuery UI isn't picking up on it either, but I think contenteditable elements should be tabbable. You can try here: codepen

The fix is easy, I think; just add [contenteditable]:not([contenteditable="false"]) to list of candidate selectors.

Anchors without an href and with a tabindex of 0 don't get picked up

This can be fixed by changing line 16 of index.js from..

|| (candidate.tagName === 'A' && !candidate.href && !candidate.tabIndex)

to..

|| (candidate.tagName === 'A' && !candidate.href && (!candidateIndex && candidateIndex !== 0))

I won't be using tabbable this way, but someone might be.

version 5.1.1 contains an arrow function

I am trying to use the library in internet explorer 11

in the transpiled version there is an arrow function
I can see it coming from line 49

let tabbableNodes = orderedTabbables
    .sort(sortOrderedTabbables)
    .map((a) => a.node)
    .concat(regularTabbables);

https://github.com/focus-trap/tabbable/blob/master/src/index.js#L49

We have two options, we can use a traditional function expression in the index.js, or we can configure the babel settings to have a target of internet explorer 9 to match the version support in the docs

Optimizing displayCheck: 'full'

It is possible to assess whether an element, or one of its ancestors, have display: none without having to traverse the DOM.

This condition can be simplified to:

if (!displayCheck || displayCheck === 'full') {
  return !node.getClientRects().length;
}

I actually don't know if checking for getClientRects().length is really equivalent to that expensive while loop.

According to the specs, the getClientRects() method returns an empty list

If the element on which it was invoked does not have an associated layout box

What exactly are all the cases in which an element does not have an associated layout box? I am not sure.

But the e2e tests currently present in the project are all passing.
However I didn't go through the tests myself and I don't know what they do, I only checked this very small set of possible edge cases and it looks like the method should be good enough.

May I open a PR?

By the way, since the getBoundingClientRect() method internally uses getClientRects(), the "full" option would become faster than the "non-zero-area" one. But I guess that the latter would still be useful for accessibility purpose.

IE11, svg with tabindex=0 does not have tabIndex property

In IE11, a SVG with tabindex="0" does not have a tabIndex property so gets added to the orderedTabbables array no matter where it is in the DOM.

candidateIndex = parseInt(candidate.getAttribute('tabindex'), 10) || candidate.tabIndex;

Always triggers the OR condition if tabindex="0", which then assumes tabIndex is a valid property. SVGs return undefined for the tabIndex property.

Tab Across Web Components

It would be great if tabbable could identify the tab ordering between input fields across multiple web components.

Unfortunately since query selectors can't span shadow roots, it may require a very different approach.

v5.3.0 breaks literally all of our usage of `tabbable` 😅 when called on a node not attached to the document

The change to use getClientRects().length completely breaks this library for us: #604

We use tabbable when initializing dialogs, to find candidate element(s) to auto-focus when the dialog appears. These dialogs are React components and the call to tabbable() occurs during the componentDidMount lifecycle method. At that moment, the DOM elements we want to traverse using tabbable are fully formed, but they are not yet attached to the document. So getClientRects().length always returns 0, and thus every single one of our calls to tabbable fails. (At this library level, we have no way to control when the dialog is actually attached to the document.)

Could this change be reverted, or can we scope it to only be used when document.body.contains(node)?

Firefox throws an error when checking disabled on non button element

Hi,

I am not sure if this is related to a recent change of Firefox, but today I got this strange error:

image

After checking this relates to this line: https://github.com/focus-trap/tabbable/blob/master/src/index.js#L405

Apparently, Firefox seems to throw an exception whenever you access the disabled attribute on an element that does not implement it.

Chrome does not experience any issue, but this should probably be changed, by checking the attribute instead.

Thanks :)

Option to check a container too

Right now only nodes within a container get tested, but not a container itself. I have use case when i want a container to be possibly included in resulted array.

In current state i may get what i need, by retrieving tabbables of a parent of a container and then filtering results by inclusion of container. My concern is that of performance. Siblings of a container can be big, while container itself may be small or empty, it may result in much of unnecessary work.

Another way is to add option which will tell to prepend a container to candidates list if it matches one of selectors.

What are your opinion on adding such option? It is ok or is it out of scope of this library?

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.