Add support for aria-disabled in toBeDisabled/toBeEnabled

[pwolaq]

What:

• Add support for aria-disabled in toBeDisabled/toBeEnabled add aria-disabled in the toBeDisabled matcher #144
• Temporarily remove floating jsdom dependency until it is fixed latest version of jsdom (15.2.0) causes focus test to fail #145
• Few changes in README after running npm run format

Closes #144

Checklist:

• Documentation
• Tests
• Updated Type Definitions N/A
• Ready to be merged
  

[gnapse]

Looks good. Some comments.

[gnapse]

I wonder if there's some issue when having conflicting signals in an element that has different values for disabled and aria-disabled. Not saying we need to do anything about it, just wondering.

[gnapse]

Not sure if this is a fix, a feat or even a BREAKING CHANGE (maybe improbable but I can see how something that before was enabled can now make a test fail because it was really disabled via a aria-disabled attribute present 🤔, but not sure if I'm overthinking this) cc. @testing-library/core-maintainers

[eps1lon]

The problem is that aria-disabled is only an atrribute for assistive technology while the disabled property has more implications (cant click or focus etc). So this matcher asserts very different things depending on which attribute is used. So I do think your test should reflect that.

[gnapse]

Hmmmm, you bring up a good point @eps1lon but so much so that I now wonder if these two slightly different concepts of disabling an element should be expressed under the same matcher. I was indeed also worried about what to do when we have conflicting signals (e.g. <span disabled aria-disabled="false" />).

I'd say that we make it so we issue an error when we see that the disabled attribute does not agree with the aria-disabled attribute when both are present. But other than that, I'd keep the current logic of considering either one of them a valid signal of wether the target element is disabled or not. And not bother with wether it's a "hard" disable state (disabled) or a "soft" one (aria-disabled). Unless you can expand more on your concerns about this.

BTW, I'm also not sure how our tests should reflect what you bring up. Are you suggesting that we test that we can or cannot focus or click on elements accordingly?

[eps1lon]

BTW, I'm also not sure how our tests should reflect what you bring up. Are you suggesting that we test that we can or cannot focus or click on elements accordingly?

If someone researches how disabled should behave for each element then we can make this work. Otherwise I would caution against introducing this since it gives the impression that the aria-disabled attribute asserts the same thing as the disabled property (focus and click being the most important but other events as well like mouseEnter).

[gnapse]

Just wanted to note that the discussion has apparently been moved to the issue (#144). Just so that we avoid having duplicate discussions about this, I guess the issue is better suited for this, since the objections are not with the implementation but with the feature proposal. Also @eps1lon already took the lead.

[gnapse]

Hi,

I'm closing this for the same reasons stated in #144 (comment). Feel free to continue the discussion there if you think there's something else yet to be said or discussed.