Visitar URL original
USWDS - Docs: Add initial Key Benefits USWDS page. by mejiaj · Pull Request #1207 · uswds/uswds-site · GitHub
Skip to content

USWDS - Docs: Add initial Key Benefits USWDS page. - #1207

Merged
thisisdano merged 8 commits into
mainfrom
jm-why-uswds
Apr 14, 2021
Merged

USWDS - Docs: Add initial Key Benefits USWDS page.#1207
thisisdano merged 8 commits into
mainfrom
jm-why-uswds

Conversation

@mejiaj

@mejiaj mejiaj commented Apr 13, 2021 •
edited
Loading

Copy link
Copy Markdown
Contributor

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

KMullan944 commented Apr 14, 2021

Copy link
Copy Markdown

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

mejiaj commented Apr 14, 2021

Copy link
Copy Markdown
Contributor Author

@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.

@mejiaj
mejiaj requested review from afeijoo and thisisdano April 14, 2021 19:53
thisisdano
thisisdano suggested changes Apr 14, 2021
View reviewed changes
Comment thread pages/about/key-benefits.md Outdated
Comment thread pages/about/key-benefits.md Outdated
Comment thread pages/about/key-benefits.md Outdated
Comment thread pages/about/key-benefits.md Outdated

<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>

@thisisdano thisisdano Apr 14, 2021

Copy link
Copy Markdown
Contributor

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>

@mejiaj mejiaj Apr 14, 2021

Copy link
Copy Markdown
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?

Comment thread pages/about/key-benefits.md Outdated
Comment thread pages/about/key-benefits.md Outdated
@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

Reviewers

@afeijoo afeijoo Awaiting requested review from afeijoo
2 more reviewers
@thisisdano thisisdano thisisdano approved these changes
@KMullan944 KMullan944 KMullan944 approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Publish System Benefits page

4 participants

@mejiaj @KMullan944 @thisisdano @katharine-carr