Skip to content
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

Merged
merged 18 commits into from
Jun 11, 2024
Merged

Refactor data #3865

merged 18 commits into from
Jun 11, 2024

Conversation

beechnut
Copy link
Contributor

Pull request summary

Overall, this PR:

  • Refactors data into clearer organization, and fixes data references in all templates
  • Removes unused layouts, partials, and data
  • Adds tags collection and updates references to tags data
  • Changes nearly all links from Jekyll format ({{ site.baseurl }}) to 11ty format ({{ "path" | url }}). Images and some navigation links remain.
  • Updates site metadata
  • Cleans up some code, formatting, logic

Closes #3854 and #3859

- 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.
@beechnut beechnut requested a review from a team as a code owner June 11, 2024 15:22
@cantsin
Copy link
Member

cantsin commented Jun 11, 2024

We get some Unexpected unnamed function linter errors -- possible to disable this in the linter? (I think these are valid uses of anonymous functions)

Copy link
Member

@cantsin cantsin left a 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.

_data/guides.yml Show resolved Hide resolved
_includes/layouts/default.html Show resolved Hide resolved
// 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?
Copy link
Member

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?

Copy link
Contributor Author

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

@beechnut
Copy link
Contributor Author

@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.

@beechnut beechnut merged commit d9484ad into replatform-main Jun 11, 2024
4 of 7 checks passed
@beechnut beechnut deleted the replatform/data branch June 11, 2024 17:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants