Conditionally display "Location" header on Service Location templates

[davidmpickett]

User Story or Problem Statement

Currently it is possible for a CMS editor to populate the field_building_name_number or field_wing_floor_or_room_number fields of Service Location Address without populating the field_clinic_name. This can lead to situations where these fields don't have an appropriate header.

There is a Drupal ticket which would resolve this issue on the editor side by conditionally requiring this fields, however, having even once that is implemented, it won't immediately fix current instances of that field being missing.

In the design ticket, Jordan has proposed adding the label “Location” when the Name field is left blank and moving it to the top of the Service Location section.

Mockup

A mockup can be seen here. This design can also be found on the Location headers page in the VAMC Master Figma file

Description or Additional Context

VAMC Health Service

VAMC Top Task Page

VBA Facility Service

Steps for Implementation

Acceptance Criteria

• Iffield_building_name_number or field_wing_floor_or_room_number are populated but field_clinic_name is not, display a header that says "Location" where field_clinic_name would normally be displayed
• I expect all Liquid Templates that have Service Location in them are updated with this logic
  • VAMC Facility (health services section)
  • VAMC Billing and Insurance, Medical records, register for care (Police if it has been updated to use non clinical services)
  • VBA facility (service accordions)
  • Confirm this works for cases where a service has either a single or multiple service locations
• Requires design review
• Requires accessibility review

[davidmpickett]

@xiongjaneg @mmiddaugh tagging you on this ticket as it is one that of the smaller actionable items to come out of Jordan's design work and might be a good candidate for prioritization

[aklausmeier]

@thejordanwood Please review and update ticket with finalized mockup, I believe you wanted to move "location" up above phone so it falls beneath facility address.

[thejordanwood]

@aklausmeier I've updated the ticket with that info and also included a mockup of the new design.

[davidmpickett]

@mmiddaugh I think there's a good case for this being launch blocking for VBA/VAMC service location. Compare before and after:

[jilladams]

Would help to provide an example service with multiple service locations for testing.

[davidmpickett]

https://www.va.gov/pittsburgh-health-care/locations/pittsburgh-va-medical-center-university-drive/#gastroenterology
is my go-to example of 2 service locations

Example of a Service with 2 service locations where 1 of the service locations meets the criteria:

field_building_name_number or field_wing_floor_or_room_number are populated but field_clinic_name is not

https://www.va.gov/syracuse-health-care/locations/syracuse-va-medical-center/#nutrition-food-and-dietary-care

[maxx1128]

@davidmpickett Are you able to give links to any other active page examples of this bug? I have a review instance up at http://9b38542627c29dcfdb00072ae2240fdc.review.vetsgov-internal/ and will be able to check the effectiveness of the fix quickly against that.

[davidmpickett]

• https://www.va.gov/greater-los-angeles-health-care/billing-and-insurance/
• https://www.va.gov/greater-los-angeles-health-care/locations/west-los-angeles-va-medical-center/#amputation-care
• For VBA you might need to create content that meets the criteria. All the examples I had previously created on the Tugboat were wiped

[maxx1128]

I was able to confirm the first two linked examples locally and updated the pull request. Once I can update my local environment to recreate multiple service locations, then this should be all set to review.

[davidmpickett]

Rerunning content release on Tugboat so this can be verified

[davidmpickett]

Verified on Prod, but still gonna verify on Tugboat that it plays well with VBA and Service Location enhancements

[davidmpickett]

Hey @maxx1128 noticing some funky spacing on Tugboat when comparing the instances where the Location header is used compared to instances with field_clinic_name

Location

field_clinic_name

https://web-xptmfsnjoodldn85nzgkrgljqtfbz24b.ci.cms.va.gov/pittsburgh-health-care/locations/pittsburgh-va-medical-center-university-drive/#mental-health-care

https://web-xptmfsnjoodldn85nzgkrgljqtfbz24b.ci.cms.va.gov/pittsburgh-health-care/locations/pittsburgh-va-medical-center-university-drive/#hivhepatitis-care

Looks like the Location header is missing class="vads-u-margin-top--0"

[maxx1128]

@davidmpickett Those tugboat links aren't working, but I can still update those headers to add the class.

[maxx1128]

@davidmpickett I created a draft pull request with the class added, you can see it here: department-of-veterans-affairs/content-build#2020

[jilladams]

#sitewide-facilities thread with some concerns from Michelle / Amanda about changes here: https://dsva.slack.com/archives/C0FQSS30V/p1713299374411909

@maxx1128 please take a look at that thread for your reference, and Jordan / Dave are tagged there to take a UX-look as well.

(edited by @davidmpickett to link to the Slack source of truth)

[thejordanwood]

@maxx1128 Just for your awareness, I think this looks good except for the spacing issue that Dave pointed out. I'm waiting to approve until I can review the draft pull request you've created to fix this issue. cc: @davidmpickett

[davidmpickett]

@jilladams While I'm out. Steps to get this closed:

• Max's new PR (contentbuild-2020) needs to target the content build integration branch rather than main so that it can be reviewed with all other changes in context
• Code review on Max's new PR @eselkin
• Design review on Max's PR env
• [Optional] Do a content release on the Integration Branch Tugboat using contentbuild-2020 as the content build version so can verify the fix works as intended on Tugboat [QA step]
• Max's new PR merged into Integration Branch
• Do a content release on the Integration Branch Tugboat using Integration Branch PR (contentbuild-1912) as the content build version so can verify the fix works as intended on Tugboat [Verification on Prod]

The alternate path of merging contentbuild-2020 to Main

• Precludes the step of QAing on Integration Branch Tugboat, might need ngrok into Max's local or something?
• would require rebasing contentbuild-1912 against Main before the final content release for "prod verification"

[maxx1128]

@thejordanwood The pull request with the change fixing the spacing issue has a working test environment you can look at here.

[thejordanwood]

@maxx1128 The spacing is a bit too close together now. Is there any way that it can be the same as the spacing between the Contact information and Behavioral health services headers in this example?

Edit: Design System guidance in Figma says spacing above H5 headers should be 22.5 px. We should get as close to this as the VADS ecosystem spacing allows.

This is what I'm seeing:

ideally, the spacing should look like this:

[laflannery]

@maxx1128 I spoke to @aklausmeier and we want to make sure that there aren't custom margins being used - that we use the standard margins provided by the DS. That being said can you:

• Remove the 0 margin on the "Location" header Jordan noted above?

• Remove the custom 1em margin that is on the "Contact information" header (which I know you didn't add if that could be removed anyway?)
  

[maxx1128]

The custom 1em margin is actually being added in the vets-website repo, not content-build, which is a little odd, but makes these updates a bit simpler. I have closed the existing pull request, so the Location header stays as it is in production without the 0 margin helper. I've also opened another pull request that removes the custom margin.

[laflannery]

Just want to close the loop on the spacing issues that Jordan and Dave had brought up in a few comments in this ticket. This is a mural of everywhere this Location header work currently has an impact right now, it shows a few issues:

• The largest issue will be handled with a separate ticket (noted in a comment in the mural)
• The other 2 issues are what was pointed out originally by Dave but then after Max resolved that one instance a new issue was pointed out by Jordan, because now in another use case the headers were too close.

We ideally should always be using DS margins/spacing and not adding custom margins. In this case, we need to look at the margins being added to the wrapping <div>. But again, we need to look at this for every possible scenario because I already know that if I remove the margin in one case, I have created an issue in another case. The overall spacing here should be reviewed/cleaned up separately (perhaps as part of the typography review)

[aklausmeier]

Thank you @laflannery! Agree with reviewing spacing as part of the typography updates.

[davidmpickett]

@laflannery Thanks for that thorough audit! I made a note about reviewing spacing on the Typography Audit.

@maxx1128, seems like once you merge your new PR and this is verified on Prod, we'll be good to close this ticket

[davidmpickett]

Confirmed on Tugboat and Prod