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

Recursive flatten color and backgroundImage #2549

Conversation

lbennett-stacki
Copy link

@lbennett-stacki lbennett-stacki commented Oct 13, 2020

Recursively flattenColorPalette and use to flatten backgroundImage utility declarations.

Docs: tailwindlabs/tailwindcss.com#562

@lbennett-stacki lbennett-stacki changed the title Recursive flatten color bg image Recursive flatten color and backgroundImage Oct 13, 2020
@lbennett-stacki
Copy link
Author

I've opened a docs PR at tailwindlabs/tailwindcss.com#562

@snack-0verflow
Copy link

@LukeeeeBennett This feature looks really neat! Looking forward to using nested colors

@adamwathan
Copy link
Member

Hey thanks! Can you please write up a clear and detailed description of what this does and include it in the PR description? I keep coming to this and when I see that I have to read through all the code to figure out what this PR is for I just keep pushing it off to work on things I can get through more quickly.

@adamwathan
Copy link
Member

Never mind read the tests and understand it. I am working on making the whole config behave more consistently so I think we need to think through if this should apply to every single utility as just standard behavior or if it should really be unique to colors and background images.

I'm also exploring supporting deep merging of colors when using "extend" and have to think through how this might interact with that idea 🤔

@lbennett-stacki
Copy link
Author

lbennett-stacki commented Oct 17, 2020

Hey thanks! Can you please write up a clear and detailed description of what this does and include it in the PR description? I keep coming to this and when I see that I have to read through all the code to figure out what this PR is for I just keep pushing it off to work on things I can get through more quickly.

Sorry @adamwathan, very lazy of me.

This allows for infinitly deeply nested color palettes using recursion. A good example to check is on the docs PR, you can see a preview here https://tailwindcss-com-git-recursive-flatten-color-bg-image.tailwindlabs.vercel.app/docs/customizing-colors#nested-object-syntax. See how we now have a dot path for theme.colors.indigo.dark.default which outputs *-indigo-dark-default classes, which was not previously possible.

I then also use the new deep flattenColorPalette to parse deeply nested backgroundImage declarations. You can see the docs preview here https://tailwindcss-com-git-recursive-flatten-color-bg-image.tailwindlabs.vercel.app/docs/background-image#background-images where we have the now parsable dot path theme.extend.backgroundImage.brand.logo.default which is used to output bg-brand-logo-default classes.

This is useful to me building a product that has a slightly more complex design system that utilizes hierarchy to organize/define its palettes. This is used to theme the same (and other) UI product dynamically in multiple ways whilst providing styling configuration to some end users.

One really quick example is dark/light theming. (ignoring tailwind dark mode features/plugin)

Instead of

colors: {
    'primary-dark': { default: '...' },
    // ... *-dark
    'primary-light': { default: '...' },
   /// ... *-light
},

we can

colors: {
    dark: {
        primary: { default: '...' }
        // ...
    },
    light: {
        primary: { light: '...' }
        // ...
    },
}

@lbennett-stacki
Copy link
Author

lbennett-stacki commented Oct 17, 2020

Never mind read the tests and understand it. I am working on making the whole config behave more consistently so I think we need to think through if this should apply to every single utility as just standard behavior or if it should really be unique to colors and background images.

I'm also exploring supporting deep merging of colors when using "extend" and have to think through how this might interact with that idea 🤔

Agreed. Ideally, this is applied across all plugins that read the config. As suggested this seemed a little overwhelming for me so I decided to tackle the cases most important to me. Happy to help move this across plugins to make sure unit tests are available, and then we/you can look at a more holistic approach to refactor flattenColorPalette as part of the parsing path for every possible config item.

@adamwathan
Copy link
Member

I realized that a PR for this (aside from the background image stuff) already exists here:

#2148

So since that PR was first I'm going to close this one sorry 😞 I think for now I would like to contain the scope of this recursive flattening stuff to just colors, and we can explore adding it elsewhere if there's enough demand for it.

@adamwathan adamwathan closed this Oct 25, 2020
@lbennett-stacki
Copy link
Author

Sure, seems a little odd to me and have the coverage so will just use my fork from now, thanks!

Should close the docs PR too.

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.

None yet

3 participants