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-Site - Header: Add variants table to guidance page #2139

Merged
merged 12 commits into from
Jul 17, 2023

Conversation

mahoneycm
Copy link
Contributor

@mahoneycm mahoneycm commented Jun 13, 2023

Summary

Added header variants table to guidance page. This change also removes empty "Using the header" section.

Related issue

Closes #1976

Preview link

Preview link:
Header variants →
Header changelog →

Problem statement

The Header guidance page was missing a variants table that can be found in other component guidance pages. There were also empty implementation sections due to each component containing variants in its frontmatter, which were not displayed in the sections.

Solution

Move the variants out of the individual header variants front matter and include it in the base header guidance markdown page.

Testing and review

  1. Visit Header guidance page
  2. View variants table and check for missing variants
  3. Check each variant section and check for empty implementation sections (screenshots below)

Screenshots

Empty implementation section

There's an empty Using the... section for each variant
image
As seen on live site →

New variants table

image

Before opening this PR, make sure you’ve done whichever of these applies to you:

  • Confirm that this code follows the 18F Front End Coding Style Guide and Accessibility Guide.
  • Run git pull origin [base branch] to pull in the most recent updates from your base and check for merge conflicts. (Often, the base branch is main).
  • Run npm test and confirm that all tests pass.
  • Run your code through HTML_CodeSniffer and make sure it’s error free.

@mahoneycm mahoneycm marked this pull request as ready for review June 13, 2023 15:52
Copy link
Contributor

@amyleadem amyleadem left a comment

Choose a reason for hiding this comment

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

Thanks for finding and fixing this, @mahoneycm! Overall it looks good, I just added a couple suggestions for small tweaks in the changelog.

  • Confirmed listed variants match those found in USWDS
  • Confirmed the empty implementation sections have been removed
  • Proofed for grammar and spelling

_data/changelogs/component-header.yml Outdated Show resolved Hide resolved
_data/changelogs/component-header.yml Outdated Show resolved Hide resolved
mahoneycm and others added 2 commits July 3, 2023 16:04
Co-authored-by: Amy Leadem <93996430+amyleadem@users.noreply.github.com>
Co-authored-by: Amy Leadem <93996430+amyleadem@users.noreply.github.com>
@mahoneycm mahoneycm requested a review from amyleadem July 3, 2023 20:05
@mahoneycm
Copy link
Contributor Author

@mejiaj Bumping for review

Copy link
Contributor

@amyleadem amyleadem left a comment

Choose a reason for hiding this comment

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

Looks good to me!

Just flagging that we should update the changelog date before merge.

Copy link
Contributor

@mejiaj mejiaj left a comment

Choose a reason for hiding this comment

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

Looks good. Confirming:

  • Variants table exists and is accurate.
  • Empty sections are gone.
  • Changelog entry exists.

@mahoneycm would you be able to briefly look into:

  1. A visual regression with touchpoints. If confirmed, can you create an issue if there isn't one already?
  2. Updating changelog date to today, so we can get this merged.

Preview (left) / Live (right)
image

@mahoneycm
Copy link
Contributor Author

@mejiaj Date updated and issue created, chef! 🫡

#2186

Copy link
Contributor

@mejiaj mejiaj left a comment

Choose a reason for hiding this comment

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

Thanks! I've pulled in latest from main branch.

@mejiaj mejiaj merged commit 3378dc2 into main Jul 17, 2023
@mejiaj mejiaj deleted the cm-header-variant-guidance branch July 17, 2023 15:09
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.

USWDS-Site - Bug: Header has empty implementation sections
3 participants