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

chore: more fully remove assetgraph-builder and canvas #5175

Merged
merged 5 commits into from
Aug 3, 2024

Conversation

JoshuaKGoldberg
Copy link
Member

@JoshuaKGoldberg JoshuaKGoldberg commented Jul 11, 2024

PR Checklist

Overview

This is a superset of #5069 as reverted by #5095. Replaces the existing assetgraph-builder + canvas system in two ways:

  • Instead of using assetgraph-builder to optimize the site's assets, resizes larger images to how they're used now and runs npx imagemin-cli on all the .pngs
    • Result: does away with the docs/_dist directory, and instead has the docs/_site be the final output
  • Instead of using canvas to determine image positioning in their fixed size parent DOM elements, uses CSS

Long-term I'd want us to move to a more modern docs site builder, with or without a reorganization to multiple pages. But this works in the interim I think.

@JoshuaKGoldberg JoshuaKGoldberg changed the title Again remove canvas chore: more fully remove assetgraph-builder and canvas Jul 11, 2024
@JoshuaKGoldberg JoshuaKGoldberg requested a review from a team July 11, 2024 01:30
@JoshuaKGoldberg JoshuaKGoldberg marked this pull request as ready for review July 15, 2024 16:15
Copy link
Member

@voxpelli voxpelli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👏👏👏 I like it!

Would want to merge this before continuing with the last parts of #5128, which is moving out the docs from nps

@voxpelli
Copy link
Member

voxpelli commented Aug 2, 2024

Scratch previous comment – but we need to adapt either this one to the other one or the reverse

We should merge this prior to #5071 at least though

@JoshuaKGoldberg
Copy link
Member Author

If you'll pardon my reach, this one makes it annoying for me to review other PRs, so I'm going to sneak it in first. 😄

@JoshuaKGoldberg JoshuaKGoldberg merged commit 1883c41 into mochajs:main Aug 3, 2024
20 checks passed
@JoshuaKGoldberg JoshuaKGoldberg deleted the again-remove-canvas branch August 3, 2024 13:21
@coveralls
Copy link

Coverage Status

coverage: 94.523%. remained the same
when pulling d059d41 on JoshuaKGoldberg:again-remove-canvas
into a8f8c8b on mochajs:main.

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.

🛠 Repo: Unable to install on ARM Mac (node-pre-gyp and gyp failures)
3 participants