-
Notifications
You must be signed in to change notification settings - Fork 311
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
Refactor data #3865
Refactor data #3865
Conversation
- Consolidates partner agency data - Removes _data/featured_agencies.yml - Adds `featured` boolean to _data/agencies.yml - Fixes HHS full name so it's findable - Simplifies partner agency logo info, from paths -> filenames. - Renames collections.{collection name} from singular to plural, e.g. to collections.posts - Replaces site.baseurl for simple links. Image sources and breadcrumbs were left as-is for later. - Adds back featured projects/agencies to homepage. - Improves footer of blog post show page - Simplifies featured-posts partial, to limit entries by default - Adds back the next/prev blog post links - RISKY: Removes the reference to styles.css. It doesn't seem to have affected styles, since the built styles are loaded elsewhere, but now link checks are passing. - Adds `npm run precommit` command to run a full rebuild and run html and link tests Todos: - It would be nice to have an {% agency_logo %} shortcode, so we don't have the path to the agency logo folder as literals in multiple places.
- Changes `lang` parameter to `locale` - Sets locale in multiple templates (was only set in default, but hardcoded HTML lang was in base) - Adds locale data to our one Spanish language post - Changes key `ga` to `google-analytics` so code readers have better context to understand that key
This commit: - Makes the site-alert partial re-usable - Renames and improves data organization of "usa anchor" (sub-footer) data - Prevents a bug that would result from too many guides being set to promoted: true
This commit: - Adds a tags collection - Adds a slugify package - not the same one 11ty uses, because that package is incompatible with CommonJS inclusion, and we'd have to switch everything to ESM. - Removes the unused tagList collection - Moves all collection creation functions into config/collections.js - Creates tag pages - Fixes tag index page, and changes it from a layout to a page - Refactors featured topics/tags data - now it looks up the tag name from the slug, instead of hardcoding a duplicate name
This commit: - Removes the data file with featured services, now that they're in _data/featured.yml - Removes the `in_groups` filter. While this filter was invoked in one template, the results weren't actually being used, so we removed it everywhere. - Refactored the way partner agencies are displayed. Incidentally, this also removes the invocation of weighted_sort. I'll ensure there's an issue to make sure we're not losing whatever it was we lost by removing it.
There was an issue where asset names were getting truncated by a one character, but it was caused by extra unnecessary files getting ingested into the asset build. This commit: - Adds to buildAssets.js an error throw, when the conditions for the truncation exist. - Removes the file that has been the main culprit — a README.md in the blog assets which 11ty thought was a .md file to be rendered into HTML. Fixes #3859
This commit: - Removes unused layouts and partials - Reviews all layouts and partials for correct data - Removes `include.` references, not part of 11ty - Removes defunct .about.yml data - Minor tweaks to code spacing, layout, indentation While working on this, I noticed some duplicate includes, like header is basically the same as usa-banner. I think that's why we don't have a menu: I suspect the `header` file in Jekyll was the top menu, but got overwritten by the "this is a .gov site" banner, which is called usa-banner in Jekyll but header in 18F/guides.
We get some |
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, it builds. would like to see the linting errors fixed but that's not strictly necessary.
// However, that library requires ESM and this project is CommonJS. | ||
// The best thing to do would be to import the slugify filter, but I'm not quite | ||
// sure how to do that from this separate file — perhaps dependency injection | ||
// in .eleventy.js? |
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.
Make a ticket so we can reference/look at later?
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.
Done, added "look at all todos" to the epic
@cantsin I added "ignore" directives to the lines causing the eslint warnings. I also added a task to post-migration tasks to remind us to double-check/un-ignore these eventually. |
Pull request summary
Overall, this PR:
tags
collection and updates references to tags data{{ site.baseurl }}
) to 11ty format ({{ "path" | url }}
). Images and some navigation links remain.Closes #3854 and #3859