department-of-veterans-affairs / va.gov-cms

Editor-centered management for Veteran-centered content.
https://prod.cms.va.gov
GNU General Public License v2.0
96 stars 70 forks source link

[Web components] [Facility locator] Convert links to va-link where possible or update link text #18537

Closed randimays closed 1 week ago

randimays commented 1 month ago

Description

The facility locator app (in vets-website) has many links that can be converted to web components (va-link). For those links that cannot be converted (due to requiring new tab behavior), "(opens in a new tab)" should be added to the link text.

Acceptance Criteria:

Converted to va-link:

Added "(opens in a new tab)" to the link text, and keep the <a> tag:

Quality / testing notes

randimays commented 1 month ago

@FranECross @Agile6MSkinner I noticed we had a bunch of links in facility locator that could be converted to va-link (or otherwise need an update). I'm not super familiar with the facility locator yet, so I don't have example URLs / "how to get there" instructions, so this ticket will need some refinement (in addition to user story / ACs). I estimated it at a 2 though because the changes are simple.

eselkin commented 2 weeks ago

In this result list

The result list titles (titles of the facilities) appear as h3 even though these classes are added vads-u-font-size--h5... Right now, if you take that class off, it doesn't do anything (maybe because of the importance of some other sizes somewhere)

If you change the anchor tag (<a>) to a va-link, the font does shrink to the h5 size. Should we be showing them as h5-sized linked h3s?

@laflannery @thejordanwood

laflannery commented 2 weeks ago

@thejordanwood As I'm sure you know, I would very much prefer not doing this - I would rather just have H3 tags with default styles, but let me know if you disagree

thejordanwood commented 2 weeks ago

@eselkin @laflannery I agree with Laura, let's use H3 tags with the default styles.

eselkin commented 2 weeks ago

Slightly related, and just visual @thejordanwood The line height of a wrapped <h3><a>text</a></h3> is 1.5 (keeps the a tag line height) but the wrapped <h3><va-link text="text"></va-link><h3> is 1.3 (keeps the h3 line height)

Should I make the lineheight 4 (1.5) in-line with the a tag (there's no 1.3 line height only 1.35)?

Below: top link is 1.3, bottom is 1.5

Screenshot 2024-08-20 at 3 49 59 PM
laflannery commented 2 weeks ago

@eselkin to clarify, you are asking if you should add a custom line-height declaration on the new <va-link> component to match the current line height being used on the Facility names in the search results?

If that is correct, we should not - we want to avoid overwriting the DS styles unless there are drastic differences or very clear reasons why we need different styles. In this case the difference in line-height (1.5 vs. 1.3) is very slight and we should keep the default DS style

thejordanwood commented 2 weeks ago

@eselkin @laflannery I agree, we should use the default DS style. Sounds like that's 1.5 in this case?

eselkin commented 2 weeks ago

@laflannery @thejordanwood thanks, that's what I expected

jilladams commented 1 week ago

Deployed and ready to verify.

laflannery commented 1 week ago

I have verified these on prod. However I am just noting in case someone else sees this, that there is a currently regression from DST regarding when <va-link> wrapped in a <hX> tag so the style is not correct. This should hopefully be resolved today or tomorrow: image