-
Notifications
You must be signed in to change notification settings - Fork 200
USWDS-Site: Enhance what's new page layout #3115
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
base: al-whats-new-directory-change
Are you sure you want to change the base?
Changes from 17 commits
175a065
5924b46
6b0ff00
e4f32f4
9bb0485
7eb1062
98d834a
e0ce487
e7af702
5397251
6b3b66a
bde2472
3e85a9e
542f519
9b5dda3
a6010e4
aa2bc2f
97aff1a
0e261c1
35c5eb4
d698d30
84d5637
9961107
5d18a6c
333d592
f5b91d3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,25 @@ | ||||||
| <!-- Turn on smooth scroll animation for anchor links --> | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. issue: Can you add a comment with a brief description and where this is used? Are there any settings related to this component?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added a comment in 84d5637 |
||||||
| <style> | ||||||
| html { | ||||||
| scroll-behavior: smooth; | ||||||
| } | ||||||
|
|
||||||
| @media screen and (prefers-reduced-motion: reduce) { | ||||||
| html { | ||||||
| scroll-behavior: auto; | ||||||
| } | ||||||
| } | ||||||
| </style> | ||||||
|
|
||||||
| <ul class="site-checklist-jump-links"> | ||||||
| {% for link in page.jump_links %} | ||||||
| <li> | ||||||
| <a href="#{{ link.href | default: link.text | downcase | slugify }}"> | ||||||
|
||||||
| <a href="#{{ link.href | default: link.text | downcase | slugify }}"> | |
| <a href="#{{ link.href | default: link.text | slugify }}"> |
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.
More polish: You could improve readability by saving this in a variable.
{% for link in page.jump_links %}
{% assign link_href = link.href | default: link.text | slugify %}
<li>
<a href="#{{ link_href }}">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.
Made these updates in 84d5637
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.
thought: If link.text is always available this is fine, but without checking for it you could run into issues.
For example
If it isn't passed you'll still have a list item, link, and icon but it'll be empty and the href will be broken (unless link.href exists).
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.
It feels to me like link.text should be mandatory here. Any objection to leaving it as is?
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -16,11 +16,13 @@ | |
|
|
||
| <article id="{{ post_id }}" class="post-preview"> | ||
| <header class="post-preview__header"> | ||
| <p class="post-category site-subheading"> | ||
| {{ post.tags[0] }} | ||
| </p> | ||
| {% if include.category %} | ||
| <p class="post-category site-subheading"> | ||
| {{ post.tags[0] }} | ||
| </p> | ||
| {% endif %} | ||
|
Comment on lines
+19
to
+23
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Note On the main what's new page, I removed the tag eyebrow to conserve vertical real estate. I added it back in on the "All news and updates" page. Flag if you would like this changed. |
||
|
|
||
| <{{ include.heading | default:"h2" }} class="post-preview__title"> | ||
| <{{ include.heading | default:"h2" }} class="post-preview__title {{ include.headingClasses }}"> | ||
|
||
| {% unless preview_url == "none" %} | ||
| <a href="{{ preview_url }}"> | ||
| {% endunless %} | ||
|
|
@@ -30,12 +32,12 @@ | |
| {% endunless %} | ||
| </{{ include.heading | default:"h2" }}> | ||
|
|
||
| {% include post-meta.html %} | ||
| {% if include.date %} | ||
| {% include post-meta.html %} | ||
| {% endif %} | ||
|
Comment on lines
+35
to
+37
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Note On the main what's new page, I removed the date to conserve vertical real estate. I added it back in on the "All news and updates" page. Flag if you would like this changed. |
||
| </header> | ||
|
|
||
| <div class="post-preview__content site-prose"> | ||
| <p> | ||
| {{ post.lead | default: post.excerpt | markdownify }} | ||
| </p> | ||
| <p>{{ post.lead | default: post.excerpt | markdownify }}</p> | ||
| </div> | ||
| </article> | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. thought: Something for next time. This component could benefit from using For example: <!-- Set in template and passed to include. -->
{% capture card_heading %}
<div>A subheading</div>
<h3>The heading</h3>
{% endcapture %}
<!-- Component re-uses `card_heading` -->
<header class="usa-card__header">
{{ include.card_heading }}
</header> |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,20 +1,40 @@ | ||
| {% comment %} | ||
| Single card element for `site-card-list`. | ||
|
|
||
| Variables: | ||
| - heading: String heading for a single card. | ||
| - headingElement: The heading HTML element type. | ||
| - subheading: String subheading for a single card. | ||
| - subheadingElement: The subheading HTML element type. | ||
| - body: String body that gets placed in a paragraph tag. | ||
| - image: The card image. | ||
| - imageAlt: The alt tag for the image. | ||
| - linkUrl: String for the link url. | ||
| - linkText: String link label. | ||
| - linkClasses: Classes for the link element. | ||
|
||
| {% endcomment %} | ||
| <div class="usa-card__container"> | ||
|
|
||
| <div class="usa-card__container {{ include.containerClasses }}"> | ||
| <header class="usa-card__header"> | ||
| <h2 class="usa-card__heading font-lang-lg">{{ include.heading }}</h2> | ||
| {% if include.subheading %} | ||
| <{{ include.subheadingElement | default:"h2" }} class="site-subheading">{{ include.subheading }}</{{ include.subheadingElement | default:"h2" }}> | ||
| {% endif %} | ||
| <{{ include.headingElement | default:"h3" }} class="usa-card__heading font-lang-lg">{{ include.heading }}</{{ include.headingElement | default:"h3" }}> | ||
amyleadem marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| </header> | ||
| {% if include.image %} | ||
| <div class="usa-card__media"> | ||
| <div class="usa-card__img"> | ||
| <img | ||
| src="{{ include.image }}" | ||
| alt="{{ include.imageAlt }}" | ||
| /> | ||
| </div> | ||
| </div> | ||
| {% endif %} | ||
| <div class="usa-card__body font-lang-sm"> | ||
| <p>{{ include.body }}</p> | ||
| </div> | ||
| <div class="usa-card__footer"> | ||
| <a class="usa-button site-button" href="{{ include.linkUrl }}">{{ include.linkText }}</a> | ||
| <a class="usa-button site-button {{ include.linkClasses }}" href="{{ include.linkUrl }}">{{ include.linkText }}</a> | ||
| </div> | ||
| </div> | ||
This file was deleted.
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.
issue (non-blocking): We already have a jump links component in
_includes/accessibility-tests/jump-links.html.We should avoid repeating ourselves in the future and create a more flexible jump links component that can be reused. This would allow you to re-use the reduced motion styles and avoid setting them in multiple places, like
accessibility-tests.html.