Skip to content

feat: Place color-scheme before CSS properties - #586

Merged
drwpow merged 1 commit into
terrazzoapp:mainfrom
nlemoine:feat-scheme-first
Nov 28, 2025
Merged

drwpow merged 1 commit into
terrazzoapp:mainfrom
nlemoine:feat-scheme-first

Conversation

@nlemoine

Copy link
Copy Markdown
Contributor

Changes

Not much, it just place the color-scheme before CSS properties.

Although there's nothing wrong being at the end, it's usually used on top of CSS props. It also aligns better with the doc examples.

How to Review

Tiny change, nothing to worry about.

@changeset-bot

changeset-bot Bot commented Nov 15, 2025 •

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: bbc206e

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@nlemoine
nlemoine force-pushed the feat-scheme-first branch 2 times, most recently from 01a25c8 to 55a599d Compare November 15, 2025 15:16

@drwpow drwpow left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Great thank you!

@drwpow

drwpow commented Nov 16, 2025

Copy link
Copy Markdown
Collaborator

Only test failure is from snapshots. If you could update those with pnpm run test -u that’d be great

Comment thread packages/plugin-css/test/fixtures/type-border/want.css Outdated
Comment thread packages/plugin-css/test/fixtures/utility-css/want.css Outdated
@drwpow

drwpow commented Nov 20, 2025

Copy link
Copy Markdown
Collaborator

Update: you may want to change your approach somewhat. Before, we were sorting properties alphabetically. But with the addition of #589, we have to make sure that insert order is preserved. Really this is probably a signal we need to move away from an object, because objects should never try and preserve order, and for the most part they didn’t until #589.

Anyways, your approach can still work overall and I’m happy to merge this PR! But I think it’ll have to just not be a part of the .sort() method anymore since that’s removed.

@nlemoine

nlemoine commented Nov 21, 2025 •

Copy link
Copy Markdown
Contributor Author

@drwpow I rebased the PR and made the relevant changes to fit the new approach.

There is one test failing on macos but I don't think it's related to this PR.

Let me know if that looks good for you.

@drwpow drwpow left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Looks great, thank you! Agreed, the macos failure is unrelated. I’ll look into it separately. Thank you! 🙂

@drwpow
drwpow merged commit d25bfce into terrazzoapp:main Nov 28, 2025
4 of 5 checks passed
@github-actions github-actions Bot mentioned this pull request Nov 28, 2025
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