Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # 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.

# for free to join this conversation on GitHub. Already have an account? # 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