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

overriding the custom css variables only works for the first dom element on the page #1752

Closed
kasperpeulen opened this issue Jun 4, 2015 · 7 comments
Assignees
Labels

Comments

@kasperpeulen
Copy link

tldr: http://plnkr.co/edit/PxEdsV?p=preview

I was playing around different material design themes, and it is not completely clear to me in which way I can override the custom css style variables.

For example, let's say that I would have two themes:

<style is="custom-style">
  .amber {
    --dark-primary-color: #FFA000;
    --default-primary-color: #FFC107;
    --light-primary-color: #FFECB3;
    --text-primary-color: #212121;
    --accent-color: #FFC107;
    --primary-text-color: #212121;
    --secondary-text-color: #727272;
    --divider-color: #B6B6B6;
  }

  .blue {
    --dark-primary-color: #1976D2;
    --default-primary-color: #2196F3;
    --light-primary-color: #BBDEFB;
    --text-primary-color: #FFFFFF;
    --accent-color: #448AFF;
    --primary-text-color: #212121;
    --secondary-text-color: #727272;
    --divider-color: #B6B6B6;
  }
</style>

Now if a dom element has such a class, and if it contains paper-elements, I would think that those paper-elements would drop their default styling and instead uses the variables defined in the class:

<paper-sample-app class="amber"></paper-sample-app>
<paper-sample-app class="blue"></paper-sample-app>

This does happen for the first dom element on the page. But the second dom element, actually get the styling of the first dom element. So, that is really strange, I would think it would maybe do nothing, and it gets the default styling, but the second element inherits the custom styling of the first dom element...

see: http://plnkr.co/edit/PxEdsV?p=preview

If I do this workaround, it works as I would expect:

<dom-module id="paper-sample-app">
  <style>
    :host {
      /* I would think that this lines shouldn't be necesarry
         as this is the default value for --paper-toolbar-background */
      --paper-toolbar-background: var(--default-primary-color);
    }
  </style>

If I have a custom element with lots of different paper-elements, it feels bit redundant if I would have to define all the default styling of the paper elements, again, especially, as it is already somewhere defined.

@sorvell
Copy link
Contributor

sorvell commented Jun 5, 2015

Thx for the detailed report. There are a couple of issues here but we have a line on tracking them and the fix is pretty simple. The elements will need to be updated because the way they are specifying defaults was relying on incorrect behavior.

@sorvell sorvell added p1 and removed p2 labels Jun 5, 2015
@sorvell
Copy link
Contributor

sorvell commented Jun 5, 2015

Beginning of fix available here: https://github.com/Polymer/polymer/tree/fix-1752.

Requires some testing before it can be merged. Elements will need to update the way the specify default values.

@sorvell
Copy link
Contributor

sorvell commented Jun 5, 2015

For reference, custom properties to not inherit values from below them in the tree. See the following in FF: http://jsbin.com/sepuwunifi/1/edit

@kasperpeulen
Copy link
Author

There are a couple of issues here but we have a line on tracking them and the fix is pretty simple. The elements will need to be updated because the way they are specifying defaults was relying on incorrect behavior.

great to hear it will get fixed !

For reference, custom properties to not inherit values from below them in the tree. See the following in FF: http://jsbin.com/sepuwunifi/1/edit

ah, okay, makes sense. I needed to change body to :root to make your snippet work though:
http://plnkr.co/edit/CJJNCo?p=preview

@alex88
Copy link

alex88 commented Jun 5, 2015

Could this issue be relative also to my case where element's defined variables for the element with a custom class doesn't work?

http://plnkr.co/edit/JgFOh8hIxp2aisOecg1O?p=preview

Update: Nope, my issue is that :host(.theme-default) selector is not implemented yet

@sorvell
Copy link
Contributor

sorvell commented Jun 8, 2015

For :host(selector) see this tracking issue: #1787

@alex88
Copy link

alex88 commented Jun 8, 2015

In my experience I've also found that sometimes generated css has only the rules with custom variables inside, like in this element's css:

<style>
::content .somediv {
  background: blue;
  color: var(--my-element-color);
}
</style>

only the color rule is generated while the background is skipped, I haven't found a reproducible test case but maybe it could be related to this sone since the first element in the page that declares similar css has all the values set correctly.

Update: issue is that element doesn't have a <template> tag inside it so rules aren't generated.
Demo: http://plnkr.co/edit/NLRjdMOTgLevWNlzMLuL

Maybe it's worth another issue or is it "per-design"?

sorvell pushed a commit that referenced this issue Jun 16, 2015
…ents are styled. Fixes #1752 where an element that uses only values supplied by variable defaults can be incorrectly styled.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants