USWDS-Site: Combine "What's new" and "News and updates pages #2958
Conversation
amyleadem
changed the title
| - /whats-new/updates/ | ||
| - /about/updates/ |
There was a problem hiding this comment.
Note
These were redirects from the deleted updates.md file
| {% elsif page.collection and include.page.subnav.type == page.collection %} | ||
| {% assign _current = true %} | ||
| <!-- | ||
| Use the page's `type` data key to connect the sidenav to a collection. | ||
| This sets "current" for the pages inside the _posts directory. | ||
| Collections are defined in config.yml, and "posts" is a default collection type in Jekyll. | ||
| --> | ||
| {% elsif page.collection and include.page.type == page.collection %} | ||
| {% assign _current = true %} |
There was a problem hiding this comment.
Note
This change allows us to set the "current" page in the sidenav of all our posts to be the "All news and updates" page.
Notes:
- To connect the pages, we needed to also add
type: poststo the front matter of the What's new pages. - The
typedata key is used in the front matter across the site, buttype: postsis not yet used in the repo.
Why I didn't use the existing subnav.type check
Currently, the site connects the posts to the "News and updates" page in the side navigation by using the conditional in lines 7-8 here, along with adding subnav.type = "posts" on the updates.md file.
However, I ran into some errors with this approach when I tried to add both a type and a link item to "All news and updates" inside the subnav in pages/whats-new/overview.md. It caused conflict that caused the side navigation to break (Please confirm that you are not able to get it to work either).
Needs discussion
Proposed additional change
I recommend we remove the subnav.type check in lines 7-8 and replace it with the check added in lines 14-15.
Currently, there is only one other page that uses subnav.type: security.md. Replacing subnav.type with type here should still connect it to the expected collection in config.yml.
I don't see anything in the repo relating to page.type that would cause any regressions, but of course we would want to test that. If we move forward with this approach, we would also need to make an update to the front matter in security.md.
Curious to hear thoughts on this approach and proposed additional change.
| {% elsif page.collection and include.page.subnav.type == page.collection %} | |
| {% assign _current = true %} | |
| <!-- | |
| Use the page's `type` data key to connect the sidenav to a collection. | |
| This sets "current" for the pages inside the _posts directory. | |
| Collections are defined in config.yml, and "posts" is a default collection type in Jekyll. | |
| --> | |
| {% elsif page.collection and include.page.type == page.collection %} | |
| {% assign _current = true %} | |
| <!-- | |
| Use the page's `type` data key to connect the sidenav to a collection. | |
| This sets "current" for the pages inside the _posts directory. | |
| Collections are defined in config.yml, and "posts" is a default collection type in Jekyll. | |
| --> | |
| {% elsif page.collection and include.page.type == page.collection %} | |
| {% assign _current = true %} |
This comment was marked as duplicate.
This comment was marked as duplicate.
Sorry, something went wrong.
There was a problem hiding this comment.
This is somewhere I was also getting caught up while testing. I think it would be a good idea to normalize this across pages. It would also reduce complexity of this current page logic which would be a win!
Praise: Thank you for breaking this down in a clear and easy to follow way!
There was a problem hiding this comment.
Thanks for consolidating this and the notes. The changes look good and the reasoning to consolidate make a lot of sense.
Added a question and a comment. Ideally, we should move away from the pattern of embedding chunks of HTML in markdown. The larger the chunk the harder it is to update/maintain and could introduce code duplication.
| {% elsif page.collection and include.page.subnav.type == page.collection %} | ||
| {% assign _current = true %} | ||
| <!-- | ||
| Use the page's `type` data key to connect the sidenav to a collection. | ||
| This sets "current" for the pages inside the _posts directory. | ||
| Collections are defined in config.yml, and "posts" is a default collection type in Jekyll. | ||
| --> | ||
| {% elsif page.collection and include.page.type == page.collection %} | ||
| {% assign _current = true %} |
This comment was marked as duplicate.
This comment was marked as duplicate.
Sorry, something went wrong.
| <ul class="usa-card-group margin-top-6"> | ||
| <li class="usa-card site-component-card desktop:grid-col-6"> | ||
| <div class="usa-card__container"> | ||
| <header class="usa-card__header"> | ||
| <h2 class="usa-card__heading font-lang-lg">Release notes</h2> | ||
| </header> | ||
| <div class="usa-card__body font-lang-sm"> | ||
| <p>Find summaries of each USWDS update on our GitHub releases page. Release notes explain bug fixes, new features, and other changes.</p> | ||
| </div> | ||
| <div class="usa-card__footer"> | ||
| <a class="usa-button" href="https://github.com/uswds/uswds/releases">Read the USWDS release notes</a> | ||
| </div> | ||
| </div> | ||
| </li> | ||
| <li class="usa-card site-component-card desktop:grid-col-6"> | ||
| <div class="usa-card__container"> | ||
| <header class="usa-card__header"> | ||
| <h2 class="usa-card__heading font-lang-lg">GitHub discussions</h2> | ||
| </header> | ||
| <div class="usa-card__body font-lang-sm"> | ||
| <p>How our growing community of government engineers, content specialists, and designers participate and contribute to improving USWDS.</p> | ||
| </div> | ||
| <div class="usa-card__footer"> | ||
| <a class="usa-button" href="https://github.com/uswds/uswds/discussions">USWDS GitHub discussions</a> | ||
| </div> | ||
| </div> | ||
| </li> | ||
| </ul> |
There was a problem hiding this comment.
issue: Can we create a component or an include for this?
Ideally a component should be reusable, but having HTML in MD can be confusing and hard to maintain.
| <!-- | ||
| Use the page's `type` data key to connect the sidenav to a collection. | ||
| This sets "current" for the pages inside the _posts directory. | ||
| Collections are defined in config.yml, and "posts" is a default collection type in Jekyll. | ||
| --> |
There was a problem hiding this comment.
polish: This could be a liquid comment.
There was a problem hiding this comment.
Barring other potential changes, this is looking good! I left one non-blocking recommendation for code consistency.
- Confirm links work
- Confirm pages display as expected at all screen widths
- Confirm changelogs are present and accurate
- Confirm sidenav highlights the correct current page on the what's new pages and posts
- Confirm redirects work as expected
- Confirm the previous link to "News and updates" (/about/updates/) now points to the "What's new" page
- Confirm the permalinks and url directory structure makes sense for the new pages and posts
- Confirm no regressions in sidenav on different page types.
- Confirm the code meets USWDS standards (pending other requested changes)
| {% elsif page.collection and include.page.subnav.type == page.collection %} | ||
| {% assign _current = true %} | ||
| <!-- | ||
| Use the page's `type` data key to connect the sidenav to a collection. | ||
| This sets "current" for the pages inside the _posts directory. | ||
| Collections are defined in config.yml, and "posts" is a default collection type in Jekyll. | ||
| --> | ||
| {% elsif page.collection and include.page.type == page.collection %} | ||
| {% assign _current = true %} |
There was a problem hiding this comment.
This is somewhere I was also getting caught up while testing. I think it would be a good idea to normalize this across pages. It would also reduce complexity of this current page logic which would be a win!
Praise: Thank you for breaking this down in a clear and easy to follow way!
| --- | ||
| title: All news and updates | ||
| layout: post | ||
| permalink: /about/whats-new/all/ | ||
| category: About | ||
| type: posts | ||
| --- | ||
|
|
||
| {% capture lead %} | ||
| Here's the full listing of all USWDS product updates, articles, case studies and more. | ||
| {% endcapture %} | ||
| {% include lead.html text=lead %} |
There was a problem hiding this comment.
It looks like this is the only page setting and including the lead in this way (the rest are done in layout.
Suggestion (non-blocking): Since this page isn't a post, it looks like we can use the styleguide layout and set the lead in the front matter like similar pages.
| --- | |
| title: All news and updates | |
| layout: post | |
| permalink: /about/whats-new/all/ | |
| category: About | |
| type: posts | |
| --- | |
| {% capture lead %} | |
| Here's the full listing of all USWDS product updates, articles, case studies and more. | |
| {% endcapture %} | |
| {% include lead.html text=lead %} | |
| --- | |
| title: All news and updates | |
| layout: styleguide | |
| permalink: /about/whats-new/all/ | |
| category: About | |
| lead: Here's the full listing of all USWDS product updates, articles, case studies and more. | |
| type: posts | |
| --- |
Summary
Combined "What's new" and "News and updates" pages.
This is a follow-up to the prototype work in #2855.
Note
This PR does not include the prototype's updates to the excerpt link structure in
post.html. It was not clear how to accommodate external links in the header links, so I bumped it for now to prevent this from being blocked. We are going to address this in a fast follow-on in #2953Related issue
Closes #2915
Preview links
Problem statement
Solution
Testing and review
All:
Content:
Devs: