Skip to content

Commit

Permalink
fix: element not allowed to be disabled being returned as disabled (#261
Browse files Browse the repository at this point in the history 
)
  • Loading branch information
@lourenci
lourenci authored Jun 14, 2020
1 parent f8d3095 commit 5e39222
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 3 deletions.
10 changes: 10 additions & 0 deletions src/__tests__/to-be-disabled.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ test('.toBeDisabled', () => {
</optgroup>
</select>
</div>
<a href="http://github.com" data-testid="deep-a-element">x</a>
</fieldset>
<a href="http://github.com" disabled={true} data-testid="a-element">x</a>
Expand All @@ -50,7 +51,11 @@ test('.toBeDisabled', () => {
expect(queryByTestId('deep-option-element')).toBeDisabled()

expect(queryByTestId('a-element')).not.toBeDisabled()
expect(queryByTestId('deep-a-element')).not.toBeDisabled()
expect(() => expect(queryByTestId('a-element')).toBeDisabled()).toThrowError()
expect(() =>
expect(queryByTestId('deep-a-element')).toBeDisabled(),
).toThrowError()
})

test('.toBeDisabled fieldset>legend', () => {
Expand Down Expand Up @@ -129,6 +134,7 @@ test('.toBeEnabled', () => {
</optgroup>
</select>
</div>
<a href="http://github.com" data-testid="deep-a-element">x</a>
</fieldset>
<a href="http://github.com" disabled={true} data-testid="a-element">x</a>
Expand Down Expand Up @@ -173,6 +179,10 @@ test('.toBeEnabled', () => {
expect(() =>
expect(queryByTestId('a-element')).not.toBeEnabled(),
).toThrowError()
expect(queryByTestId('deep-a-element')).toBeEnabled()
expect(() =>
expect(queryByTestId('deep-a-element')).not.toBeEnabled(),
).toThrowError()
})

test('.toBeEnabled fieldset>legend', () => {
Expand Down
17 changes: 14 additions & 3 deletions src/to-be-disabled.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,12 @@ function isElementDisabledByParent(element, parent) {
)
}

function canElementBeDisabled(element) {
return FORM_TAGS.includes(getTag(element))
}

function isElementDisabled(element) {
return FORM_TAGS.includes(getTag(element)) && element.hasAttribute('disabled')
return canElementBeDisabled(element) && element.hasAttribute('disabled')
}

function isAncestorDisabled(element) {
Expand All @@ -49,10 +53,17 @@ function isAncestorDisabled(element) {
)
}

function isElementOrAncestorDisabled(element) {
return (
canElementBeDisabled(element) &&
(isElementDisabled(element) || isAncestorDisabled(element))
)

This comment has been minimized.

Sign in to view

Copy link
@alexmund-bbc

alexmund-bbc Jul 8, 2020 •
edited
Loading

I think there's a bug here!

I had this DOM:

<button disabled>
    <span>Hello</span>
</button>

And this working test:

const button = target.getByText('Hello'); // actually a <span> in the <button>
expect(button).toBeDisabled();

I upgraded @testing-library/jest-dom from 4.2.4 to 5.11.0, and now my test is failing.

I think the problem is that, because a cannot be disabled, the toBeDisabled matcher is not checking whether any ancestor ( in this case) is disabled.

This comment has been minimized.

Sign in to view

Copy link
@alexmund-bbc

alexmund-bbc Jul 8, 2020

Reading this discussion (#265), I think this is intended behaviour.

Therefore, the solution is for me to change my test code to:

const button = target.getByRole('button', { name: 'Hello' });

or:

const button = target.getByText('Hello').closest('button')

then expect(button).toBeDisabled().

👍
}

export function toBeDisabled(element) {
checkHtmlElement(element, toBeDisabled, this)

const isDisabled = isElementDisabled(element) || isAncestorDisabled(element)
const isDisabled = isElementOrAncestorDisabled(element)

return {
pass: isDisabled,
Expand All @@ -71,7 +82,7 @@ export function toBeDisabled(element) {
export function toBeEnabled(element) {
checkHtmlElement(element, toBeEnabled, this)

const isEnabled = !(isElementDisabled(element) || isAncestorDisabled(element))
const isEnabled = !isElementOrAncestorDisabled(element)

return {
pass: isEnabled,
Expand Down

0 comments on commit 5e39222

Please sign in to comment.