Include text directly in components (removing verbiage files)

[benmartin-coforma]

Existing Pattern

The pattern in our other apps is to create verbiage objects, with text properties. Many components have a required verbiage prop, which provides the text of that component.

In theory, this approach allows us to

1. Control text versions - by storing old verbiage objects, we can reproduce how the app appeared in the past
2. More easily internationalize - by passing in verbiage/es/help.ts instead of verbiage/en/help.ts, we could render a Spanish version of the Help page with no changes to components/pages/Help.tsx
3. Increase component reuse - if we have the same visual construct multiple places in the app, which differ only in text, we could reuse the same component markup by providing the appropriate verbiage in each location.

What does this PR change

This PR embeds most of our app's text directly in the component code.

The big win here is simplicity - by removing the layer of indirection between component and text, our app becomes easier to understand and (hopefully) easier to change as needs arise.

We lose the advantages outlined above, but

1. Our only need to control text versions is within reports; we have never had a need to version the contents of, say, the Help page. (Nor did we have a mechanism to do so; we would have had to build that first.)
2. We do not have a need to internationalize; we are developing websites for use by U.S. government officials, and nearly all official government work is done in English. Should this need ever arise, we can and should take a more systematic approach to fulfill it.
3. Component reuse enabled by verbiage swaps rarely arises. In most cases where it does, I believe the simplicity win offsets the redundancy price.

The final disadvantage I can think of is a minor one: our repo becomes slightly less greppable. Whereas we ignore our line-length prettification rules in verbiage files, they do affect formatting in component files. Therefore, searching our repo for a phrase copied from the site may no longer find that text - because it may include line breaks in the component file. Again, I believe this is more than offset by the simplicity win (especially given the other ways to find pages in our codebase, such as by consulting AppRoutes.tsx)

Change Details

• The FaqAccordion, InstructionsAccordion, and TemplateCardAccordion have been removed. Each was built to accept verbiage objects, and could display very different things based on the shape of the verbiage that was passed in (text, a table, a list, or even a CustomHtml object). In almost all cases, the shape of the verbiage was static, so the accordion could be inlined in the parent component without far-reaching consequences (for example, the Help page now uses Accordion & AccordionItem directly, rather than through the FaqAccordion).
  • The one exception was components/report/Element.tsx:accordionElement() - it was using a TemplateCardAccordion, specifically for its ability to parseCustomHtml. This is an instance of Point 1 mentioned earlier: we do version report verbiage. Therefore this function now calls parseCustomHtml directly (and does so within a direct use of Accordion and AccordionItem).
  • While making accordion changes, I noticed that AccordionItem had a [key: string]: any escape hatch in its Props type. The only prop we ever pass here (other than label and children, which were already listed explicitly) is sx, which gets passed through to the underlying Chakra component. This was (potentially) overriding the sx included in the component itself. I removed the escape hatch and made this override behavior more explicit.
    • This override doesn't currently cause any unexpected behavior. The default style just has borderStyle: none, and everywhere we pass in an override, that override includes its own borderStyle: none. This is a potentially precarious situation - if we decide to add a property to the default style, or if we override it from a new location without passing in a borderStyle, there's an unfortunate potential for time spent trying to figure out why things look wrong. But I don't want to add anything more to this PR, so we will have to solve for that later (if ever).
• The Alert component now takes its body as a standard React children prop, rather than a CustomHtmlElement description prop. This is a more natural way for us to declare alerts in general: as <Alert>my alert content</Alert> rather than <Alert description="my alert content" />
  • One of the places we do this is when creating errors specific to the admin banner form. And those errors are the one verbiage file that has survived in this PR. It maps error codes from the backend to more user-friendly descriptions on the frontend, and we then embed those descriptions into an Alert component. I may still attempt to refactor this further; perhaps we could skip the description step, and map directly from error code to Alert component. But for now I am happy that the CustomHtmlElement[] has been removed from verbiage/error.ts.
  • We also use <Alert> from <Banner> and <ErrorAlert>. Both of those have been changed to pass children instead of description, as well as everything that uses those.
  • While reworking Alert, I removed the [key: string]: any escape hatch from its props as well. As with the AccordionItem props, this currently causes no unexpected behavior - except in the case of any error alerts from LoginCognito.tsx. Since that is not a customer-facing page, I don't solve for it in this PR.
• The Error page (which is separate from the Alert and ErrorAlert components) had very straightforward changes - just copying strings from its verbiage file and pasting them into the markup.
  • I also created a constant for the help desk email address. It is used here, and throughout the app.
  • I also stopped using the createEmailLink helper function. It would make some sense if we were building email subjects into our hrefs, but we are not doing so at this time.
    • Perhaps it would make sense to make a tiny helper component specifically for the support email address, since it appears on four separate pages? I did not do that with this PR, but it may be neat.
• The Home page had the biggest refactor. Previously it included 3 instances of TemplateCard, passing different verbiage into each. Now it includes 3 different cards, each of which contains its own verbiage.
  • Each of these three cards (QmsIntroductionCard, TaIntroductionCard, and CaIntroductionCard) wraps all of its contents inside of a ReportIntroCard, which gives them a consistent appearance.
  • Each of these three cards contains a ReportIntroCardActions component, which includes the download link for the User Guide and the navigation to that report type's dashboard.
  • These three cards have about as much redundancy as I'm willing to tolerate. If they grow apart as we learn more about the CI and TA reports, then we've made a good change. If they remain nearly identical, and/or if we add a fourth that is also identical, then we should probably keep looking for an alternative approach.
    • Fortunately, since these components contain no behavior, the unit tests are exceedingly simple: just a render check and an a11y check.
• The Help page was of medium complexity. It contains two cards with short descriptions and email addresses. Previously these were instances of a component called EmailCard. I moved the email addresses out of EmailCard.tsx into the markup of HelpPage.tsx, so that was no longer an appropriate name; EmailCard is now HelpCard.
• The ExportedReportBanner was the easiest change so far - although it highlighted a future need. The "Quality Measure Set" text I copied from the verbiage file will need to have a switch in the future, so that it can display the appropriate report name. The same goes for the meta subject on the ExportedReportPage.
• The AdminDashSelector was another trivial copy-paste job - as was the AdminPage, the NotFound page, and the Profile page.
• The Dashboard page has an inlined accordion, and another instance of "Quality Measure Set" text that will need to be adjusted, same as the ExportedReportBanner.

Related ticket(s)

N/A

---

How to test

Log in to the site and poke around! There should be no visual or behavioral changes. I'm 90% sure I didn't screw anything up.

Important updates

Uh... see above. If you're adding a new component, and it feels like adding a verbiage file would be a natural solution, let's talk! We may be able to come up with an alternative approach. Or, we may not! My goal isn't to eradicate all verbiage everywhere - my goal is to make the app as simple as possible.

---

Author checklist

• I have performed a self-review of my code
• I have added thorough tests, if necessary
• I have updated relevant documentation, if necessary

[angelaco11]

Awesome work here, I fully support this effort. Going through this PR and seeing less abstraction, less complexity and less obscurity was a true pleasure!

[rocio-desantiago]

where we have email links, we may need to add spaces with {" "}

[rocio-desantiago]

Errors page and Not Found page

[codeclimate]

Code Climate has analyzed commit f58776b and detected 0 issues on this pull request.

The test coverage on the diff in this pull request is 96.9% (90% is the threshold).

This pull request will bring the total coverage in the repository to 93.8% (0.0% change).

View more on Code Climate.

[rocio-desantiago]

Awesome work, Ben!