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: deduplicate palette colors #39

Closed
wants to merge 1 commit into from

Conversation

Skagoo
Copy link

@Skagoo Skagoo commented Jan 11, 2023

This refactor takes care of color hex code duplication between _catppuccin.scss and _<flavour>.scss.

Specific flavours get their palette from _catppuccin.scss and use this to create their color variables.

Feedback is welcome, happy to revisit if required! 🐱

@nekowinston
Copy link
Contributor

Thanks for your PR!

The original idea was to have the flavour files separately, in case people only needed one/a specific set of flavours.
I can see what you're doing, but I guess the documentation would have to be updated, telling people to include e.g. _catppuccin.scss AND _frappe.scss in their project.

@Skagoo
Copy link
Author

Skagoo commented Jan 19, 2023

Ok, that's indeed an obvious overhead I missed. Not sure if it is even worth solving it like this then.

@nekowinston
Copy link
Contributor

Yeah, requiring people to use two files for what's supposed to be the more minimalist usage is pretty counter-intuitive.

I'd happily accept changes to the PR generating those SCSS files, or might do it myself. De-duplicating where possible is always welcome. 🙂

Copy link
Contributor

@nekowinston nekowinston left a comment

Choose a reason for hiding this comment

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

Just marking this as reviewed, see my PR comment.

@Skagoo
Copy link
Author

Skagoo commented Jan 19, 2023

Okay so a generator that spits out the scss palettes would be cool? I'll edit the PR somewhere this weekend.

@nekowinston
Copy link
Contributor

Yeah, if possible similarly to #36, which changed our generator scripts

@Skagoo
Copy link
Author

Skagoo commented Jan 19, 2023

Okay, would like the different color formats as well?

@nekowinston
Copy link
Contributor

I don't think it's needed since the Sass preprocessor offers the functions to change opacity, darken/lighten, regardless of the color format.

If I'm wrongly informed on that, and there would be some benefit, then probably yes.

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