Vacms 15315 front end match style to sketch, testing #1790
Conversation
eselkin
changed the title
…5-front-end-match-style-testing
va-vfs-bot
temporarily deployed
to
master/main/VACMS-15315-front-end-match-style-testing
Co-authored-by: Peter Hill <[email protected]>
| href="tel:{{ fieldPhoneNumber }}">{{ fieldPhoneNumber }}</a> | ||
| </div> | ||
| {% endif %} | ||
| {% if fieldRegionPage.entity.fieldVaHealthConnectPhone %} | ||
| <div class="vads-u-margin-bottom--1"> |
There was a problem hiding this comment.
Issue: this should be a <p> tag.
There was a problem hiding this comment.
None of the other templates have the telephone sections in p tags, just divs. Are we sure we want <p> tags?
There was a problem hiding this comment.
Looking at it on a live page, any <strong> and <a> tags are meant to be inside of a <p> tag. Otherwise, we have text inside a <div> tag, which has no semantic meaning. It will need to add the vads-u-margin-top--0 class to remove the extra top margin, but otherwise, it should look the same to visual users.
As for the other templates not using it, that may need to be a follow-up ticket to address it in other areas better. But we should probably (a)sync with @laflannery on it first.
There was a problem hiding this comment.
@maxx1128 and @eselkin Couple questions:
- Are we using the
<va-telephone>component on the new VBA page? - I know we aren't currently using the component in other places, it's something we need to work on updating so I think this would be good to roll into one ticket. To clarify the new ticket should be to update phone number markup within other facility templates to look like this correct?
There was a problem hiding this comment.
- Yes, this page uses the
<va-telephone>component. - It's not a requirement to have anchor tags in paragraph tags, but it is preferred since it makes more semantic sense for an anchor tag to be in a larger text element, like a paragraph tag. The paragraph tag would need the
vads-u-margin-top--0class, which should be the same visually. I can also do a little extra research and testing on this.
There was a problem hiding this comment.
@maxx1128 Make sure to take a look at https://web-w1a5evfuonymjntciz3pduj1ofhepwky.demo.cms.va.gov/vba-facilities/locations/seattle-regional-office/
I think the screenshot was of a different tugboat environment or something because it didn't have the va-telephone.
There was a problem hiding this comment.
@eselkin I see the <p> but the class didn't get added correctly, it's missing the =:
There was a problem hiding this comment.
@eselkin I tried adding the <p> tag locally and saw that our stylesheet added some top margin to them. Removing the top margin just keeps it more consistent with the styles we currently have for the phone numbers.
There was a problem hiding this comment.
@eselkin I see the
<p>but the class didn't get added correctly, it's missing the=:
That is so weird... I'm rebuilding, locally and in github they have the equal and the right quotation marks.
There was a problem hiding this comment.
The rebuild fixed the odd quotes and equal sign it seemed. Not sure how that happened.
va-vfs-bot
temporarily deployed
to
master/main/VACMS-15315-front-end-match-style-testing
va-vfs-bot
temporarily deployed
to
master/main/VACMS-15315-front-end-match-style-testing
Co-authored-by: Peter Hill <[email protected]>
Bumps [actions/checkout](https://github.com/actions/checkout) from 3.5.2 to 4.1.1. - [Release notes](https://github.com/actions/checkout/releases) - [Changelog](https://github.com/actions/checkout/blob/main/CHANGELOG.md) - [Commits](actions/checkout@8e5e7e5...b4ffde6) --- updated-dependencies: - dependency-name: actions/checkout dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Peter Hill <[email protected]>
va-vfs-bot
temporarily deployed
to
master/main/VACMS-15315-front-end-match-style-testing
va-vfs-bot
temporarily deployed
to
master/main/VACMS-15315-front-end-match-style-testing
va-vfs-bot
temporarily deployed
to
master/main/VACMS-15315-front-end-match-style-testing
|
@eselkin and @maxx1128 I reviewed the page linked above (which may or may not have been ready?) but there are some questions/comments below:
|
maxx1128
force-pushed
the
va-14940-vba-facility-content-skeleton
branch
from
c00627d to
77649de
Compare
va-vfs-bot
temporarily deployed
to
master/main/VACMS-15315-front-end-match-style-testing
There was a problem hiding this comment.
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.
| <div class="social-links-email vads-u-margin-bottom--2"> | ||
| <a class="vads-u-display--flex vads-u-align-items--baseline vads-u-text-decoration--none" rel="noreferrer" | ||
| href="{{fieldNews.uri}}"> | ||
| <i class="fas fa-envelope vads-u-margin-right--1"></i> |
There was a problem hiding this comment.
@laflannery since we're using raw anchor tags here, is there some description about how these should be conveyed textually?
| {% if fieldFacebook != empty %} | ||
| <div class="social-links social-links-facebook vads-u-margin-bottom--2"> | ||
| <a class="vads-u-display--flex vads-u-align-items--baseline vads-u-text-decoration--none" rel="noreferrer" href="{{ fieldFacebook.uri }}"> | ||
| <i class="fab fa-facebook-square vads-u-margin-right--1"></i> |
| {% if fieldTwitter != empty %} | ||
| <div class="social-links social-links-twitter vads-u-margin-bottom--2"> | ||
| <a class="vads-u-display--flex vads-u-align-items--baseline vads-u-text-decoration--none" rel="noreferrer" href="{{ fieldTwitter.uri }}"> | ||
| <i class="fab fa-twitter vads-u-text-decoration--none vads-u-margin-right--1"></i> |
| {% if fieldFlickr != empty %} | ||
| <div class="social-links social-links-flickr vads-u-margin-bottom--2"> | ||
| <a class="vads-u-display--flex vads-u-align-items--baseline vads-u-text-decoration--none" rel="noreferrer" href="{{ fieldFlickr.uri }}"> | ||
| <i class="fab fa-flickr vads-u-text-decoration--none vads-u-margin-right--1"></i> |
| {% if fieldInstagram != empty %} | ||
| <div class="social-links social-links-instagram vads-u-margin-bottom--2"> | ||
| <a class="vads-u-display--flex vads-u-align-items--baseline vads-u-text-decoration--none" rel="noreferrer" href="{{ fieldInstagram.uri }}"> | ||
| <i class="fab fa-instagram vads-u-text-decoration--none vads-u-margin-right--1"></i> |
| {% if fieldYoutube != empty %} | ||
| <div class="social-links social-links-youtube vads-u-margin-bottom--2"> | ||
| <a class="vads-u-display--flex vads-u-align-items--baseline vads-u-text-decoration--none" rel="noreferrer" href="{{ fieldYoutube.uri }}"> | ||
| <i class="fab fa-youtube vads-u-text-decoration--none vads-u-margin-right--1"></i> |
| <div class="vads-u-padding-top--1"> | ||
| <p> | ||
| <a class="vads-u-display--block" href="{{ entity.fieldCta.entity.fieldButtonLink.url }}" > | ||
| <span> {{ entity.fieldCta.entity.fieldButtonLabel }} <i class="fa fa-chevron-right vads-facility-hub-cta-arrow" aria-hidden="true"></i></span> |
va-vfs-bot
temporarily deployed
to
master/main/VACMS-15315-front-end-match-style-testing
Should be there now! Sorry about missing that.
Keeping as H2 |
va-vfs-bot
temporarily deployed
to
master/main/VACMS-15315-front-end-match-style-testing
va-vfs-bot
temporarily deployed
to
master/main/VACMS-15315-front-end-match-style-testing
That is indeed the correct URL. VBA uses an entirely different scheduling tool than VAMCs. I defer to @laflannery on whether this would be an exception to the normal external link guidance. We might also need to have CAIA re-review this specific instance and reword it. @xiongjaneg |
|
@eselkin:
|
|
RE: https://va.my.site.com/VAVERA/s/ being an external link Michelle verified that is the only appointment link used for VBA. I can't speak to internal/external link guidance, but there is no other link to provide for VBA appointments. It is the VA appointment tool for VBA. |
I updated in the code, it didn't make it to the tugboat |
eselkin
merged commit 9d61d0b
into
va-14940-vba-facility-content-skeleton
…for VBA (#1762) * VA-14940: Update VBA Facility Query * VA-14940: Add VBA Facility to larger build processes * VA-14940: Update VBA Facility skeleton template * remove NearbyVetCenters widget after discussion and update link to find-locations * VA-14940: Update VBA Facility Query * VA-14940: Add VBA Facility to larger build processes * VA-14940: Update VBA Facility skeleton template * remove NearbyVetCenters widget after discussion and update link to find-locations * VA-14940: Remove unused VBA Facility query field * Vacms 15315 front end match style to sketch, testing (#1790) * working va-telephone components, cc benefits hotline, cc cannot find benefits * back to top and above footer * describe structure of processed better * Add breadcrumb data and update component * p tags with margin top 0 * uswds on alert, office hours, etc * social updates * vba links at top * testing for updates filters * fix mock data * claim status link ---- Co-authored-by: Max Antonucci <[email protected]> Co-authored-by: Nate Douglas <[email protected]> --------- Co-authored-by: Eli Selkin <[email protected]> Co-authored-by: Eli Selkin <[email protected]>
Good catches, @eselkin. I just removed the extraneous |
Summary
Related issue(s)
Testing done
Screenshots
see: https://web-w1a5evfuonymjntciz3pduj1ofhepwky.demo.cms.va.gov/vba-facilities/locations/seattle-regional-office/
What areas of the site does it impact?
Solely VBA, conditionals were added to existing liquid templates used in other areas to only display certain aspects when VBA variables are set. Many of the conditionals already existed and were just expanded upon.
Acceptance criteria
Quality Assurance & Testing
Error Handling
Authentication
Requested Feedback