pascalduez / postcss-apply

PostCSS plugin enabling custom properties sets references
The Unlicense
164 stars 12 forks source link

Overriding custom property sets #10

Closed javivelasco closed 7 years ago

javivelasco commented 7 years ago

Hi! I have a proposal that I'm not sure if skips the standard.

With custom properties you can define kind of a variable in the :root {} scope and use it in a block. If there is another :root {} below the usage redefining the property the final output would keep that value.

/* define a component */
:root {
  --my-color: blue
}

.myClass {
  color: var(--my-color);
}

/* override the custom property */
:root {
  --my-color: red;
}

Outputs

.myClass { 
  color: red
}

I was wondering why can't we do the same with custom property sets. I think it's a desirable behavior to customize components. You can provide custom property sets for different nodes and then import the css at the top and redefine the default behavior.

Thanks!!

gharwood1 commented 7 years ago

+1, @javivelasco Is this causing a priority issue with react-css-themr? I think I just ran into this. I expected a nested theme to redefine properties set by react-toolbox, but ThemeProvider only seems to apply the new properties and not overwrite the ones already set.

Note: The properties I expect to be overwritten are getting applied to the browser, they are just not higher in priority.

javivelasco commented 7 years ago

@gharwood1 This issue is not directly related but I think it could be useful to solve some use cases of themr. By default themr combines theme objects and the final className respects the default but you can give a composeTheme={false}property and then it will just override the full object. If you want to override only the given classes composeTheme="softly".

This proposal would cover the case when you compose deeply to augment/override original styles of a particular node. You would give an @apply rule in your node classNames that could be defined later from the :root adding/overriding styles.

Feel free to open an issue or contact in Discord if you need to!

pascalduez commented 7 years ago

Hi,

I think the @apply plugin should follow the custom properties one closely. Also in a browser environment I guess redefining a set on :root should "propagate" to the block where it's used.

I'll look into it.

javivelasco commented 7 years ago

Good to know!! Thanks @pascalduez !

pascalduez commented 7 years ago

I'm currently implementing this. I ended up with some sort of "granular" override (merging declarations), but I'm really not confident this would be "spec" compilant.

So calling @tabatkins again for help :)

What would be the desired behaviour when re-defining a set.

Given the following input:

:root {
  --foo: {
    color: red;
    background-color: red;
  };
}

.before {
  @apply --foo;
}

:root {
  --foo: {
    color: blue;
  };
}

.after {
  @apply --foo;
}

Result

1 - Overrides all declarations.

.before {
  color: blue;
}

.after {
  color: blue;
}

2 - Overrides only selected declarations (merge).

.before {
  color: blue;
  background-color: red;
}

.after {
  color: blue;
  background-color: red;
}

EDIT: From the spec I guess it's more about result number 1 (full override).

Note that custom property sets override each other wholly, rather than cascading together like colliding style rules do.

tabatkins commented 7 years ago

Yes. Custom properties work the exact same regardless of how you're using them; they're just properties, and override each other.

(That said, we have a long-standing issue with CSS that you sometimes do want to merge with lower-specificity versions of a property, and custom property sets are definitely in that category. I may have written a draft for this that'll get proposed to the CSSWG at the big meeting next week...)

pascalduez commented 7 years ago

@tabatkins thanks a lot!

pascalduez commented 7 years ago

Fixed as of postcss-apply@0.4.0