-
Notifications
You must be signed in to change notification settings - Fork 4.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
Components: Fix no-container
violations in FormGroup
tests
#45662
Conversation
Open in CodeSandbox Web Editor | VS Code | VS Code Insiders |
Size Change: 0 B Total Size: 1.29 MB ℹ️ View Unchanged
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Neat 🚀
<FormGroup labelHidden id="fname" label="First name"> | ||
<TextInput /> | ||
</FormGroup> | ||
); | ||
|
||
const label = container.querySelector( 'label' ); | ||
expect( label ).toContainHTML( 'First name' ); | ||
// @todo: Refactor this after adding next VisuallyHidden. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure what this TODO was about, do you think we're safe to remove it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think so. I looked over the history and didn't see an obvious reason for it to be there in the first place.
What?
With the recent work to improve the quality of tests, we fixed a bunch of ESLint rule violations. This PR fixes a few
no-container
rule violations in theFormGroup
component.Why?
The end goal is to enable that ESLint rule once all violations have been fixed.
How?
We're refactoring most of the straightforward
querySelector
/querySelectorAll
instances to use screen queries instead.Testing Instructions
Verify all tests still pass.