Sitewide integration: R&S Checklist migration #837

randimays · 2024-12-12T18:08:43Z

Description

Combines the contents of Sitewide-integration with main. This integration branch was created to avoid merging any code into main before Events were successfully deployed to production. Now that the Events deploy has been successfully validated in production, we can move in this code and stop using an integration branch.

The code that is in this branch was added as part of the Checklist template code and includes updates to existing components to bring them up to speed with production and updates of the design system dependencies.

Details for each update are in these PRs:

Ticket

Relates to #19386

Developer Task

Tasks

Give feedback

PR submitted against the main branch of next-build.
Link to the issue that this PR addresses (if applicable).
Define all changes in your PR and note any changes that could potentially be breaking changes.
PR includes steps to test your changes and links to these changes in the Tugboat preview (if applicable).
Provided before and after screenshots of your changes (if applicable).
Alerted the #accelerated-publishing Slack channel to request a PR review.
You understand that once approved, you are responsible for merging your changes into main. (Note that changes to main will move automatically into production.)
Options

Testing Steps

Explain the steps needed for testing

QA steps

What needs to be checked to prove this works?
What needs to be checked to prove it didn't break any related things?
What variations of circumstances (users, actions, values) need to be checked?

Check the Events page(s) to ensure there are no unexpected regressions
App should build successfully
App should not have any type(script) errors
All tests should pass

Screenshots

Note that testing was specifically regression checks for the /outreach-and-events/events page and individual event pages. Since there were updates to the design system dependencies, spacing here and there has been updated and is expected.

Events - Desktop

`/outreach-and-events/events`

Note: the underlines on the pagination numbers are also not showing on the main branch, but they appear fine in production - not sure why this is.

Before	After

`/central-western-massachusetts-health-care/events/73034/`

Before	After

`/outreach-and-events/events/75180/`

Before	After

Events - Tablet

`/outreach-and-events/events`

Before	After

Events - Mobile

`/outreach-and-events/events`

Before	After

`/outreach-and-events/events/75180/`

Before	After

`/central-western-massachusetts-health-care/events/73034/`

Before	After

Is this PR blocked by another PR?

Add the DO NOT MERGE label
Add links to additional PRs here:

Reviewer

Reviewing a PR

This section lists items that need to be checked or updated when making changes to this repository.

Standard Checks

Tasks

Give feedback

Code Quality: Readabilty, Naming Conventions, Consistency, Reusability
Test Coverage: 80% coverage
Functionality: Change functions as expected with no additional bugs
Performance: Code does not introduce performance issues
Documentation: Changes are documented in their respective README.md files
Security: Packages have been approved in the TRM
Options

Merging an Approved Layout

When merging a layout, you must ensure that the content type has been turned on for next-build inside the CMS. This CMS flag must be turned on for editors to preview their work using the next build preview server.

Resource types (layouts) that have not been approved by design should NOT be pushed to production. Ensure that slug.tsx does not include your resource type if it is not approved.

The status of layouts should be kept up to date inside templates.md. This includes QA progress, development progress, etc. A link should be provided for where testing can occur.

Merging a Non-Approved Layout

Your new resource type should not be included inside slug.tsx. Items added here will go into production once merged into the main branch. It is imperative that we do not push anything live that has not been approved.

Ensure that this layout has been added to the templates.md file with the current status of the work.

cvalarida

I think this all looks good to me. The only thing I couldn't account for was the src/templates/common/meta/index.tsx. I'm maybe just blind, but I couldn't find it in any of those PRs or as a separate commit in the history of this branch. 😕

Other than that, I think we're golden. Before I approve, I'd like to test it locally (or in a preview server, honestly, if those are up and running 🤞). What's the best way to do that?

cvalarida · 2025-02-13T22:54:17Z

README.md

@@ -34,7 +34,9 @@ You should set these up before attempting to install the repo.

 7. In the `next-build` directory, run `yarn setup` to pull initial built assets from the `vets-website` repo. This will grab a bunch of files from a vets-website S3 bucket and place them into the appropriate `public/` folders.

-8. Run `yarn dev`.
+8. In your `envs/.env.local` file, uncomment `FEATURE_NEXT_BUILD_CONTENT_ALL=true`.


This will be uncommented by default in #884

I'll remove this change.

cvalarida · 2025-02-13T22:56:13Z

additional.d.ts

+    'va-link-action'
+    'va-on-this-page'
+    'va-telephone'


For my own clarity: The va-link-action and va-telephone are net new here.

cvalarida · 2025-02-13T22:58:04Z

generator-templates/query/test.hbs

+/**
+* @jest-environment node
+*/


Suggested change

/**

* @jest-environment node

*/

/**

* @jest-environment node

*/

cvalarida · 2025-02-13T23:04:27Z

src/data/queries/index.ts

@@ -1,51 +1,51 @@
 import { createQueries } from 'next-drupal-query'
-import * as PressRelease from './pressRelease'


For my own clarity: I just copied the before and after into files, sorted the before, and did a diff against the after. They're all the same imports, just alphabetized.

cvalarida · 2025-02-13T23:18:24Z

src/pages/[[...slug]].tsx

@@ -13,15 +13,7 @@ import dynamic from 'next/dynamic'
 import Script from 'next/script'
 import { drupalClient } from '@/lib/drupal/drupalClient'
 import { getGlobalElements } from '@/lib/drupal/getGlobalElements'
-import { Wrapper } from '@/templates/layouts/wrapper'


For my own clarity: All these are the same, just reorganized.

cvalarida · 2025-02-13T23:21:38Z

src/templates/common/link/index.tsx

@randimays Why was this deleted? I assume it's not used, right? I looked in the original PR, but didn't see a comment for this one.

How can I double-check that it doesn't break anything?

cvalarida · 2025-02-13T23:32:54Z

src/templates/common/meta/index.tsx

Alright, I'm struggling with this one. 😅 I can't seem to find it in any of those PRs you listed up top. What's the story behind this change?

cvalarida · 2025-02-13T23:46:08Z

src/templates/common/relatedLinks/index.test.tsx

+    expect(container.innerHTML).toContain(
+      'Eligibility for burial in a VA national cemetery'
+    )
+    expect(container.innerHTML).toContain(
+      'href="https://va.gov/burials-memorials/eligibility"'
+    )
+    expect(container.innerHTML).toContain(
+      'Here is a summary for this URL so you can see how it displays underneath.'
+    )
+    expect(container.innerHTML).toContain(
+      'Schedule a burial for a Veteran or family member'
+    )
+    expect(container.innerHTML).toContain(
+      'href="https://va.gov/burials-memorials/schedule-a-burial"'
+    )


Any reason we're not using semantic queries here?

const links = container.getAllByRole('link'); expect(links).toHaveLength(2); // etc.

Or even

const first = container.querySelector('[href="https://va.gov/burials-memorials/eligibility"]'); // etc.

The <a> tags are in the shadow DOM (of the va-link web component). Unfortunately the test runner can't find them by role.

Ahhhh, that makes sense! Thanks!

I may fiddle around with that at some point. Not sure if this is a problem because we just aren't hydrating the web components in our test setup, the way JSDOM handles web components, or something else. 🤔 For now, this looks fine. If I get the chance, I'll see if I can fix this up in another PR.

There has been quite a bit of talk around trying to get access to the shadow DOM in unit tests (see here minimally; there are lots of results if you search DSVA Slack for "shadow DOM unit test"). If you manage to get it working, definitely wave the victory flag somewhere - lots of folks will be interested :D

…ts (#814)

randimays · 2025-02-14T17:53:53Z

@cvalarida I don't see changes to src/templates/common/meta/index.tsx. I just updated the branch with what's in main, so maybe those diffs dropped off?

cvalarida · 2025-02-14T18:08:51Z

💨 It's gone! Looks like updating with main made it disappear into the ether. 👍

randimays · 2025-02-14T18:10:58Z

@cvalarida The Link component (src/templates/common/link/index.tsx) is not used. Apologies for the confusion, it was not my original intention to create so many PRs from this code and I've lost track of what is documented where. The original checklist PR has a reference to it:

Removes the unused custom component

If you search the source code for Link (case-sensitive), you can see that the only place it's ever imported from is next/link, so that custom component isn't used.

As far as testing goes - this one is a bit tricky. Pretty much all of my changes are to Resources & Support common components because they were in support of the checklist template, and we don't have any other next-build R&S templates prod-ready yet. You can see the components I changed in the Storybook view (relatedLinks, secondaryButtonGroup, contactInfo). Otherwise there's just a lot of alphabetization/organization. You can regression check by running the code locally at /outreach-and-events/events (to be sure nothing is going to break prod).

va-cms-bot · 2025-02-20T17:02:27Z

Tugboat has finished building the preview for this pull request!

Links:

Link (redis):

https://pr837-ujhjgc659wtnoiphwmwhjlr3r49gykjh.tugboat.vfs.va.gov

Dashboard:

https://tugboat.vfs.va.gov/675b26afe2b213c9356eb760

cvalarida

Looks generally good. I noticed no regressions, but the ContactInfo was confusing to me. Mind adding some docs?

cvalarida · 2025-02-21T21:05:38Z

src/templates/components/contactInfo/index.tsx

+    return (
+      <li className="vads-u-margin-top--1">
+        <strong>{label}&nbsp;</strong>
+        <va-telephone contact={number} extension={extension || null} />
+      </li>
+    )


This one doesn't look quite right. Shouldn't it show the phone number? The link is correct (tel:...), but why does the link text say "Phone Number"?

Update: Default contact. That little contactType="DC" magic property renders the default, not this PhoneContact component. Got it.

This behavior isn't what I'd expect, honestly. I assume we don't have much leeway in changing the behavior, do we? Probably requires an upstream change in Drupal and corresponding content-build template. 😮‍💨

Can we get some documentation on this component describing what's going on? Ideally, I'd also like a JSDoc string on the ContactInfo type and its properties so we can leverage the language server hover docs. 🤞

cvalarida · 2025-02-21T22:52:25Z

src/templates/components/contactInfo/index.tsx

-                  className="usa-unstyled-list vads-u-display--flex vads-u-flex-direction--column"
-                  role="list"
-                >
+                <ul className="usa-unstyled-list vads-u-display--flex vads-u-flex-direction--column">


This is outside the scope of this PR since it was this way before, but for posterity, this should probably be a description list, not an unordered list. I think the default formatting for that will be funky, but it's more semantically correct, I believe.

cvalarida · 2025-02-21T23:00:05Z

src/templates/components/contactInfo/index.tsx

For posterity: I can't leave a comment on line 104 of the new version because it's out of scope of this PR, but I'm not sure we should add that 0 x padding on large displays (desktop-lg:vads-u-padding-x--0). 🤷

va-cms-bot temporarily deployed to Tugboat December 12, 2024 18:08 Destroyed

va-cms-bot temporarily deployed to Tugboat December 12, 2024 18:27 Destroyed

randimays marked this pull request as ready for review December 12, 2024 18:30

randimays requested a review from timcosgrove December 12, 2024 18:30

randimays force-pushed the Sitewide-integration branch from 70dbef9 to 6bdf487 Compare December 16, 2024 15:42

va-cms-bot temporarily deployed to Tugboat December 16, 2024 15:42 Destroyed

timcosgrove mentioned this pull request Dec 16, 2024

Alphabetize imports, types, etc in code #846

Open

va-cms-bot temporarily deployed to Tugboat December 16, 2024 20:42 Destroyed

va-cms-bot temporarily deployed to Tugboat December 16, 2024 20:51 Destroyed

jilladams changed the title ~~Sitewide integration~~ Sitewide integration: R&S Checklist migration Jan 24, 2025

cvalarida reviewed Feb 13, 2025

View reviewed changes

randimays and others added 5 commits February 14, 2025 10:13

Update Design System dependencies to latest (#812)

f821733

VACMS-19386 Add SecondaryButtonGroup and RelatedLinks common componen…

b52ea60

…ts (#814)

VACMS-19386 Cleanup in support of checklist template (#815)

c9008ad

VACMS-19386 Update contact info component (#821)

8060ab1

Update DST packages. (#845)

a628053

randimays force-pushed the Sitewide-integration branch from 6801613 to a628053 Compare February 14, 2025 16:14

randimays requested review from a team as code owners February 14, 2025 16:14

va-cms-bot temporarily deployed to Tugboat February 14, 2025 16:14 Destroyed

va-cms-bot temporarily deployed to Tugboat February 14, 2025 16:29 Destroyed

Revert README change

5918b87

randimays force-pushed the Sitewide-integration branch from c57dbb7 to 5918b87 Compare February 14, 2025 17:46

va-cms-bot temporarily deployed to Tugboat February 14, 2025 17:46 Destroyed

Delete scripts/.../public/generated

67a6bb3

va-cms-bot temporarily deployed to Tugboat February 18, 2025 20:19 Destroyed

Merge branch 'main' into Sitewide-integration

6b0bc1b

va-cms-bot temporarily deployed to Tugboat February 19, 2025 18:09 Destroyed

randimays mentioned this pull request Feb 20, 2025

Release Date bug fix to maintain parity with prod CMS #894

Open

13 tasks

Merge branch 'main' into Sitewide-integration

e978753

randimays requested a review from a team as a code owner February 20, 2025 16:29

va-cms-bot deployed to Tugboat February 20, 2025 16:29 View deployment

timcosgrove mentioned this pull request Feb 20, 2025

Sitewide integration tfc #848

Open

cvalarida approved these changes Feb 22, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Sitewide integration: R&S Checklist migration #837

Sitewide integration: R&S Checklist migration #837

randimays commented Dec 12, 2024 •

edited

Loading

Tasks

Tasks

cvalarida left a comment

cvalarida Feb 13, 2025

randimays Feb 14, 2025

cvalarida Feb 13, 2025

cvalarida Feb 13, 2025

cvalarida Feb 13, 2025

cvalarida Feb 13, 2025

cvalarida Feb 13, 2025

cvalarida Feb 13, 2025

cvalarida Feb 13, 2025

randimays Feb 14, 2025

cvalarida Feb 14, 2025

randimays Feb 14, 2025

randimays commented Feb 14, 2025

cvalarida commented Feb 14, 2025

randimays commented Feb 14, 2025

va-cms-bot commented Feb 20, 2025

cvalarida left a comment

cvalarida Feb 21, 2025

cvalarida Feb 22, 2025

cvalarida Feb 21, 2025

cvalarida Feb 21, 2025

		@@ -1,51 +1,51 @@
		import { createQueries } from 'next-drupal-query'
		import * as PressRelease from './pressRelease'

Sitewide integration: R&S Checklist migration #837

Are you sure you want to change the base?

Sitewide integration: R&S Checklist migration #837

Conversation

randimays commented Dec 12, 2024 • edited Loading

Description

Ticket

Developer Task

Tasks

Testing Steps

QA steps

Screenshots

Events - Desktop

/outreach-and-events/events

/central-western-massachusetts-health-care/events/73034/

/outreach-and-events/events/75180/

Events - Tablet

/outreach-and-events/events

Events - Mobile

/outreach-and-events/events

/outreach-and-events/events/75180/

/central-western-massachusetts-health-care/events/73034/

Is this PR blocked by another PR?

Reviewer

Reviewing a PR

Standard Checks

Tasks

Merging an Approved Layout

Merging a Non-Approved Layout

cvalarida left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

randimays commented Feb 14, 2025

cvalarida commented Feb 14, 2025

randimays commented Feb 14, 2025

va-cms-bot commented Feb 20, 2025

cvalarida left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

randimays commented Dec 12, 2024 •

edited

Loading

`/outreach-and-events/events`

`/central-western-massachusetts-health-care/events/73034/`

`/outreach-and-events/events/75180/`

`/outreach-and-events/events`

`/outreach-and-events/events`

`/outreach-and-events/events/75180/`

`/central-western-massachusetts-health-care/events/73034/`