feat: New Component SiteAlert

[SirenaBorracha]

Summary

This PR adds the USWDS SiteAlert component.

USWDS SiteAlert has four variants: info, emergency, slim, and no-icon that each apply different styles to the <section> root element. Brandon pointed out that these styles are not all mutually exclusive.

In the pursuit of customizability, instead of including them all as possible values of the variant prop, two of the four are passed in as individual, optional properties: showIcon and slim. Separate props are preferred so that these styles can be combined with the two mutually exclusive variants, info and emergency, however the user wants.

The USWDS examples don't show all possible combinations, but I've included stories for the most likely combinations. In the last example, at Hana's suggestion, I used Storybook Controls to show the different applications and combinations of styles with the slim, showIcon, and variant properties. If your control panel is hidden in Storybook, pressing the D key should make it reappear. Controls are only available for the last Storybook example.

Related Issues or PRs

closes #981

How To Test

Check out this branch and run yarn test to execute tests and yarn storybook to see the new component in action.

Alternatively, you can scroll to the bottom of this page and click "Show environments" on the right above the comment input box to access this component in Storybook. To see the different styles applied with Controls on the last story, press the D key in Storybook to access your controls panel and toggle the application of the slim, showIcon, and variant properties.

Screenshots

* Please note: I also thought the links were different colors because I accidentally took screenshots using one regular and one incognito browser window, but the colors are actually the result of USWDS styling

info links are blue, emergency links are grey

Standard SiteAlert, informational and emergency

SiteAlert no header, informational and emergency

SiteAlert with list, informational and emergency

Slim Emergency Alert, Emergency Alert No Icon

SiteAlert With Custom Controls (default values)

SiteAlert With Custom Controls slim, showIcon, and variant

[brandonlenz]

At initial glance, this <p> is something I would want to encapsulate similar to #1100 (generifying the usa-alert__text class into an AlertText component could be used for a child of both Alert and SiteAlert). That can be an enhancement.

I'm also thinking about how Alert (which is very similar) is very rigid, while SiteAlert has this flexible approach. I think the SiteAlert approach is in line with our current design philosophy.

Does that then mean we should add a backlog item for a breaking release where we change Alert to be a bit more flexible like SiteAlert, and allow things like lists to be passed in? Mostly just thinking out loud and noticing some things that would be nice for us to decide how to be consistent about.

[haworku]

Good call out. Another idea: I could see the children of SiteAlert (and honestly Alert as well) accepting children of type string | JSX.element. If a string is passed in we put the string in a <p> tag with the usa-alert class) but if a component is passed in we just display that component. What do folks think of that approach?

In terms of refactoring Alert - I think that could be a good task for the backlog. It probably should be handling flexible content. Right now it uses the validation prop to adjust classes but also to be more flexible (validations are lists) which is quite weird. I think I wrote that code too 😆

[brandonlenz]

I like that approach! We might need to expand to accept string | JSX.Element | JSX.Element[] in the event of multiple child elements.

I think the only advantage of making the <AlertText /> subcomponent is for the scenario where someone wanted to do something like:

<SiteAlert>
  <AlertText>Here's some text about the list below:<AlertText />
  <ul className="usa-list">
    <li>something</li>
    <li>something else</li>
  </ul>
<SiteAlert>

The string | JSX.Element | JSX.Element[] approach doesn't preclude that so we could proceed that route and always leave AlertText as an enhancement should the need arise.

[haworku]

The string | JSX.Element | JSX.Element[] approach doesn't preclude that so we could proceed that route and always leave AlertText as an enhancement should the need arise.

Nice.

[haworku]

LGTM! I wanted to check a couple things though a bit outside the actual React component work here:

• @SirenaBorracha have you explored storybook controls at all? It might be something you would be interested in. We could very easily reduce our storybook stories and instead show the slim and showIcon differences with Storybook controls. I'd be happy to pair with you in the future to check out how that would work.
• @brandonlenz @suzubara Didn't we say we want to start trying to make the uswds version bumps their own separate PR? Maybe this would be a good time.

[haworku]

Good call out. Another idea: I could see the children of SiteAlert (and honestly Alert as well) accepting children of type string | JSX.element. If a string is passed in we put the string in a <p> tag with the usa-alert class) but if a component is passed in we just display that component. What do folks think of that approach?

In terms of refactoring Alert - I think that could be a good task for the backlog. It probably should be handling flexible content. Right now it uses the validation prop to adjust classes but also to be more flexible (validations are lists) which is quite weird. I think I wrote that code too 😆

[haworku]

Suggestion: This test could include a check that the heading text is indeed rendered. Right now we just check that a heading exists, but not its text content.

[brandonlenz]

ooo and a quick follow up on these lines since I didn't notice on my first pass:

@SirenaBorracha if you ever find yourself doing getBy___ or queryBy___ for the same thing multiple times in a test you can factor that out into a const, e.g:

const heading = getByRole('heading')
expect(heading).toBeInTheDocument()
expect(heading).toHaveClass('usa-alert__heading')

I'm not sure if the performance impact of querying the dom for a specific element is high, but its an easy optimization we can do to keep the tests efficient.

[brandonlenz]

@brandonlenz @suzubara Didn't we say we want to start trying to make the uswds version bumps their own separate PR? Maybe this would be a good time.

@haworku yep! We've got #1097 for this effort. I got started on it today and hope to finish this week so we can get that in before these. Among other things it'll make the Happo diff for this PR just show the SiteAlert added renders. So we might get this to a point where its "approved" but hold off on merging to main. We have a couple of outstanding 2.9.0 PRs that probably should also go through first as well*.

* Unless we go with Option 2.A in the release & branching ADR in which case we just get things merge in whatever order they're ready 😉

[brandonlenz]

I like that approach! We might need to expand to accept string | JSX.Element | JSX.Element[] in the event of multiple child elements.

I think the only advantage of making the <AlertText /> subcomponent is for the scenario where someone wanted to do something like:

<SiteAlert>
  <AlertText>Here's some text about the list below:<AlertText />
  <ul className="usa-list">
    <li>something</li>
    <li>something else</li>
  </ul>
<SiteAlert>

The string | JSX.Element | JSX.Element[] approach doesn't preclude that so we could proceed that route and always leave AlertText as an enhancement should the need arise.

[SirenaBorracha]

...
@ SirenaBorracha have you explored storybook controls at all? It might be something you would be interested in. We could very easily reduce our storybook stories and instead show the slim and showIcon differences with Storybook controls. I'd be happy to pair with you in the future to check out how that would work.
...

@haworku I'd prefer to remove some of those stories in this PR given the choice.

I just spent ~30m trying to get controls working for slim and then variant following the instructions, but I'm having trouble. The panel doesn't show in Chrome because I closed it that one time, so instead I'm using Safari, but then even when I do see the panel in Safari there's an alert above it (lol meta) "This story is not configured to handle controls" with a link to the Controls README. I just threw some time on @brandonlenz's calendar for tomorrow, but if he and I can't figure it out I'll put time on yours 🙃

[brandonlenz]

🥳

[brandonlenz]

Oh I approved preemptively. Could you remove the empty whitespace added to package.json to this one as well? I'll eagerly approve after that!

[suzubara]

where did this white space come from?

[SirenaBorracha]

I have no idea. Removing it.