-
Notifications
You must be signed in to change notification settings - Fork 148
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 - Layout Grid: Guidance refactor #2276
Conversation
| <h2 id="responsive-variants">Responsive variants</h2> | ||
|
|
||
| <div> | ||
| <table class="usa-table--borderless site-table-responsive site-table-simple"> | ||
| <caption>Default responsive sizes</caption> | ||
| <thead> | ||
| <tr> | ||
| <th scope="col" class="flex-2">Width</th> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved under the Responsive Variants guidance on line 517
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mahoneycm Thanks for taking this on! I added some comments and suggestions below. Also, can you add a changelog to the page?
Let me know if you want to talk through the comments!
|
😀
Cathlene S Baptista, Contractor
Bixal
Technology Transformation Services
U.S. General Services Administration
703-402-3096 | ***@***.***
Contact the Digital.gov team: ***@***.***
…On Fri, Jul 12, 2024 at 5:37 PM Charlie Mahoney ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In _utilities/layout-grid.md
<#2276 (comment)>:
> + <td data-title="Example" class="display-inline-flex flex-2">
+ <span>
+ @include grid-offset(4)
+ </span>
+ </td>
+ </tr>
+ </tbody>
+ </table>
+ {% include utilities/utility-mixin-using.html %}
+</section>
+
+<section id="advanced-settings" class="padding-top-4">
+ <h2 class="site-h2 margin-y-0">Advanced settings</h2>
+
+ {% include utilities/responsive-variants.html %}
+ No settings available.
Created #2748 <#2748> to track.
I tagged it with good first issue because I thought it might be a good
one for @cathybaptista <https://github.com/cathybaptista> or
@RachelCorsino <https://github.com/RachelCorsino> !
—
Reply to this email directly, view it on GitHub
<#2276 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/BGR7KQGQINHPYW43OULH5OTZMBECXAVCNFSM6AAAAAA4WYVTISVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDCNZVG44DANBZHA>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mahoneycm thanks for these updates.
Two minor issues
- SCSS example mixins are missing semicolons.
- Values in mixin table don't match up.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the updates.
Build error is coming from HTMLProofer.
Details
#!/bin/bash -eo pipefail
npm run proof
> proof
> bundle exec htmlproofer --enforce-https=false --allow-missing-href=true --ignore-status-codes 0,403,429,503,302 --swap-urls 'https\://designsystem.digital.gov/:/' --ignore-urls '/github.com/uswds/uswds/issues/' --ignore-files '/whats-new/updates/2017/,/whats-new/updates/2018/,/documentation/code-guidelines/' ./_site
Running 3 checks (Scripts, Links, Images) in ["./_site"] on *.html files...
Checking 1219 external links
Checking 917 internal links
Checking internal link hashes in 225 files
Ran on 356 files!
For the Links > External check, the following failures were found:
* At ./_site/documentation/showcase/index.html:2714:
External link https://commerce.gov/ failed: got a time out (response code 301) (status code 301)|
@mejiaj I pulled in the latest from For the Links > External check, the following failures were found:
* At ./_site/design-tokens/color/overview/index.html:3107:
External link https://w3c.github.io/wcag/coga/gap-analysis.html failed (status code 404)
HTML-Proofer found 1 failure!
Exited with code exit status 1Created #2753 to address |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mahoneycm Thanks for your patience with this review. I added a number of small suggestions and tweaks. I am hoping these are all that remains to fix.
Users can use the order design token page for guidance
There was a problem hiding this 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! Thanks @mahoneycm
Summary
Refactored layout grid guidance to visually match other utility pages. The markdown now uses settings tables, includes mixin guidance, and now features the copy code button on code blocks.
Related issue
Closes #2210
Closes #2611
Preview link
Layout Grid guidance →
Problem statement
The layout grid guidance doesn’t match other utility guidance pages. Rather than using our site
includes, it uses highlight code blocks to display spacing settings and mixin examples.Solution
Utilize template includes to insert formatted tables and improve reader experience.
Warning
This PR removes the example at the bottom of the page which displayed how to use the mxins to create a two-column layout with a gap.
We should discuss if we would like to keep this example, refactor it, or keep it removed.
Testing and review
Testing checklist