-
Notifications
You must be signed in to change notification settings - Fork 12
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
VACMS 18326 content-build fontawesome removal #2133
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
@@ -1,19 +1,19 @@ | |||
{% if fieldOperatingStatusFacility == 'notice' %} | |||
<span class="operating-status-flag operating-status-flag-notice operating-status usa-alert usa-alert-info background-color-only vads-u-margin-top--1 vads-u-padding-y--1 vads-u-padding-x--1p5"> | |||
<i class="fa fa-info-circle" aria-hidden="true"></i> | |||
<va-icon icon="info" size="3" class="os-icon"></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
Facility notice | ||
<i class="fa fa-chevron-right vads-u-padding-left--0p5" aria-hidden="true"></i> | ||
<va-icon icon="chevron_right" size="3" class="os-chevron-right vads-u-padding-left--0p5"></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
</span> | ||
{% elsif fieldOperatingStatusFacility == 'limited' %} | ||
<span class="operating-status-flag operating-status-flag-warning operating-status usa-alert usa-alert-warning background-color-only vads-u-margin-top--1 vads-u-padding-y--1 vads-u-padding-x--1p5"> | ||
<i class="fa fa-exclamation-triangle" aria-hidden="true"></i> | ||
<va-icon icon="warning" size="3" class="os-icon"></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
Limited services and hours | ||
<i class="fa fa-chevron-right vads-u-padding-left--0p5" aria-hidden="true"></i> | ||
<va-icon icon="chevron_right" size="3" class="os-chevron-right vads-u-padding-left--0p5"></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
</span> | ||
{% elsif fieldOperatingStatusFacility == 'closed' %} | ||
<span class="operating-status-flag operating-status-flag-error operating-status usa-alert usa-alert-error background-color-only vads-u-margin-top--1 vads-u-padding-y--1 vads-u-padding-x--1p5"> | ||
<i class="fa fa-exclamation-circle" aria-hidden="true"></i> | ||
<va-icon icon="error" size="3" class="os-icon"></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
Facility Closed | ||
<i class="fa fa-chevron-right vads-u-padding-left--0p5" aria-hidden="true"></i> | ||
<va-icon icon="chevron_right" size="3" class="os-chevron-right vads-u-padding-left--0p5"></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
<span class="fas fa-user circular-profile-missing-icon"></span> | ||
</span> | ||
<div class="circular-profile-image bio-paragraph-image vads-u-position--relative vads-u-background-color--gray-lightest vads-u-display--block"> | ||
<div class="circular-profile-missing-icon"><va-icon size="6" icon="person"></va-icon></div> |
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.
LGTM!
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.
Added a question.
When filling out the PR template, could you please make sure any boilerplate is removed (including the message above the Summary heading)?
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.
LGTM
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.
Approving on behalf of Facilities FE since it has FE code review and Daniel's out.
Summary
Related issue(s)
Testing done
Screenshots
Staff Profile without images with PR
Staff Profiles past - fontawesome
(outdated should be removed)Operation Status page - old
(outdated should be removed)Operation Status page - with PR (follows pattern from operating status page)
(Describe what parts of the site are impacted if code touched other areas)
Acceptance criteria
Quality Assurance & Testing
Error Handling
Authentication
#sitewide-public-websites
Slack channel for questionsRequested Feedback
Check that in conjunction with vets-website PR of same name that Staff profiles without photos show new va-icon and that icon is centered in gray circle. Check that Operating status page shows normal text for normal notices and slim alerts for all others.