-
Notifications
You must be signed in to change notification settings - Fork 128
Fixes #5179 - Alt text for homepage img tags #5180
Fixes #5179 - Alt text for homepage img tags #5180
Conversation
Hold as we need to localize alt texts once test passes |
@6a68 With axe extension analyze , no errors shows up for img-alt @kimberlythegeek thoughts why npm run test:accessibility is failing for img-alt on home page |
28b9cc1
to
17b1c53
Compare
@6a68 PR fixes accessibility tests on homepage. Thanks! |
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.
👍 Nice work
imgSettings = | ||
.alt = Settings Icon | ||
imgSignIn = | ||
.alt = Sign In Icon |
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.
"Sign In" is a verb, "sign-in" is the noun, and I believe you want the latter in this context
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 we should have empty alt strings here anyway since we have it on a the parent anchor
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.
Created #5181 to use empty alt text
No description provided.