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

USWDS - Web components: Beta 2 banner additions #65

Open
wants to merge 24 commits into
base: develop
Choose a base branch
from

Conversation

mejiaj
Copy link
Collaborator

@mejiaj mejiaj commented Sep 4, 2024

Summary

Improvements to Banner. Added JSDocs and component is now configurable via CSS custom properties.

Related issue

Closes #39.

Preview link

Preview link:

Banner preview →

Problem statement

Banner was missing:

  • a11y checklist requirements
  • theme settings
  • JSDoc comments
  • Unit tests

Additionally, the HTTPS lock icon was using unsafe SVG.

Solution

Added all the items above and swapped the SVG icon for a background image.

Major changes

Global CSS properties

Shared global properties created in a new core directory. They contain global token values and theme settings. They're meant to be attached at :root level and leaves :host for component-level settings.

.
├── components
└── core
    ├── colors.css
    ├── fonts.css
    └── index.css

Core stylesheets also added as an export in package.json.

  "exports": {
    "./core/*": "./src/core/*"
  },

Example

:root {
  // Atom/molecule
  --font-stack-source-sans-pro: "Source Sans Pro", "Helvetica Neue", "Helvetica",
    "Roboto", "Arial", sans-serif;
  --font-stack-public-sans: "Public Sans Web", -apple-system, BlinkMacSystemFont,
    "Segoe UI", Roboto, Helvetica, Arial, sans-serif, "Apple Color Emoji",
    "Segoe UI Emoji", "Segoe UI Symbol";

  // System token
  --usa-font-sans: var(--font-stack-source-sans-pro);
  
  // Theme token
  --usa-font-ui: var(--usa-font-sans);
}

Component properties

Caution

Missing the theme setting theme-banner-max-width because media queries don't allow custom props as a value. Potential solutions: SASS, PostCSS custom media queries, or Lightning CSS each requires additional dependencies for everyone.

Property Default value Fallback value Description
--theme-banner-background-color --usa-base-lightest #f0f0f0 Sets banner background color.
--theme-banner-font-family --usa-font-ui system-ui, sans-serif Sets banner font family.
--theme-banner-link-color --theme-link-color #005ea2 Sets the default link color.
--theme-banner-link-hover-color --theme-link-hover-color #1a4480 Sets the default link color.

Link settings and names match those defined in USA Link component.

Lock icon replaced

The icon is added via CSS on a class. This allows users to not worry about pasting in a big block of SVG markup. Especially useful when adding custom content via slots.

Before After
image image

Following pattern set by @aduth in uswds/uswds#5829.

Testing and review

  1. Visit Banner preview
  2. Verify there aren't regressions in functionality.
  3. Unit tests pass.
  4. Theme settings can be overridden.

Dependency updates

Dependency name Previous version New version
@chromatic-com/storybook 1.6.1 1.8.0
@rollup/rollup-linux-x64-gnu 4.20.0 4.21.2
@storybook/addon-a11y 8.2.7 8.2.9
@storybook/addon-essentials 8.2.7 8.2.9
@storybook/addon-links 8.2.7 8.2.9
@storybook/blocks 8.2.7 8.2.9
@storybook/manager-api 8.2.7 8.2.9
@storybook/test 8.2.7 8.2.9
@storybook/theming 8.2.7 8.2.9
@storybook/web-components 8.2.7 8.2.9
@storybook/web-components-vite 8.2.7 8.2.9
@uswds/uswds 3.8.1 3.8.2
axe-playwright 2.0.1 2.0.2 2.0.3
eslint 9.8.0 9.9.1
shadow-dom-testing-library 1.11.2 1.11.3
storybook 8.2.7 8.2.9
vite 5.3.5 5.4.3 5.4.8
@vitest/eslint-plugin - 1.1.4

@@ -57,8 +58,9 @@
"@storybook/theming": "^8.2.9",
"@storybook/web-components": "^8.2.9",
"@storybook/web-components-vite": "^8.2.9",
"@vitest/eslint-plugin": "^1.1.4",
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Helps get rid of errors when writing unit tests.

Before After
image image

Copy link
Contributor

Choose a reason for hiding this comment

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

Looked into this while doing some POAM for this repo. Good addition 👍

@@ -4,58 +4,59 @@ import { html, nothing } from "lit";

import { userEvent, expect, waitFor } from "@storybook/test";
import { within } from "shadow-dom-testing-library";
import { unsafeHTML } from "lit/directives/unsafe-html.js";
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Tip

Helps render custom slotted content in HTTPS body because it includes HTML.

Copy link
Collaborator Author

@mejiaj mejiaj Oct 4, 2024

Choose a reason for hiding this comment

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

Tip

Will look into @property now that it has baseline support.

Comment on lines 34 to +52
<ColorPalette>
<ColorItem
title="Base"
subtitle="Grayscale colors"
colors={{
White: "#FFFFFF",
"Longer custom text": "#F3F3F3",
Black: "#000",
"--usa-base-lightest": "#F0F0F0",
}}
/>
</ColorPalette>

## Global

<ColorPalette>
<ColorItem
title="Link"
subtitle="Link colors"
colors={{
"--usa-theme-link-color": "#005EA2",
"--usa-theme-link-hover-color": "#1A4480",
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Tip

Starting to document the shared properties here.


- Variants
- Properties
- CSS Custom Properties
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Tip

We'll need to remove this if we continue with custom elements manifest.

@mejiaj mejiaj changed the title USWDS - Web components: Beta banner additions USWDS - Web components: Beta 2 banner additions Oct 4, 2024
@mejiaj mejiaj marked this pull request as ready for review October 4, 2024 18:19
Comment on lines +12 to +30
* @summary The usa-banner component.
*
* @attribute {string} lang - The element's language.
* @attribute {string} label - The custom aria label users can override.
* @attribute {string} tld - The top level domain for the site.
*
* @cssprop --theme-banner-background-color - Sets banner background color.
* @cssprop --theme-banner-font-family - Sets banner font family.
* @cssprop --theme-banner-link-color - Sets the default link color.
* @cssprop --theme-banner-link-hover-color - Sets the default link color.
*
* @slot banner-text - The text for official government website text.
* @slot banner-action - Action text label "Here's how you know."
* @slot domain-heading - Heading text for the domain section.
* @slot domain-text - Body text for domain section.
* @slot https-heading - Heading for HTTPs section.
* @slot https-text - Body text for HTTPs section.
*
* @tagname usa-banner
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Note

StorybookJS docs are generated from this. There's currently an issue for hiding internal props. A small downside, but still benefits us in not having to re-write these in Story Docs.

Example
banner-docs

@mejiaj mejiaj requested review from mahoneycm and removed request for amyleadem October 8, 2024 18:23
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.

Web Component Banner: Develop Beta 2
3 participants