Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

fix(tab): exclude hidden descendants #579

Merged
merged 4 commits into from
Mar 9, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 22 additions & 2 deletions src/__tests__/tab.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ test('fires events when tabbing between two elements', () => {
userEvent.tab()
expect(getEventSnapshot()).toMatchInlineSnapshot(`
Events fired on: div

input#a[value=""] - keydown: Tab (9)
input#a[value=""] - focusout
input#b[value=""] - focusin
Expand Down Expand Up @@ -45,7 +45,7 @@ test('does not change focus if default prevented on keydown', () => {
userEvent.tab()
expect(getEventSnapshot()).toMatchInlineSnapshot(`
Events fired on: div

input#a[value=""] - keydown: Tab (9)
input#a[value=""] - keyup: Tab (9)
`)
Expand Down Expand Up @@ -418,6 +418,26 @@ test('should not focus disabled elements', () => {
expect(five).toHaveFocus()
})

test('should not focus elements inside a hidden parent', () => {
setup(`
<div>
<input data-testid="one" />
<div hidden="">
<button>click</button>
</div>
<input data-testid="three" />
</div>`)

const one = document.querySelector('[data-testid="one"]')
const three = document.querySelector('[data-testid="three"]')

userEvent.tab()
expect(one).toHaveFocus()

userEvent.tab()
expect(three).toHaveFocus()
})

test('should keep focus on the document if there are no enabled, focusable elements', () => {
setup(`<button disabled>no clicky</button>`)
userEvent.tab()
Expand Down
19 changes: 18 additions & 1 deletion src/__tests__/utils.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import {isInstanceOfElement} from '../utils'
import { screen } from '@testing-library/dom'
import {isInstanceOfElement, isVisible} from '../utils'
import {setup} from './helpers/utils'

// isInstanceOfElement can be removed once the peerDependency for @testing-library/dom is bumped to a version that includes https://github.com/testing-library/dom-testing-library/pull/885
Expand Down Expand Up @@ -71,3 +72,19 @@ describe('check element type per isInstanceOfElement', () => {
expect(() => isInstanceOfElement(element, 'HTMLSpanElement')).toThrow()
})
})

test('check if element is visible', () => {
setup(`
<input data-testid="visibleInput"/>
<input data-testid="hiddenInput" hidden/>
<input data-testid="styledHiddenInput" style="display: none">
<input data-testid="styledDisplayedInput" hidden style="display: block"/>
<div style="display: none"><input data-testid="childInput" /></div>
`)

expect(isVisible(screen.getByTestId('visibleInput'))).toBe(true)
expect(isVisible(screen.getByTestId('styledDisplayedInput'))).toBe(true)
expect(isVisible(screen.getByTestId('styledHiddenInput'))).toBe(false)
expect(isVisible(screen.getByTestId('childInput'))).toBe(false)
expect(isVisible(screen.getByTestId('hiddenInput'))).toBe(false)
})
8 changes: 6 additions & 2 deletions src/tab.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import {fireEvent} from '@testing-library/dom'
import {getActiveElement, FOCUSABLE_SELECTOR} from './utils'
import {getActiveElement, FOCUSABLE_SELECTOR, isVisible} from './utils'
import {focus} from './focus'
import {blur} from './blur'

Expand Down Expand Up @@ -31,7 +31,11 @@ function tab({shift = false, focusTrap} = {}) {
const enabledElements = [...focusableElements].filter(
el =>
el === previousElement ||
(el.getAttribute('tabindex') !== '-1' && !el.disabled),
(el.getAttribute('tabindex') !== '-1' &&
!el.disabled &&
// Hidden elements are not tabable
isVisible(el)
),
)

if (enabledElements.length === 0) return
Expand Down
14 changes: 14 additions & 0 deletions src/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -293,6 +293,19 @@ function isClickableInput(element) {
)
}

function isVisible(element) {
const getComputedStyle = getWindowFromNode(element).getComputedStyle

for(; element && element.ownerDocument; element = element.parentNode) {
const display = getComputedStyle(element).display
if (display === 'none') {
return false
}
}

return true
}

function eventWrapper(cb) {
let result
getConfig().eventWrapper(() => {
Expand Down Expand Up @@ -367,4 +380,5 @@ export {
getSelectionRange,
isContentEditable,
isInstanceOfElement,
isVisible,
}