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.

Sign up for GitHub

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

feat: New Component SiteAlert #1099

Show file tree
Hide file tree
Changes from 1 commit
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
25 changes: 13 additions & 12 deletions src/components/SiteAlert/SiteAlert.stories.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import React from 'react'

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

export default {
title: 'Components/SiteAlert',
Expand All @@ -25,9 +26,9 @@ const infoHeading = 'Short alert message'
const additionalContext = (
<p className="usa-alert__text">
Additional context and followup information including{' '}
<a className="usa-link" href="#">
<Link className="usa-link" href="#">
brandonlenz marked this conversation as resolved.
Show resolved Hide resolved
a link
</a>
</Link>
.
</p>
)
Expand All @@ -38,16 +39,16 @@ const infoWithList = (
<ul className="usa-list">
<li>
The primary informational message and{` `}
<a className="usa-link" href="#">
<Link className="usa-link" href="#">
a link
</a>
</Link>
{` `}for supporting context.
</li>
<li>
Another message,{` `}
<a className="usa-link" href="#">
<Link className="usa-link" href="#">
and another link
</a>
</Link>
.
</li>
<li>A final informational message.</li>
Expand All @@ -58,16 +59,16 @@ const emergencyWithList = (
<ul className="usa-list">
<li>
The primary emergency message and{` `}
<a className="usa-link" href="#">
<Link className="usa-link" href="#">
a link
</a>
</Link>
{` `}for supporting context.
</li>
<li>
Another message,{` `}
<a className="usa-link" href="#">
<Link className="usa-link" href="#">
and another link
</a>
</Link>
.
</li>
<li>A final emergency message.</li>
Expand All @@ -78,9 +79,9 @@ 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;
<a className="usa-link" href="#">
<Link className="usa-link" href="#">
a link
</a>
</Link>
.
</p>
)
Expand Down
58 changes: 28 additions & 30 deletions yarn.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5389,18 +5389,18 @@ define-property@^2.0.2:
is-descriptor "^1.0.2"
isobject "^3.0.1"

del@^5.1.0:
version "5.1.0"
resolved "https://registry.yarnpkg.com/del/-/del-5.1.0.tgz#d9487c94e367410e6eff2925ee58c0c84a75b3a7"
integrity sha512-wH9xOVHnczo9jN2IW68BabcecVPxacIA3g/7z6vhSU/4stOKQzeCRK0yD0A24WiAAUJmmVpWqrERcTxnLo3AnA==
del@^6.0.0:
version "6.0.0"
resolved "https://registry.yarnpkg.com/del/-/del-6.0.0.tgz#0b40d0332cea743f1614f818be4feb717714c952"
integrity sha512-1shh9DQ23L16oXSZKB2JxpL7iMy2E0S9d517ptA1P8iw0alkPtQcrKH7ru31rYtKwF499HkTu+DRzq3TCKDFRQ==
dependencies:
globby "^10.0.1"
graceful-fs "^4.2.2"
globby "^11.0.1"
graceful-fs "^4.2.4"
is-glob "^4.0.1"
is-path-cwd "^2.2.0"
is-path-inside "^3.0.1"
p-map "^3.0.0"
rimraf "^3.0.0"
is-path-inside "^3.0.2"
p-map "^4.0.0"
rimraf "^3.0.2"
slash "^3.0.0"

delayed-stream@~1.0.0:
Expand Down Expand Up @@ -6450,7 +6450,7 @@ fast-glob@^2.2.6:
merge2 "^1.2.3"
micromatch "^3.1.10"

fast-glob@^3.0.3, fast-glob@^3.1.1, fast-glob@^3.2.5:
fast-glob@^3.1.1, fast-glob@^3.2.5:
version "3.2.5"
resolved "https://registry.yarnpkg.com/fast-glob/-/fast-glob-3.2.5.tgz#7939af2a656de79a4f1901903ee8adcaa7cb9661"
integrity sha512-2DtFcgT68wiTTiwZ2hNdJfcHNke9XOfnwmBRWXhmeKM8rF0TGwmC/Qto3S7RoZKp5cilZbxzO5iTNTQsJ+EeDg==
Expand Down Expand Up @@ -7192,18 +7192,16 @@ [email protected]:
merge2 "^1.3.0"
slash "^3.0.0"

globby@^10.0.1:
version "10.0.2"
resolved "https://registry.yarnpkg.com/globby/-/globby-10.0.2.tgz#277593e745acaa4646c3ab411289ec47a0392543"
integrity sha512-7dUi7RvCoT/xast/o/dLN53oqND4yk0nsHkhRgn9w65C4PofCLOoJ39iSOg+qVDdWQPIEj+eszMHQ+aLVwwQSg==
globby@^11.0.1:
version "11.0.3"
resolved "https://registry.yarnpkg.com/globby/-/globby-11.0.3.tgz#9b1f0cb523e171dd1ad8c7b2a9fb4b644b9593cb"
integrity sha512-ffdmosjA807y7+lA1NM0jELARVmYul/715xiILEjo3hBLPTcirgQNnXECn5g3mtR8TOLCVbkfua1Hpen25/Xcg==
dependencies:
"@types/glob" "^7.1.1"
array-union "^2.1.0"
dir-glob "^3.0.1"
fast-glob "^3.0.3"
glob "^7.1.3"
ignore "^5.1.1"
merge2 "^1.2.3"
fast-glob "^3.1.1"
ignore "^5.1.4"
merge2 "^1.3.0"
slash "^3.0.0"

globby@^11.0.2:
Expand Down Expand Up @@ -7251,7 +7249,7 @@ good-listener@^1.2.2:
dependencies:
delegate "^3.1.2"

graceful-fs@^4.1.11, graceful-fs@^4.1.15, graceful-fs@^4.1.2, graceful-fs@^4.1.6, graceful-fs@^4.1.9, graceful-fs@^4.2.0, graceful-fs@^4.2.2, graceful-fs@^4.2.4:
graceful-fs@^4.1.11, graceful-fs@^4.1.15, graceful-fs@^4.1.2, graceful-fs@^4.1.6, graceful-fs@^4.1.9, graceful-fs@^4.2.0, graceful-fs@^4.2.4:
version "4.2.4"
resolved "https://registry.yarnpkg.com/graceful-fs/-/graceful-fs-4.2.4.tgz#2256bde14d3632958c465ebc96dc467ca07a29fb"
integrity sha512-WjKPNJF79dtJAVniUlGGWHYGz2jWxT6VhN/4m1NdkbZ2nOsEF+cI1Edgql5zCRhs/VsQYRvrXctxktVXZUkixw==
Expand Down Expand Up @@ -7740,7 +7738,7 @@ ignore@^4.0.3, ignore@^4.0.6:
resolved "https://registry.yarnpkg.com/ignore/-/ignore-4.0.6.tgz#750e3db5862087b4737ebac8207ffd1ef27b25fc"
integrity sha512-cyFDKrqc/YdcWFniJhzI42+AzS+gNwmUzOSFcRCQYwySuBBBy/KjuxWLZ/FHEH6Moq1NizMOBWyTcv8O4OZIMg==

ignore@^5.1.1, ignore@^5.1.4, ignore@^5.1.8:
ignore@^5.1.4, ignore@^5.1.8:
version "5.1.8"
resolved "https://registry.yarnpkg.com/ignore/-/ignore-5.1.8.tgz#f150a8b50a34289b33e22f5889abd4d8016f0e57"
integrity sha512-BMpfD7PpiETpBl/A6S498BaIJ6Y/ABT93ETbby2fP00v4EbvPBXWEoaR1UBPKs3iR53pJY7EtZk5KACI57i1Uw==
Expand Down Expand Up @@ -8187,10 +8185,10 @@ is-path-cwd@^2.2.0:
resolved "https://registry.yarnpkg.com/is-path-cwd/-/is-path-cwd-2.2.0.tgz#67d43b82664a7b5191fd9119127eb300048a9fdb"
integrity sha512-w942bTcih8fdJPJmQHFzkS76NEP8Kzzvmw92cXsazb8intwLqPibPPdXf4ANdKV3rYMuuQYGIWtvz9JilB3NFQ==

is-path-inside@^3.0.1:
version "3.0.2"
resolved "https://registry.yarnpkg.com/is-path-inside/-/is-path-inside-3.0.2.tgz#f5220fc82a3e233757291dddc9c5877f2a1f3017"
integrity sha512-/2UGPSgmtqwo1ktx8NDHjuPwZWmHhO+gj0f93EkhLB5RgW9RZevWYYlIkS6zePc6U2WpOdQYIwHe9YC4DWEBVg==
is-path-inside@^3.0.2:
version "3.0.3"
resolved "https://registry.yarnpkg.com/is-path-inside/-/is-path-inside-3.0.3.tgz#d231362e53a07ff2b0e0ea7fed049161ffd16283"
integrity sha512-Fd4gABb+ycGAmKou8eMftCupSir5lRxqf4aD/vd0cD2qc4HL07OjCeuHMr8Ro4CoMaeCKDB0/ECBOVWjTwUvPQ==

is-plain-obj@^1.1.0:
version "1.1.0"
Expand Down Expand Up @@ -13947,13 +13945,13 @@ use@^3.1.0:
resolved "https://registry.yarnpkg.com/use/-/use-3.1.1.tgz#d50c8cac79a19fbc20f2911f56eb973f4e10070f"
integrity sha512-cwESVXlO3url9YWlFW/TA9cshCEhtu7IKJ/p5soJ/gGpj7vbvFrAY/eIioQ6Dw23KjZhYgiIo8HOs1nQ2vr/oQ==

uswds@2.9.0:
version "2.9.0"
resolved "https://registry.yarnpkg.com/uswds/-/uswds-2.9.0.tgz#002089d74582960099b0ee836f89f043cdf6d0bc"
integrity sha512-5IMVgMCUUlgWVFrB7Wf1qTvv5L3oDDlSsAgvJ02+/sV2uh4JzO9YPm3493RTaMuHTc+feRCuYyEIloXsEQY0Pg==
uswds@2.10.0:
version "2.10.0"
resolved "https://registry.yarnpkg.com/uswds/-/uswds-2.10.0.tgz#c7ecb779c82302fa189b36ca3eefe59dfde7f782"
integrity sha512-A6HjL42ryf9pdfbm6JrGxZywKzCZraHO4v3Ve21uFDqoA3ijoNjiSYME+3SG86CIgC6zRasrbQVuI3bvd3Xv2w==
dependencies:
classlist-polyfill "^1.0.3"
del "^5.1.0"
del "^6.0.0"
domready "^1.0.8"
elem-dataset "^2.0.0"
lodash.debounce "^4.0.7"
Expand Down