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

Sprinkles condition not overidden #1469

Open
2 tasks done
manuelbosi opened this issue Aug 29, 2024 · 3 comments
Open
2 tasks done

Sprinkles condition not overidden #1469

manuelbosi opened this issue Aug 29, 2024 · 3 comments
Labels
recipes Issue related to the recipes package sprinkles Issue related to the sprinkles package

Comments

@manuelbosi
Copy link

manuelbosi commented Aug 29, 2024

Describe the bug

Basically my sprinkles defines color conditions for light, dark mode and for both also define states: lightHover, darkHover etc..

Inside my button I used data attributes to style element based on values that came from react-aria useButton hook, and each compound variant style key is result of calling sprinkles function.

{
  variants: {
    variant: "filled",
    color: "indigo",
  },
  /* APPLYING STYLE LIKE THIS WORKS */
  // style: {
  //   color: colors.white,
  //   background: colors.indigo6,
  //   borderColor: colors.indigo6,
  //   selectors: {
  //     [DATA_HOVERED]: {
  //       background: colors.indigo8,
  //       borderColor: colors.indigo8,
  //     },
  //     [DATA_PRESSED]: {
  //       background: colors.indigo7,
  //       borderColor: colors.indigo7,
  //     },
  //   },
  // },
  /* APPLYING STYLE LIKE THIS DOESN'T WORK */
  style: sprinkles({
    background: {
      dark: "indigo6",
      darkHover: "indigo8",
      darkPressed: "indigo7", // this will be always overridden by darkHover
    },
    color: "white",
    borderColor: {
      dark: "indigo6",
      darkHover: "indigo8",
      darkPressed: "indigo7", // this will be always overridden by darkHover
    },
  }),
}

These class are generated in this order in dev mode:
Button__pwy3ag3 --> base button style
sprinkles_background_indigo6_dark__28ssfrzv --> base button background
sprinkles_background_indigo8_darkHover__28ssfr10o --> base button background on hover
sprinkles_background_indigo7_darkPressed__28ssfr10e --> base button on press
sprinkles_color_white_dark__28ssfrjj --> base button text color
sprinkles_borderColor_indigo6_dark__28ssfr1d3 --> base button border color
sprinkles_borderColor_indigo8_darkHover__28ssfr1dw --> base button border color on hover
sprinkles_borderColor_indigo7_darkPressed__28ssfr1dm --> base button border on pressed

This is what happen
image

Is using sprinkles like that a bad way?

EDIT: I tried to assign a complete different color like red and seems working with sprinkles function, but I have no idea why in this case works.
image

Reproduction

https://codesandbox.io/p/sandbox/vanilla-extract-specificity-8g2m3r

System Info

System:
    OS: Linux 6.6 Debian GNU/Linux 12 (bookworm) 12 (bookworm)
    CPU: (12) arm64 unknown
    Memory: 6.28 GB / 7.66 GB
    Container: Yes
    Shell: 5.2.15 - /bin/bash
  Binaries:
    Node: 20.12.1 - /usr/local/bin/node
    Yarn: 1.22.22 - /usr/local/bin/yarn
    npm: 10.5.0 - /usr/local/bin/npm
    pnpm: 9.7.1 - /usr/local/bin/pnpm
  npmPackages:
    @vanilla-extract/css: 1.15.3 => 1.15.3
    @vanilla-extract/dynamic: 2.1.2 => 2.1.2
    @vanilla-extract/recipes: 0.5.5 => 0.5.5
    @vanilla-extract/sprinkles: 1.6.3 => 1.6.3
    @vanilla-extract/vite-plugin: 4.0.13 => 4.0.13
    vite: 5.4.0 => 5.4.0

Used Package Manager

pnpm

Logs

No response

Validations

@manuelbosi manuelbosi changed the title Sprinkles condition Sprinkles condition not overidden Aug 29, 2024
@askoufis
Copy link
Contributor

You're using a darker colour for hover than for pressed, so when both are applied, the darker colour wins because it's generated later in the stylesheet. This is because of the order sprinkles generates classnames, which is in the order the tokens are provided.

So the reason it works with a red colour is because the red tokens are defined after the indigo tokens.

Using a regular style with selectors works because those styles are generated by recipe, in the order they're defined, so you're not bound by the order of sprinkles classnames.

You could try a combination of sprinkles and regular selectors if you want to maximise sprinkles usage. The this only works because you happen to be using the same background and border colour:

style:
  [
    sprinkles({
      background: {
        dark: "indigo6",
      },
      color: "white",
      borderColor: {
        dark: "indigo6",
      },
    }),
    {
      selectors: {
        [DATA_HOVERED]: {
          background: colors.indigo8,
          borderColor: colors.indigo8,
        },
        [DATA_PRESSED]: {
          background: colors.indigo7,
          borderColor: colors.indigo7,
        },
      },
    },
  ],

Specificity issues are quite common with recipes and sprinkles, and there are existing discussions around their interoperability. I don't think we've put in explicit effort to make them work better together. I wonder if simply iterating over sprinkles properties different would address this?

@askoufis askoufis added sprinkles Issue related to the sprinkles package recipes Issue related to the recipes package and removed pending triage labels Aug 31, 2024
@manuelbosi
Copy link
Author

You're using a darker colour for hover than for pressed, so when both are applied, the darker colour wins because it's generated later in the stylesheet. This is because of the order sprinkles generates classnames, which is in the order the tokens are provided.

So the reason it works with a red colour is because the red tokens are defined after the indigo tokens.

Using a regular style with selectors works because those styles are generated by recipe, in the order they're defined, so you're not bound by the order of sprinkles classnames.

You could try a combination of sprinkles and regular selectors if you want to maximise sprinkles usage. The this only works because you happen to be using the same background and border colour:

style:
  [
    sprinkles({
      background: {
        dark: "indigo6",
      },
      color: "white",
      borderColor: {
        dark: "indigo6",
      },
    }),
    {
      selectors: {
        [DATA_HOVERED]: {
          background: colors.indigo8,
          borderColor: colors.indigo8,
        },
        [DATA_PRESSED]: {
          background: colors.indigo7,
          borderColor: colors.indigo7,
        },
      },
    },
  ],

Specificity issues are quite common with recipes and sprinkles, and there are existing discussions around their interoperability. I don't think we've put in explicit effort to make them work better together. I wonder if simply iterating over sprinkles properties different would address this?

Thanks for your reply @askoufis, now it's clear why using sprinkles like that create some strange behaviors.

Your solution could works but using that approach we mix simple styles with sprinkles and honestly add some complexity while reading and mantaining styles.
Also with my sprinkles I can define light and dark mode colors.

After some testing, I think using something like this seems much cleaner and readable, but yes is longer than previous.

{
  variants: {
    variant: "filled",
    color: "indigo",
  },
  style: style({
    "@media": {
      "(prefers-color-scheme: dark)": {
        color: vars.colors.white,
        background: vars.colors.indigo9,
        borderColor: vars.colors.indigo9,
        selectors: {
          [DATA_HOVERED]: {
            background: vars.colors.indigo8,
            borderColor: vars.colors.indigo8,
          },
          [DATA_PRESSED]: {
            background: vars.colors.indigo7,
            borderColor: vars.colors.indigo7,
          },
        },
      },
      "(prefers-color-scheme: light)": {
        color: vars.colors.white,
        background: vars.colors.indigo9,
        borderColor: vars.colors.indigo9,
        selectors: {
          [DATA_HOVERED]: {
            background: vars.colors.indigo8,
            borderColor: vars.colors.indigo8,
          },
          [DATA_PRESSED]: {
            background: vars.colors.indigo7,
            borderColor: vars.colors.indigo7,
          },
        },
      },
    },
  }),
}

Depending on the project needs (for example the application has a theme toggle) we could adopt another solution, not handling different media using prefers-color-scheme but define two different themes for light and dark mode using createTheme and change some class a root level to change custom properties values.

What do you think about the solution above compared to different theme class with tokens value for light and dark mode?

@askoufis
Copy link
Contributor

askoufis commented Sep 3, 2024

Your solution would work, and is very flexible, but it is quite verbose.

Leaning in to CSS variables more could help. Using semantic vars instead of putting your entire color palette inside vars would mean you could define two themes at build time, one for dark and one for light. For example, these themes would each set vars.borderColor.focus to a value from your palette (not a var), which would then be used by your button for its focus style.

This separates your palette from your semantic tokens, and allows you to swap to darkmode by just changing a single classname, rather than defining a media query for every style.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
recipes Issue related to the recipes package sprinkles Issue related to the sprinkles package
Projects
None yet
Development

No branches or pull requests

2 participants