-
Notifications
You must be signed in to change notification settings - Fork 791
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
feat(label,select-name): allow placeholder to pass label rule, add select-name rule #2448
Conversation
"impact": "serious", | ||
"messages": { | ||
"pass": "Element has a placeholder attribute", | ||
"fail": "Element has no placeholder attribute or the placeholder attribute is empty" |
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.
Lets distinguish between these two. I realise this is also true for non-empty-title, I'll open a tech debt ticket for that.
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.
Doing this would make the generic check attr-non-space-content
change, which in turn affects the 3 other checks that rely on it: none-empty-alt
, none-empty-value
, non-empty-title
(the tests and messages would need to be updated in each of those to handle this.data
being called in the check).
As such, this change would be outside the scope of the PR and I vote we do this in a separate PR (the ticket you created).
Label rule also applied to
select
elements which can't have placeholder attribute, so had to split the rule up.The placeholder name calculation is part of step 2D, native text methods, so that's where I put it.
Also, we had tests in the accessible-name calculation code for placeholder that were skipped, so I could just unskip them.
Closes issue: #2413
Reviewer checks
Required fields, to be filled out by PR reviewer(s)