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: Introduce the USWDS Site Alert as info banner, use the USWDS Banner as intended #1328

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

dzole0311
Copy link
Collaborator

@dzole0311 dzole0311 commented Dec 16, 2024

Closes: #1235

Demo: https://www.loom.com/share/f7c5868033054d859a0a4177722951a6?sid=42347137-07f4-46de-8f1d-25021ccf3204

Description of Changes

  1. Replaced the Banner component with SiteAlert but kept some of the core functionalities such as the expiration date feature and close button
  2. Re-factored the Banner to align with its intended USWDS role i.e. showing government information at the top of every page

Notes & Questions About Changes

I'm still a little unsure whether the Banner and SiteAlert components should live within the veda-ui library or be implemented specifically within the instances. Since Banner was already part of the veda-ui, I followed the same approach for the SiteAlert.

Validation / Testing

See the configuration for the SiteAlert and the Banner here. Note: Run yarn clean to clear the Parcel cache if the changes you make in the config are not reflected in the UI

Banner testing:

  1. Check that the banner appears consistently across all pages when navigating routes
  2. Check that the expand/collapse button ("Here's how you know") works
  3. Check that the banner looks good on smaller screens as well

SiteAlert testing:

  1. Check that the SiteAlert is shown when:
  • Content exists for the alert
  • The expiration date is not reached
  • The alert has not been dismissed by the user
  1. Check these refresh scenarios:
  • Without closing the alert, refresh the page to verify that it reappears
  • After closing the alert, refresh the page to verify that it remains hidden

Copy link

netlify bot commented Dec 16, 2024

Deploy Preview for veda-ui ready!

Name Link
🔨 Latest commit 04b81e7
🔍 Latest deploy log https://app.netlify.com/sites/veda-ui/deploys/6762896e718d2e0008258431
😎 Deploy Preview https://deploy-preview-1328--veda-ui.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@dzole0311 dzole0311 marked this pull request as draft December 16, 2024 14:01
@dzole0311 dzole0311 marked this pull request as ready for review December 16, 2024 14:22
@dzole0311 dzole0311 changed the title feat: Introduce the USWDS Site Alert as a info banner, use the USWDS Banner as intended feat: Introduce the USWDS Site Alert as info banner, use the USWDS Banner as intended Dec 16, 2024
@@ -181,6 +182,7 @@
"google-polyline": "^1.0.3",
"gulp-postcss": "^10.0.0",
"gulp-sass": "^6.0.0",
"he": "^1.2.0",
Copy link
Member

Choose a reason for hiding this comment

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

What is he doing? 😄

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I needed a way to properly decode the html string passed in the configuration for the banner, so it would be rendered as a valid html, but I'm open to other approaches or suggestions.

app/scripts/components/common/banner/index.tsx Outdated Show resolved Hide resolved
app/scripts/components/common/banner/index.tsx Outdated Show resolved Hide resolved
app/scripts/components/common/banner/index.tsx Outdated Show resolved Hide resolved
app/scripts/components/common/banner/index.tsx Outdated Show resolved Hide resolved
app/scripts/components/common/site-alert/index.tsx Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants