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 - Docs: Add initial Key Benefits USWDS page. #1207

Merged
merged 8 commits into from
Apr 14, 2021
Merged

Conversation

mejiaj
Copy link
Contributor

@mejiaj mejiaj commented Apr 13, 2021

Description

Preview link

Closes uswds/uswds#4150

Before you hit Submit, make sure you’ve done whichever of these applies to you:

  • Follow the 18F Front End Coding Style Guide and Accessibility Guide.
  • Run npm test and make sure the tests for the files you have changed have passed.
  • Run your code through HTML_CodeSniffer and make sure it’s error free.
  • Title your pull request using this format: [Website] - [UI component]: Brief statement describing what this pull request solves.

@mejiaj mejiaj changed the title USWDS - Docs: Add initial Why use USWDS page. USWDS - Docs: Add initial Key Benefits USWDS page. Apr 14, 2021
@mejiaj mejiaj marked this pull request as ready for review April 14, 2021 18:37
@mejiaj mejiaj requested a review from KMullan944 April 14, 2021 18:37
@KMullan944
Copy link

This looks great! Just a few edits, per below.

  • In that intro section where we list the 6 benefits we expound upon in sections below, could you add jump-links to those sections? Or is that too distracting and was it omitted as a design decision?
  • In "Cross-functional design system community" section, there's an external link to Github that has an icon with it. Pls delete.
  • In the footer (above the Identifier?), "Become part of the community," there are 2 external links that need icon removed: Github and Twitter

And I think that's it! It reads well and is clear -- those are my only recommended edits. Thank you :)

@mejiaj
Copy link
Contributor Author

mejiaj commented Apr 14, 2021

@KMullan944 thank you for the feedback! The external links will be updated on a separate pull request (#1204).

Yes, I'll go ahead and update the list so it links below.

pages/about/key-benefits.md Outdated Show resolved Hide resolved
pages/about/key-benefits.md Outdated Show resolved Hide resolved
pages/about/key-benefits.md Outdated Show resolved Hide resolved

<div class="margin-top-7 border-05 border-primary-light bg-primary-lighter padding-3">

<h2 id="how-to-make-the-case" class="margin-top-0">How to make the case for USWDS</h2>
Copy link
Member

Choose a reason for hiding this comment

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

Remove margin

Suggested change
<h2 id="how-to-make-the-case" class="margin-top-0">How to make the case for USWDS</h2>
<h2 id="how-to-make-the-case" class="margin-x-0">How to make the case for USWDS</h2>

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should this be margin-y-0?

pages/about/key-benefits.md Outdated Show resolved Hide resolved
pages/about/key-benefits.md Outdated Show resolved Hide resolved
@thisisdano thisisdano merged commit 338dd55 into main Apr 14, 2021
@thisisdano thisisdano deleted the jm-why-uswds branch April 14, 2021 22:05
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.

Publish System Benefits page
4 participants