-
Notifications
You must be signed in to change notification settings - Fork 65
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
Removes obsolete or redundant guides #966
Conversation
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.
lgtm!
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.
LGTM
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.
@mark-meyer and/or @jfredrickson I removed this filter because we don't need it for the limited guides we're keeping. Could one of you review this? I suspect there's more to this change than the markup.
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.
I left a couple comments in the layout/guides/list.html
file. I'm happy to make this small change if you want me to.
layouts/guides/list.html
Outdated
@@ -7,24 +7,7 @@ <h1>GSA Tech Guides</h1> | |||
{{ .Content }} | |||
|
|||
<div class="grid-row grid-gap"> |
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.
Hey @brentryanjohnson — I think we can delete the grid divs altogether since we no longer have columns to worry about. So deleting this div and the one below, which will promote the one <div class="guides-category" data-category="{{ .Key | urlize }}">
to the top level.
layouts/guides/list.html
Outdated
</div> | ||
</div> | ||
|
||
|
||
<div class="tablet:grid-col-9"> | ||
{{ range .Pages.GroupByParam "category" }} |
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.
I think it's fine to keep this even thought there is only one category…but just pointing out that if we'll never have anything other than Agile, we could simplify this too.
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.
This looks good to me!
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.
LGTM
Fixes #884