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.

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

toBeChecked suggested to replace check for aria-checked="mixed" #43

Open
gnapse opened this issue May 6, 2020 · 4 comments
Open

toBeChecked suggested to replace check for aria-checked="mixed" #43

gnapse opened this issue May 6, 2020 · 4 comments
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@gnapse
Copy link
Member

gnapse commented May 6, 2020

  • eslint-plugin-jest-dom version: 2.1.0

Relevant code or config

expect(element).toHaveAttribute("aria-checked", "mixed")

What you did:

I want to assert that the checked status is "mixed". It generally is only "true" or "false", but it can also be "mixed".

What happened:

The rule to prefer toBeChecked kicked in, but I cannot assert what I want with that custom matcher.

Suggested solution:

I think we need to either make an exception here with "mixed", but that would also be a problem, because for legibility in my test I'd like to use the same kind of assertion as my checkbox state changes. So it would not be ideal to use toBeChecked for some cases but not others with only a couple of lines of difference.

Or maybe there's nothing to do here. The "mixed" case is so rare, that maybe what we need to do is to disable the rule for that line. But I still wanted to discuss.

@benmonro
Copy link
Member

benmonro commented May 6, 2020

hmm interesting, wasn't aware of that. I'd say there are a few options

  1. maybe open a PR on jest-dom to add support for toBeChecked('mixed') then this plugin can use that.
  2. just look for that special case in this plugin
  3. yeah just disable it for that line.

I'd prefer to do 1 or 2, but don't have the cycles right now if you'd like to open a PR.

@benmonro benmonro added enhancement New feature or request help wanted Extra attention is needed labels May 6, 2020
@gnapse
Copy link
Member Author

gnapse commented May 6, 2020

Yup, I thought something along the lines of no. 1, could work. Will propose it in jest-dom.

@gnapse
Copy link
Member Author

gnapse commented May 19, 2020

We now have .toBePartiallyChecked as of v5.8.0 just released. See testing-library/jest-dom#249.

So this issue can be now about making the eslint plugin aware of this for as a replacement for .toHaveAttribute("aria-checked", "mixed").

@benmonro
Copy link
Member

Care to open a PR @gnapse ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

2 participants