-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Added alt tags to img elements #1026
Conversation
Reviewer's Guide by SourceryThis pull request adds Updated class diagram for SeZoomclassDiagram
class SeZoom {
+constructor()
+connectedCallback()
+disconnectedCallback()
+updateZoom()
}
note for SeZoom "Added alt attribute to img element"
Updated class diagram for SeMenuclassDiagram
class SeMenu {
+constructor()
+connectedCallback()
+disconnectedCallback()
+attributeChangedCallback(name, oldValue, newValue)
}
note for SeMenu "Added alt attribute to img element"
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
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 @mscherotter - I've reviewed your changes - here's some feedback:
Overall Comments:
- Consider using a more descriptive
alt
text than just 'logo'. - It might be helpful to extract the image creation logic into a separate function for better readability.
Here's what I looked at during the review
- 🟢 General issues: all looks good
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
@jfhenon Thanks for approving the PR. What is your schedule on updating the NPM module from master? |
I can do it this week. |
PR description
Added alt text to images in UI to make it more accessible.
Checklist
Note that we require UI tests to ensure that the added feature will not be
nixed by some future fix and that there is at least some test-as-documentation
to indicate how the fix or enhancement is expected to behave.
npm test
, ensuring linting passes and that Cypress UI tests keepcoverage to at least the same percent (reflected in the coverage badge
that should be updated after the tests run)
help both for future users and for the PR reviewer.
Summary by Sourcery
Enhancements:
img
elements for better accessibility.