-
-
Notifications
You must be signed in to change notification settings - Fork 32.3k
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
[Checkbox] Add test showcase for checked checkbox #20571
Conversation
Would it make sense to test the checked state transition of human interactions? |
Sure, we don't have a Q&A department though. Makes testing human interaction pretty hard 😛 Goofing around aside: I'll add one ;) |
Hi, answered on my initial PR but to sum it up relying on React props wasn't really the spirit of my PR. It will allow you to write a test here, but then you may have trouble during e2e test, and it doesn't feel right to me. |
Details of bundle changes.Comparing: 3817432...6f040d8 Details of page changes
|
Better, I have some sample code: it("can unselect asset", () => {
cy.get(checkboxSelector)
.first()
.as("firstCheckbox")
.click({ force: true })
.should("have.prop", "checked", true);
//.should("have.attr", "aria-checked", "true");
cy.get("@firstCheckbox")
.click({ force: true })
.should("have.prop", "checked", false);
//.should("have.attr", "aria-checked", "false");
}); It won't pass with prop "checked", it will with "aria-checked" attribute (but need to explictely handle it atm, hence my proposal to integrate its support in mui) |
I understand but querying the checked attribute will not get you anywhere with JS+DOM regardless of framework/library. This is intended behavior by spec. We don't need to polyfill the behavior you want because we know from react core members that this lead to a lot of issues. In addition, you can achieve the same thing by asserting the
This is probably your issue. You should query by role (and possible accessible name). cypress-testing-library has the necessary tools for that. |
The issues with I already get the checkbox using role and accessible name, that will give you the I should probably investigate However after a quick test through Firefox element inspector I have consistency issues, it seems to be activated only after a while. Will try to give more insights. |
Btw checking pseudoselector is not very easy, though doable https://stackoverflow.com/questions/55516990/cypress-testing-pseudo-css-class-before |
That is the same as using the
The issue is that only accept the attribute as a solution when the DOM property (not to be mistaken with a React prop) does all the same things. I can't help if you're not accepting help. Does cypress not support asserting on DOM properties? Using
|
If you have a clonable repro using cypress + Material-UI that queries by role and is not able to assert the checked state please open a new issue. This should work. |
Yeah I'll try to open a repro. My bad, the I can't manage to write a test using the pseudo-class |
Closes #20569
Actual test: 9b37deb
Includes refactoring to full testing-lib test using expect.