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

Programmatically generate variants #45

Merged
merged 5 commits into from
Dec 10, 2024

Conversation

mxdvl
Copy link
Contributor

@mxdvl mxdvl commented Nov 20, 2024

Why are we doing this?

So that manual changes to the SVG only have to be made in css.svg.

Closes #31

What does this change?

Parse the source SVG as a document, and update the relevant elements with new content for the following variants, which are then written back to the repo as SVG:

  • css.light.svg
  • css.dark.svg
  • css.square.svg

Everything is visually identical, as is confirmed by the lack of diff in the image files.

- light
- dark
- square

parsing the source SVG as a document and updating
the relevant elements and their content.
assets.mjs Outdated
title.innerHTML = "CSS Logo Square";
description.innerHTML =
"A purple square with the letters CSS inside in white";
bg.setAttribute("d", "M0,0H1000V1000H-1000Z");
Copy link
Contributor Author

@mxdvl mxdvl Nov 20, 2024

Choose a reason for hiding this comment

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

The alternative would be to remove the path element and replace it with a rect as is currently the case, but it would save very few bytes and would be much more complex.

Instead we draw a full-width square by:

  1. M0,0 moving to the origin point
  2. H1000 drawing a 1000 horizontal line to the right edge
  3. V1000 drawing a 1000 vertical line to the bottom edge
  4. H0 drawing a horizontal line back to the left edge
  5. Z closing the rectangle (optional)

css.svg Outdated Show resolved Hide resolved
@rol4nd909
Copy link
Contributor

Love this approach, but I think #35 should be merged first ;)

@mxdvl mxdvl requested a review from rol4nd909 November 21, 2024 11:44
Copy link
Contributor

@rol4nd909 rol4nd909 left a comment

Choose a reason for hiding this comment

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

Nice work done here. 2 small things:

  • I think css.square.svg, css.dark.svg and css.light.svg can beremoved?
  • And I made I small update to them because the shape of the C was broken. Fixed the shape of the C #47

@mxdvl
Copy link
Contributor Author

mxdvl commented Nov 21, 2024

  • I think css.square.svg, css.dark.svg and css.light.svg can be removed?

Don't you want to make these assets available for users? Happy either way it's a small change to produce a byte buffer than can be consumed by sharp: new TextEncoder().encode(svg.toString())

Alternatively if we keep the files we could have a test that ensures they remain all in sync during CI. Let me know what you'd prefer.

Fair enough, I'll endeavour to merge these changes soon.

@rol4nd909
Copy link
Contributor

I thought that those SVG's where rendered by the script. But if not then they have to stay 😊

Sorry for the misunderstanding.

@mxdvl
Copy link
Contributor Author

mxdvl commented Nov 22, 2024

I thought that those SVG's where rendered by the script. But if not then they have to stay 😊

They are indeed generated/overwritten by the script currently, but I thought we would want to expose them anyway?

Maybe the following diagram helps understand the process a bit better…

flowchart TD
    css.svg("fa:fa-draw-square css.svg")
    
    css.svg --> linkedom --> css.light.svg & css.dark.svg & css.square.svg
    css.svg --> sharp

    linkedom[assets.mjs - linkedom]

    css.light.svg & css.dark.svg & css.square.svg --> sharp

    sharp[assets.mjs - sharp]
    
    sharp --> css.png & css.ico & css.avif & css.webp
Loading

I tried to follow the principles of #39 (comment) and keep the canonical “source” as the css.svg document.

I think the worse that can happen with the current setup is that someone makes manual changes to one of the variants SVGs (light, dark or square) and it gets overwritten when they run assets.mjs. Maybe we should update the docs to make this clearer? I’m not entirely sure how we should phrase this but I could have a go.

Copy link
Contributor

@rol4nd909 rol4nd909 left a comment

Choose a reason for hiding this comment

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

I really appreciate your work and I think it's a big improvement maybe @argyleink or @Que-tin can also give there thoughts.

@mxdvl mxdvl requested a review from rol4nd909 November 26, 2024 15:18
@mxdvl
Copy link
Contributor Author

mxdvl commented Dec 10, 2024

Feel free to merge this whenever suits you 😁

@argyleink argyleink merged commit a31b1b7 into CSS-Next:main Dec 10, 2024
@mxdvl mxdvl deleted the mxdvl/generate-variants branch December 10, 2024 19:46
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.

changing fill programatically when generating assets
3 participants