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

Fixes misreported HTML errors when input labels are changed by CSS #712

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ShahneRodgers
Copy link

Hi, first of all, thanks so much for Wallaby - it's so helpful and nice to use!

I've tried to address issue #236 by using the first approach you suggested

One is to not use the same validate_html logic for checkboxes and radio buttons. This is similar to what we do for buttons

since the other two approaches seemed a bit too involved for my first contribution. If your thinking has evolved since creating the issue and you'd rather take another approach, feel free to let me know; I'm happy to just close this PR.

Thanks!

This fixes elixir-wallaby#236 by using the validate_html logic, that was previously
used exclusively by buttons, for both checkboxes and radio buttons.
This changes the error message Wallaby reports from complaining about
a missing label to one that simply states the label does not exist.

This also extends the HTML validation for buttons so that helpful hints
can still be provided, and adds a new error message for when the element
isn't found but the CSS query finds the label.
@mhanberg
Copy link
Member

I'll be able to review this soon, thanks for the PR!

@mhanberg
Copy link
Member

Will take a closer look at this soon, thank you for the patience!

@ShahneRodgers
Copy link
Author

Will take a closer look at this soon, thank you for the patience!

Thanks @mhanberg. Don't worry about it if you're busy though - there's no rush 🙂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants