-
Notifications
You must be signed in to change notification settings - Fork 128
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
[#179] Update Task Blue (10182 form) to match new designs #32775
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.
Icon found
Icons can be decorative, but sometimes they are used to convey meaning. If there are any semantics associated with an icon, those semantics should also be conveyed to a screen reader.
What you can do
Review the markup and see if the icon provides information that isn't represented textually, or wait for a VSP review.
Note:
Font Awesome is deprecated. Please use va-icon instead. For more information, visit the migration documentation: Migrate from font awesome to va-icon
// > | ||
// <span> | ||
// <strong>{editText}</strong> | ||
// <va-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.
icon found
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.
is there a reason for commenting these out? can we just delete?
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.
hopefully once the unit tests issue is resolved we can update with main and get this merged.
// > | ||
// <span> | ||
// <strong>{editText}</strong> | ||
// <va-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.
is there a reason for commenting these out? can we just delete?
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.
Are you removing, renaming or moving a folder in this PR?
Did you change site-wide styles, platform utilities or other infrastructure?
Summary
Related issue(s)
#179
Testing done
Tests pass locally.
Screenshots
What areas of the site does it impact?
N/A
Acceptance criteria
Quality Assurance & Testing
Error Handling
Authentication
Requested Feedback
(OPTIONAL) What should the reviewers know in addition to the above. Is there anything specific you wish the reviewer to assist with. Do you have any concerns with this PR, why?