-
Notifications
You must be signed in to change notification settings - Fork 800
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
Ignore readonly autocomplete field #4721
base: develop
Are you sure you want to change the base?
Ignore readonly autocomplete field #4721
Conversation
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.
Hey @eddie0329 Thank you so much for the contribution. While This addresses the issue, I don't think this is quite the right way to do this. The way you've done this will result in the rule passing. What I think we'd want here is for the rule to be inapplicable on readonly input elements. The way to do that is to move the check to the matches function in lib/rules/autocomplete-matches.js
.
You'd then need to put a test for it in test/rule-matches/autocomplete-matches.js
. We'll also want at least one E2E test to validate the result, which you can add to test/integration/rules/autocomplete-valid/autocomplete-valid.html
.
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.
LGTM, just need a couple integration tests in test/integration/rules/autocomplete-valid/autocomplete-valid.html.
@WilcoFiers Thanks for review. Command ,
My current chrome version is 134. Instead of down garde my original chrome version, is there any way using like virtual environment? |
@eddie0329 if you run the command |
<< Describe the changes >>
Closes: #4708
adding feature that autocomplete-valid rule should not fail on readonly fields.