-
Notifications
You must be signed in to change notification settings - Fork 7
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
Update AriaLabelIsWellFormatted
logic
#66
Conversation
<button aria-label="11 open" ></button> | ||
HTML | ||
def test_does_not_warn_with_string_interpolation | ||
@file = '<button aria-label="Add a cat <%= "or dog " if dog_allowed? %>pet"></button>' |
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.
This aria-label
value wasn't being parsed correctly in possible_attribute_values
.
possible_attribute_values(tag, "aria-label")
was returning or dog
, which is causing it to be flagged. This appears to because basic_conditional_code_check
is trying to parse the conditional, but it's not working very well. This basic_conditional_code_check
was introduced long before we pulled this into this library, and I never examined closely what it was doing. It doesn't seem to be doing what it's supposed to do so I'm removing it.
HTML | ||
@linter.config.exceptions = ["hello"] | ||
@linter.config.exceptions = ["hello", "git checkout <%= some_branch %>"] |
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.
Testing to see if more "complex" exceptions using string interpolation can be made.
👋 Hello and thanks for pinging us! An accessibility first responder will review this soon.
|
When I was working on the update in our app (staff-only: https://github.com/github/github/pull/269682), I noticed there were some scenarios that were being flagged that shouldn't be.
I am updating the logic of
AriaLabelIsWellFormatted
to make sure it's not flagging what it's not supposed to. I am also cleaning up the logic to separate the starts with lowercase check + includes newline check. In addition, I added a test to make sure that excepting interpolated string works.