Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: New Component SiteAlert #1099

Merged
merged 29 commits into from
Apr 26, 2021
Merged
Show file tree
Hide file tree
Changes from 26 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
ac34b15
Add files, render some text in a div in Storybook, add one test
SirenaBorracha Apr 6, 2021
c1d88db
Add minimum Storybook examples, add test to check for application of …
SirenaBorracha Apr 6, 2021
8fa9302
Update Storybook examples to match USWDS, add test data specific to e…
SirenaBorracha Apr 6, 2021
379cbbf
Make Storybook examples match those in USWDS
SirenaBorracha Apr 6, 2021
46bd6f7
Add class name to Storybook links so they render the correct color
SirenaBorracha Apr 6, 2021
255d10d
Additional conditional for not applying no-header to slim site alert
SirenaBorracha Apr 6, 2021
61f326a
EsLint disable anchor check, update anchors with simplified href values
SirenaBorracha Apr 6, 2021
1fcd8f3
Add test checking that the component renders with both slim and no-ic…
SirenaBorracha Apr 6, 2021
6d06203
Fix conditional application of no-heading class so that it is not app…
SirenaBorracha Apr 7, 2021
08f2d38
Update Storybook withList examples with custom lists
SirenaBorracha Apr 7, 2021
a1d396d
Export SiteAlert from index.ts
SirenaBorracha Apr 7, 2021
bf5355b
Expand on SiteAlert tests
SirenaBorracha Apr 7, 2021
c90bd6a
Respond to PR feedback, add test for overwriting the aria-label when …
SirenaBorracha Apr 7, 2021
5454a4e
Missed a comment on the PR; update anchors to use internal Link compo…
SirenaBorracha Apr 7, 2021
d1c50cc
Factor query into single variable, add test checking for passed in he…
SirenaBorracha Apr 8, 2021
0f0cdb2
Attempt at adding controls for slim and showIcon styling differences …
SirenaBorracha Apr 8, 2021
69737ff
Use storybook controls to add storybook example Alert With Custom Con…
SirenaBorracha Apr 8, 2021
6f658dd
Use Link instead of anchor in .stories and .tests, update classNames …
SirenaBorracha Apr 8, 2021
2b66fcf
Merge branch 'main' into ak-new-component-site-alert-981
SirenaBorracha Apr 9, 2021
87de42f
Merge branch 'main' into ak-new-component-site-alert-981
SirenaBorracha Apr 12, 2021
bb32c87
Merge branch 'main' into ak-new-component-site-alert-981
SirenaBorracha Apr 15, 2021
c164a87
Merge branch 'main' into ak-new-component-site-alert-981
SirenaBorracha Apr 19, 2021
8aab95c
Merge branch 'main' into ak-new-component-site-alert-981
SirenaBorracha Apr 20, 2021
6e1db38
Remove whitespace from package.json
SirenaBorracha Apr 23, 2021
eb7d678
Merge branch 'main' into ak-new-component-site-alert-981
SirenaBorracha Apr 23, 2021
485b301
Merge branch 'main' into ak-new-component-site-alert-981
SirenaBorracha Apr 23, 2021
7315713
Remove weird whitespace addition to yarn.lock
SirenaBorracha Apr 26, 2021
a1e709f
Merge branch 'ak-new-component-site-alert-981' of github.com:trusswor…
SirenaBorracha Apr 26, 2021
9a023c6
Merge branch 'main' into ak-new-component-site-alert-981
brandonlenz Apr 26, 2021
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
149 changes: 149 additions & 0 deletions src/components/SiteAlert/SiteAlert.stories.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,149 @@
/* eslint-disable jsx-a11y/anchor-is-valid */

import React from 'react'

import { SiteAlert } from './SiteAlert'
import { Link } from '../Link/Link'

export default {
title: 'Components/SiteAlert',
component: SiteAlert,
parameters: {
docs: {
description: {
component: `
### USWDS 2.0 SiteAlert component

Source: http://designsystem.digital.gov/components/site-alert
`,
},
},
},
argTypes: {
slim: {
control: {
type: 'boolean',
},
},
showIcon: {
control: {
type: 'boolean',
},
},
variant: {
control: {
type: 'select',
options: ['info', 'emergency'],
},
defaultValue: 'info',
},
},
}

const infoHeading = 'Short alert message'

const additionalContext = (
<p className="usa-alert__text">
Additional context and followup information including{' '}
<Link className="usa-link" href="#">
brandonlenz marked this conversation as resolved.
Show resolved Hide resolved
a link
</Link>
.
</p>
)

const emergencyHeading = 'Emergency alert message'

const infoWithList = (
<ul className="usa-list">
<li>
The primary informational message and{` `}
<Link href="#">a link</Link>
{` `}for supporting context.
</li>
<li>
Another message,{` `}
<Link href="#">and another link</Link>.
</li>
<li>A final informational message.</li>
</ul>
)

const emergencyWithList = (
<ul className="usa-list">
<li>
The primary emergency message and{` `}
<Link href="#">a link</Link>
{` `}for supporting context.
</li>
<li>
Another message,{` `}
<Link href="#">and another link</Link>.
</li>
<li>A final emergency message.</li>
</ul>
)

const shortAlertContent = (
<p className="usa-alert__text">
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 😆

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

<strong>Short alert message.</strong>
&nbsp;Additional context and followup information including&nbsp;
<Link href="#">a link</Link>.
</p>
)

export const standardInformationalSiteAlert = (): React.ReactElement => (
<SiteAlert variant="info" heading={infoHeading}>
{additionalContext}
</SiteAlert>
)

export const standardEmergencySiteAlert = (): React.ReactElement => (
<SiteAlert variant="emergency" heading={emergencyHeading}>
{additionalContext}
</SiteAlert>
)

export const informationalAlertWithNoHeader = (): React.ReactElement => (
<SiteAlert variant="info">{shortAlertContent}</SiteAlert>
)

export const emergencyAlertWithNoHeader = (): React.ReactElement => (
<SiteAlert variant="emergency">{shortAlertContent}</SiteAlert>
)

export const informationalAlertWithList = (): React.ReactElement => (
<SiteAlert variant="info" heading={infoHeading}>
{infoWithList}
</SiteAlert>
)

export const emergencyAlertWithList = (): React.ReactElement => (
<SiteAlert
variant="emergency"
heading={emergencyHeading}
aria-label="Site alert">
{emergencyWithList}
</SiteAlert>
)

export const slimEmergencyAlert = (): React.ReactElement => (
<SiteAlert slim variant="emergency">
{shortAlertContent}
</SiteAlert>
)

export const emergencyAlertNoIcon = (): React.ReactElement => (
<SiteAlert showIcon={false} variant="emergency">
{shortAlertContent}
</SiteAlert>
)

export const alertWithCustomControls = (argTypes): React.ReactElement => (
<SiteAlert
slim={argTypes.slim}
showIcon={argTypes.showIcon}
variant={argTypes.variant}>
{shortAlertContent}
</SiteAlert>
)
124 changes: 124 additions & 0 deletions src/components/SiteAlert/SiteAlert.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,124 @@
/* eslint-disable jsx-a11y/anchor-is-valid, react/jsx-key */
import React from 'react'
import { render } from '@testing-library/react'

import { Link } from '../Link/Link'

import { SiteAlert } from './SiteAlert'

const testChildren = (
<p className="usa-alert__text">
some default text&nbsp;
<Link href="#">with a link</Link>.
</p>
)

const testDefaultProps = {
heading: 'test heading',
children: testChildren,
}

const testChildrenWithList = (
<ul>
<li>
some default text&nbsp;
<Link href="#">with a link&nbsp;</Link>
</li>
<li>
another list item&nbsp;
<Link href="#">with a link</Link>
</li>
<li>another list item, no link</li>
</ul>
)

describe('SiteAlert component', () => {
it('renders without errors', () => {
const { getByTestId, getByRole } = render(
<SiteAlert variant="info" {...testDefaultProps} />
)

expect(getByTestId('siteAlert')).toBeInTheDocument()
expect(getByRole('link')).toBeInTheDocument()
})

it('accepts a className', () => {
const { getByTestId } = render(
<SiteAlert
variant="info"
className="custom-class-name"
{...testDefaultProps}
/>
)

expect(getByTestId('siteAlert')).toHaveClass(
'usa-site-alert usa-site-alert--info custom-class-name'
)
})

it('accepts a custom aria-label', () => {
const { getByTestId } = render(
<SiteAlert
variant="emergency"
aria-label="custom aria label"
{...testDefaultProps}
/>
)

expect(getByTestId('siteAlert')).toHaveAttribute(
'aria-label',
'custom aria label'
)
})

it('renders a passed in heading', () => {
const { getByRole, queryByText } = render(
<SiteAlert variant="info" {...testDefaultProps} />
)

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

it('renders emergency site alert without errors', () => {
const { getByTestId } = render(
<SiteAlert variant="emergency" {...testDefaultProps} />
)

expect(getByTestId('siteAlert')).toHaveClass(
'usa-site-alert usa-site-alert--emergency'
)
})

it('renders passed in link', () => {
const { getByRole } = render(
<SiteAlert variant="info">{testChildren}</SiteAlert>
)

expect(getByRole('link')).toBeInTheDocument()
expect(getByRole('link')).toHaveClass('usa-link')
})

it('renders a passed in list', () => {
const { getAllByRole } = render(
<SiteAlert variant="emergency">{testChildrenWithList}</SiteAlert>
)
expect(getAllByRole('link')).toHaveLength(2)
})

it('renders slim and no icon when passed both, and does not apply no-header class', () => {
const { getByTestId } = render(
<SiteAlert variant="info" slim={true} showIcon={false}>
{testChildren}
</SiteAlert>
)
expect(getByTestId('siteAlert')).toHaveClass(
'usa-site-alert usa-site-alert--info usa-site-alert--no-icon usa-site-alert--slim'
)
expect(getByTestId('siteAlert')).not.toHaveClass(
'usa-site-alert--no-heading'
)
})
})
49 changes: 49 additions & 0 deletions src/components/SiteAlert/SiteAlert.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
import React from 'react'
import classnames from 'classnames'

interface SiteAlertProps {
variant: 'info' | 'emergency'
children: React.ReactNode
heading?: string
showIcon?: boolean
slim?: boolean
className?: string
}

export const SiteAlert = ({
variant,
children,
heading,
showIcon = true,
slim = false,
className,
...sectionProps
}: SiteAlertProps & JSX.IntrinsicElements['section']): React.ReactElement => {
const classes = classnames(
'usa-site-alert',
{
'usa-site-alert--info': variant === 'info',
'usa-site-alert--emergency': variant === 'emergency',
'usa-site-alert--no-heading': heading === undefined && !slim,
brandonlenz marked this conversation as resolved.
Show resolved Hide resolved
'usa-site-alert--no-icon': !showIcon,
'usa-site-alert--slim': slim,
},
className
)
return (
<section
data-testid="siteAlert"
className={classes}
aria-label="Site alert"
{...sectionProps}>
<div className="usa-alert">
<div className="usa-alert__body">
{heading && <h3 className="usa-alert__heading">{heading}</h3>}
{children}
</div>
</div>
</section>
)
}

export default SiteAlert
2 changes: 2 additions & 0 deletions src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,8 @@ export { StepIndicatorStep } from './components/stepindicator/StepIndicatorStep/

export { Search } from './components/Search/Search'

export { SiteAlert } from './components/SiteAlert/SiteAlert'

/** Truss-designed components */
export {
Modal,
Expand Down
1 change: 1 addition & 0 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -13948,6 +13948,7 @@ uswds@2.10.3:
version "2.10.3"
resolved "https://registry.yarnpkg.com/uswds/-/uswds-2.10.3.tgz#16d34cee81897762d6d69d3ac83aa55129826fa6"
integrity sha512-krNRzx1jRzOJpuH/qtmQhd5zxnXTaDVqrPNYT99sJbxzWUqjb1zZHh3jFNo+xKDpNuiO0XMPwZwlaSp2YdZ3Ag==

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

where did this white space come from?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have no idea. Removing it.

dependencies:
classlist-polyfill "^1.0.3"
del "^6.0.0"
Expand Down