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

CustomStyleService not working for css-mixin object #29

Closed
firstor opened this issue Jun 5, 2017 · 24 comments
Closed

CustomStyleService not working for css-mixin object #29

firstor opened this issue Jun 5, 2017 · 24 comments
Labels

Comments

@firstor
Copy link

firstor commented Jun 5, 2017

According to the Custom Style guide, CustomStyleService works fine on custom css variables, but it doesn't work for mixin objects.
Here is an example:

/* my-custom-ctrl.component.ts */
import { Component, OnInit } from '@angular/core';
import { CustomStyleService } from '@codebakery/origami';
...

@Component({
  selector: 'my-custom-ctrl',
  templateUrl: 'my-custom-ctrl.component.html',
    styleUrls: ['my-custom-ctrl.component.html.css']
})
export class MyCustomCtrl implements OnInit {
  constructor(private customStyle: CustomStyleService) { }

  ngOnInit() {
    this.customStyle.updateCustomStyles();
  }
 ...
}
/* my-custom-ctrl.component.css */
...
paper-textarea {
    --paper-input-container-input: {
        color: darkgreen;
        font-size: 20px;
    };
   --paper-input-container-color: red;
   @apply --some-other-styles-mixin;
} 
...

With CustomStyleService adopted, the component style is correctly wrapped with <custom-style> element, but paper-textarea gets compiled incorrectly. It gets transpiled in the following (captured by Chrome Inspector):

paper-textarea.create-message-input[_ngcontent-c11] {
    \--paper-input-container-input: {
        color: darkgreen;
        font-size: 20px;
    }
}

As given above, --paper-input-container-color and the next @apply disappeared, they appear and work correctly as our expectation once I comment out --paper-input-container-input.
However, if I move paper-textarea styles into the document-level styling, all work fine, this is somewhat weird.

Hopefully get help to solve this problem!

@hotforfeature
Copy link
Owner

Let me make sure I understand your problem. If you remove the --paper-input-container-input custom property mixin block from paper-textarea, the @apply --some-other-styles-mixin; works as intended?

My first thought is that backslash escape character that Angular is adding when it compiles a custom mixin property is breaking something somewhere along the line. I'd be interested to see if anything changes if you declare the mixin property at the end of the block.

While

div {
  --custom-property: red;
}

is valid CSS,

div {
  --custom-property: { color: red }
}

is technically not, since @apply and mixins aren't in the CSS spec. Angular parses component CSS, and that parser probably is misinterpreting what you're trying to do, since it isn't valid CSS.

It works when you declare it at the document level (in HTML or in external CSS files) because Angular isn't trying to parse those.

I wonder if Angular is only parsing CSS for the emulated view encapsulation, that [_ngcontent-c11] stuff. Do you get any different results if you use ViewEncapsulation.Native? (Don't forget to pass in the component's ElementRef to CustomStyleService this.customStyle.updateCustomStyles(this.elementRef) when using native shadow dom).

I'll dig into it and try to figure out what specifically is causing that problem. I have a feeling that it's going to end up being a bug ticket with the Angular compiler.

@firstor
Copy link
Author

firstor commented Jun 5, 2017

@hotforfeature Thank you for your response.
Yes, you picked up the point. If I remove the --paper-input-container-input block from paper-textarea, --paper-input-container-color variable and @apply rule work fine. And I tried with ViewEncapsulation.Native mode, but the \ would be kept same and custom styling still doesn't work.
However, I have a question: you said the backslash is added by Angular when it finds broken css during compilation, but the paper-textarea styling worked fine on Angular2.x + Polymer1.x, so could you explain about this? Do you think it's a bug with Angular 4 compiler?
Thank you in advance.

@hotforfeature
Copy link
Owner

Were you using https://github.com/platosha/angular-polymer when you tried this in Angular 2.x + Polymer 1.x?

This library handles custom styles quite differently. Specifically https://github.com/platosha/angular-polymer/blob/master/src/renderer/shared-custom-styles-host.ts#L29 is a good indicator of the difference. Instead of wrapping what Angular generates as a <style> element (CustomStyleService does this), it takes the raw text content before Angular creates the <style> element and applies it directly to a <custom-style> element.

Two different methodologies, both with their pros and cons. That SharedCustomStylesHost may be solving a problem I overlooked though.

I'm not 100% familiar with how Angular generates styles to know if that's where the backslash is coming from, or if it's somewhere else. This is just my first guess and where I'll start looking.

@firstor
Copy link
Author

firstor commented Jun 6, 2017

@hotforfeature Yes, I was using angular-polymer. I really appreciate your explanation.
By the way, I found another problem and I am wondering whether I have to create another issue.
To ask directly here, can I use CustomStyleService several times on several components? I used CustomStyleService in two places on 2 different components like as I gave in the first example. One component is going to be loaded several times, and all work fine on Chrome, but the app works strangely when an action is fired then the component is loaded multiple times on FireFox (v53.0.3 x64). Removing this.customStyle.updateCustomStyles(); from the component can recover the normal state. It's difficult for me to provide sample code with which someone can reproduce my case. If you can't catch my case, I may be able to share my screen to help you.

@hotforfeature
Copy link
Owner

I'm not sure I understand what state you're talking about, are styles not applying? Or is there some other application state that is breaking? If so, what isn't working? Do you get console error messages?

CustomStyleService only modifies the document head's styles, so without knowing more about what action your components are doing, it's hard to say if there'd be any conflicts going on. Do you get an error message in the console? How quickly are these new components created and destroyed?

You could try moving this.customStyle.updateCustomStyles(); to the constructor of your component instead of in ngOnInit if there may be something weird with lifecycles that your components are doing.

@firstor
Copy link
Author

firstor commented Jun 6, 2017

@hotforfeature Moving updateCustomStyles() to constructor doesn't make any change. I am curious to know why it's working so strangely on FireFox. Using updateCustomStyles() with FireFox seems to break other web components.

@hotforfeature
Copy link
Owner

If you have a sample repo or plunker highlighting the problem I can take a look at the issue.

@firstor
Copy link
Author

firstor commented Jun 6, 2017

@hotforfeature I am sorry but it's difficult for me to make a sample for you to use to reproduce my case. Would you like to check via my screen?

@hotforfeature
Copy link
Owner

Looks like webpack's css-loader is the culprit. I created a simple test webpack project.

index.js

require('./style.css')

style.css

html {
  --my-mixin: {
    color: blue;
  }
}

webpack.config.js

module.exports = {
  module: {
    loaders: [{
      test: /\.css$/,
      loader: 'css-loader'
    }]
  }
};

webpack ./index.js index.bundle.js compiles everything, and here's what css-loader adds in the bundle:

(function(module, exports, __webpack_require__) {

exports = module.exports = __webpack_require__(1)(undefined);
// imports


// module
exports.push([module.i, "html {\n  \\--my-mixin: {\n    color: blue;\n  }\n}\n", ""]);

// exports


/***/ })

The CSS @apply proposal is still very much a rough draft and in early stages of talks across browsers. I don't think we'd get much response or motivation filing a feature request to support it in css-loader, especially when you could use PostCSS in webpack and there's a PostCSS @apply plugin for it.

Nor do I think we'd get anywhere asking Angular to switch from the simple css-loader to a complex PostCSS setup.

Where do we go from here?

Well, I think the best thing we can do is intercept the styles before they are added to the dom as a script tag. angular-polymer may have the right idea in a specialized SharedStylesHost.

We could intercept the raw CSS strings and fix the escaped mixin blocks. The rest of the CSS data is there, it just breaks when put after the escaped mixin. We'd also be able directly create and manage the <custom-style> elements, rather than doing a global wrap. Another benefit is that we wouldn't have to call customStyle.updateSharedStyles() anymore.

I'll start working on a rough draft implementation. Thanks @First87 for finding this problem!

@firstor
Copy link
Author

firstor commented Jun 6, 2017

@hotforfeature No problem. I really appreciate your support. I will consider the possibility of interception. Thanks again.

@hotforfeature
Copy link
Owner

@First87 I think I may have discovered a possible workaround in my development and discovery. ShadyCSS's apply shim works by converting

paper-textarea {
    --paper-input-container-input: {
        color: darkgreen;
        font-size: 20px;
    };
} 

to

paper-textarea {
    --paper-input-container-input_-_color: darkgreen;
    --paper-input-container-input_-_font-size: 20px;
} 

You may be able to use mixins immediately by writing your mixin block in the "parsed" output that ShadyCSS is expecting.

As a side note update, my progress in hacking a fix is going well! I should have it within the next couple of days if work doesn't swamp me.

@firstor
Copy link
Author

firstor commented Jun 8, 2017

@hotforfeature It might be kind of a good solution. Btw, sometimes we need to define and use our own mixins. For example, I define a mixin object to apply word-wrapping styles to elements:

    --my-word-wrapping: {
        overflow-wrap: break-word;
        word-wrap: break-word;
        -ms-word-wrap: break-word;
        word-break: break-word;
        -ms-word-break: break-word;
        ...
    };

Then I use --my-word-wrapping in some places:

   .message-item .text {
        @apply --komed-word-wrapping;
        /* more styles here */
   }

I feel it's not reasonable to give parsed css to everywhere the custom mixin is used. Well, of course, I can replace --my-word-wrapping with a css class (e.g. apply-word-wrapping) and then give this css class to every element's class (but I am not still happy to use that since~).

It will be better to me if origami fully supports CSS mixin. :)

However, I found another problem in css-mixins with Angular4+Polymer2. The problem is that the document-level styling seems to be parsed differently on different browsers.
Here is an example of document-level styling:

/* index.html */
...
  <!-- custom-style with Polymer 2.x syntax -->
  <custom-style>
    <style>
      * {
        ...
      }
      :root {
        ...
      }
      body {
        ...
      }
      .app-def-btn {
         ...
      }
      ...
    <style>
  <custom-style>
...

On Google Chrome, all work as intended, however, when I inspect on FireFox and Edge, they are parsed differently as * converted to *:not(.style-scope), :root to html:not(.style-scope), body to body:not(.style-scope), and .app-def-btn to .app-def-btn:not(.style-scope). Since they're parsed with :not(.style-scope) suffix, the web page is displayed ugly on FF & Edge.

So I am wondering if it's possible to allow Polymer2's custom style (the document-level styling) and Angular's component-level stylings work together.
Hopefully get your idea of it.
Thank you.

@hotforfeature
Copy link
Owner

You can still define and use your own mixins with this workaround method.

Change

html {
  --my-word-wrapping: {
        overflow-wrap: break-word;
        word-wrap: break-word;
        -ms-word-wrap: break-word;
        word-break: break-word;
        -ms-word-break: break-word;
  }
}

to

html {
  --my-word-wrapping_-_overflow-wrap: break-word;
  --my-word-wrapping_-_word-wrap: break-word;
  --my-word-wrapping_-_-ms-word-wrap: break-word;
  --my-word-wrapping_-_word-break: break-word;
  --my-word-wrapping_-_-ms-word-break: break-word;
}

That is what ShadyCSS will convert your mixin to. The only problem (that's now fixed) is that Angular isn't allowing ShadyCSS to get to that point.

There's nothing about Origami that does or doesn't support mixins. Angular does not support CSS mixins. All I've done is added a hack to force Angular to accept them so that developers don't have to use the above workaround.

:not(.style-scope) is ShadyCSS, not Angular. Angular adds [_ng_content-x] attributes when using emulated view encapsulation. Additionally, you are using the :root selector in your index.html outside of the scope of an element. That's actually incorrect, and ShadyCSS is correctly converting that to html.

@firstor
Copy link
Author

firstor commented Jun 8, 2017

@hotforfeature Thank you for explaining.
I found a similar issue (Polymer/polymer#2639) reported on Polymer project. So I am wondering if :not(.style-scope) is a kind of Polymer bug? Or do you think what I miss?

@hotforfeature
Copy link
Owner

It is not a bug, it's how ShadyCSS works. That specific issue is actually about the class name .foo--bar not having :not(.style-scope) added to it when it should have.

Everything you mentioned in your comment is working as intended.

@firstor
Copy link
Author

firstor commented Jun 8, 2017

@hotforfeature Ok, thanks. Then is there any way to prevent ShadyCSS adding :not(.style-scope) at each style definition of custom-style in document-level? I am also curious to know why this doesn't happen on Chrome. On Google Chrome, everything works as intended without :not(.style-scope) inserted, but on FireFox and Edge.

@hotforfeature
Copy link
Owner

Chrome natively supports Shadow DOM while Firefox and Edge do not. Scoping isn't a feature you'd want to disable, it's very important in what ShadyCSS is doing. If something isn't styling properly, it's quite likely that your style rules are not using Shadow DOM correctly.

Take a look at https://github.com/webcomponents/shadycss#about-scopingshim for a deeper explanation into what ShadyCSS is doing.

@firstor
Copy link
Author

firstor commented Jun 8, 2017

@hotforfeature Got it. So basically Polymer's new custom-style and ShadeCSS aim to scope styles and prevent them leaking into the shadow trees of other components for the browsers with native support of Shadow DOM v1. Right?

However, I asked a way to remove style-scoping :not(.style-scope), since I have been upgrading Polymer 1.x to 2.0 and doc-level styles have been written just as a top-level of styles cascading down all decendent elements from Polymer 1.x. Thus I think the fastest way is to recover the app's appearance is to remove all :not(.style-scope)s from all style selectors. (I found that one reason of the issue was :not(.style-scope) added by ShadyCSS.)

So finally, in order to make app work fine on the browsers without native support of Shadow DOM, shall we style elements while caring for :not(.style-scope) at all time (if the app is written with non-Polymer components - e.g. Angular component)? What should be a good way in such case? Could you tell me what you recommend?

Many thanks for your support so far in a timely manner.

@hotforfeature
Copy link
Owner

This really isn't new, it's been a core concept about Shadow DOM: view encapsulation.

Previously, Polymer 1.x had ways to break through this encapsulation, such as /deep/ or ::shadow. This goes against the Shadow DOM spec.

Take the following markup:

<div class="blue">
  <my-element>
    #shadow-root
      <div class="red"></div>
  </my-element>
</div>

Shadow DOM v1 means there is no possible way for you to break my element's shadow root and style my div. This used to be possible with v0, but not anymore.

Say you have this document-level CSS:

my-element .red {
  background: blue;
}

ShadyCSS would turn this into

my-element:not(.style-scope) .red {
  background: blue;
}

On Chrome, if you removed :not(.style-scope), the element's background would be red, not blue. Why? Because the native Shadow DOM is preventing you from styling it. On Firefox, that HTML turns into this:

<div class="blue">
  <my-element class="style-scope">
    <div class="red"></div>
  </my-element>
</div>

We don't have a native Shadow DOM, so we're emulating it. Now if you removed :not(.style-scope), the element's background would be blue, even though the author intended it to be red. Firefox and Chrome would have different styling, not the same.

What you're wanting to do will break styling on browsers that do not support native Shadow DOM, not fix it. This isn't a feature to turn off or disable, this is Shadow DOM.

If something isn't styling properly because of Shadow DOM encapsulation and you're trying to fix it, you may be approaching document-wide styling the wrong way when using web components.

tl;dr;

We're talking very abstract though, and maybe there's just miscommunication. If you have a project repo or plunker you want to share highlighting the specific problem, I'd be happy to take a look at it. There may be a correct solution to your problem that doesn't involve trying to break through Shadow DOM.

@firstor
Copy link
Author

firstor commented Jun 10, 2017

@hotforfeature It seems that we're saying on different topics, it's not your fault. I think I have mixed up various things together and that must have confused you. I am sorry for that. Anyway I have been totally trying to mention scoping and styling with Angular and Polymer. I mentioned custom-style and ShadeCSS in the late comment, because :not(.style-scope) is not what I wanted.

Before I enter the topic, I'd like to make sure I have the same point of ShadowDOM v1 spec and support of it with you. (If something wrong, please correct me first.)

  • Shadow DOM lets a component placed in a scoped subtree and component styles are only applied to its own scope. So styles inside a shadow tree don't affect elements outside the tree and likewise styles outside the tree don't match selectors inside the tree. Style cascading doesn't work at all (but element still inherits some styling information like color that applies to its parent, so we can use CSS inheritance from the document-level styles: http://plnkr.co/edit/7ugStflqbexg2dNqmtDQ?p=preview)
  • Polymer 2 supports ShadowDOM, but Angular doesn't. And Chrome natively supports ShadowDOM v1 spec and FF and Edge don't. Polymer 2 adopts ShadyCSS that simulates ShadowDOM so that web components work correctly on non-Chrome browsers.

Let's come back to the main topic.
All you mentioned ShadowDOM and ShadyCSS itself are correct. But I am mentioning them in relation with Angular and Polymer. Let me explain with an example.

<!-- #eg1 -->
<my-component>
  #sharow-root
  <style>
    :host {...}
    .title {...}
    my-element-inner {...}
  </style>
  <div class="title">...</title>
  <my-element-inner>...</my-element-inner>
</my-component>

This is how my-component is rendered on Chrome - the browser with native support of ShadowDOM v1 spec. The component's style is placed in a scoped subtree and doesn't affect outside the parent of my-component as well as inside my-component-inner. But the component is going to work inproperly on non-Chrome browsers. To make it work properly on the other browsers, it's required to do some refactorings. So my-component will be rendered as below: (that's just how Polymer 2.0 components are rendered on non-Chrome browsers)

<!-- #eg2 -->
<head>
...
  <style scope="my-component">
    my-component {...}
    .title.my-component {...}
    my-element-inner.my-component {...}
  </style>
...
<head>
...
<my-component>
  <div class="title my-component">...</title>
  <my-element-inner class="my-component">...</my-element-inner>
</my-component>

You see there is no :not(.style-scope). I am NOT meaning ShadyCSS doesn't work correctly, it's used for inproper purpose. Why?

I think we should carefully focus on how Angular renders component and how custom-style is parsed by Polymer. Angular component is just a component. We define a component with component-level style and template and Angular applies scoping to them with its own way. We ignored that!

Let's say that my-component is an Angular component, then it should be rendered like as #eg2, but Angular doesn't. It applies its own scoping to the component's template and style and then attach <style> to the <head>. Since the rendered component is not compatible with ShadowDOM, ShadeCSS will process the component's template and style individually and that will break the relationship between two parts of my-component. That is the main problem!

Did you catch my idea?

I am sorry, but in my poor opinion, origami doesn't work properly as a bridge between Angular 4 and Polymer 2. And I am wondering if I should open a new issue for this ...

  • Regarding application of <custom-style> to Angular components' styles:
    I think component-level style should NOT be a custom-style theoretically, so origami shouldn't wrap all <style> elements with <custom-style> without any fair reason. Since origami is only used with Angular 4 and Polymer2, and, in order Angular component to be rendered like as #eg2, origami may adopt ShadeCSS to process component's template and style as a pair before they're attached to the document.
    I am not familiar with ShadeCSS, but as I saw from an imported Polymer component, it may be possible to do scoping template and style as a pair with custom scope (e.g. my-component).
    So is it possible to apply scoping with ShadeCSS
    a). to the component style with simply adding scope='component-selector' to component's <style> (that is, <style scope='my-component'>)?
    b). to the component template with custom scope coming from component selector?
    Need to get your feedback of possibility to use ShadeCSS for this purpose.

  • Regarding other external styles:
    As just mentioned, I think origami doesn't need to care for other styles though external styles are deprecated by Polymer. Once <custom-style> wraps the <style>s, ShadeCSS will parse them and generate unexpected result. As far as I can see by inspection on browser, Polymer components' styles are imported properly without <custom-style>. So let them go as they are. And if a <style> is required to handle as custom-style, it will be wrapped manually with <custom-style> by developer as Polymer upgrade guide says, then Polymer will handle it.

Please feel free to share your feedback of my poor thought.

Further I want to get your short guide to use <custom-style> since I don't have much knowledge of what <custom-style> does or what it is for. I think it's for a document-level styling to define styles for the purpose to apply CSS globally. (See also Polymer's Style Shadow DOM and an example)
Then is it fair to append :not(.style-scope) to all selectors in document-level styles?
Basically I want to implement architecture to share and apply some common/global styles with/to all elements (i.e. fonts, colors and some style mixins etc).

<!-- document-level styling -->
<!-- :not(.style-scope) is appended to all selectors here after ShadeCSS handles -->
<custom-style>
  <style>
    * {
      font-family: ...; /* globally used font */
      ...
    }
    body {
      margin: ...;
      line-height: ...;
      min-height: ...;
      background-color: #fbfbfb;
    }
    /* more global CSS and css variables and mixins that will be applied through all elements */
    html {
      --a-css-variable-for-Polymer-or-other-web-components: ...;
      --a-css-mixin-for-Polymer-or-other-web-components: {
        ...
      }
      --a-css-variable-for-custom-Angular-components: ...;
      --a-css-mixin-for-custom-Angular-components: {
        ...
      }
    }
    .def-btn {
      /* will apply for buttons like as <paper-button class="def-btn" ... */
    }
  </style>
</custom-style>

What is the best way to implement this architecture? Hope to get your recommendation for it.

Thank you for leaving feedback.

@hotforfeature
Copy link
Owner

You are still talking very much in the abstract. We could go back and forth for quite a while, but I don't think that is the best use of our time.

The TL;DR; response is that Origami registers all of Angular's styles with ShadyCSS (by wrapping them in <custom-style>, that's all that element does) to enable the polyfills for CSS custom properties and CSS mixins. Without that, you would be unable to use any CSS custom properties or mixins in Angular components.

Can you link your project's repository, or a sample repo that reproduces the problem? Once you have that, explain which styles are not working correctly on Chrome and Firefox, and what you expect the result to be.

I'll then go through it and see if I can figure out what the problem is.

@firstor
Copy link
Author

firstor commented Jun 10, 2017

@hotforfeature Excuse me, but could you read out the last comment? I am implying something there. Yes, I am talking in the abstract, but I wrote there what I think of origami and what I wish origami does. Thank you.

@hotforfeature
Copy link
Owner

@First87 I spent an hour drafting a full response before I realized that we were getting nowhere just by talking. We need to get our hands into code to see the problems. I did read your comment fully.

ShadyCSS and Angular's style scoping do not conflict with each other. I do not believe there would be any benefit in using ShadyCSS's scope attribute feature.

It is also impossible to use CSS custom properties and mixins in Angular without ShadyCSS. This is the reason I chose to wrap all styles with <custom-style>. The purpose of Origami is to offer seamless integration for Angular devs using Polymer.

When writing a Polymer component, you don't have to use <custom-style>. Similarly, you shouldn't have to use <custom-style> when writing an Angular component. The only time a developer should have to manually use that element is in the index.html, like the Polymer documentation suggests.

Angular components are the same as web components. They're emulated by default, but that is what they're modeled after.

If I have overlooked something, I'd be glad to fix it. However, I don't see from your samples any issue that would cause conflict. This is why I'm asking for a project. Once we have our hands on code we can run it in the browser and see where the gaps are.

@firstor
Copy link
Author

firstor commented Jun 13, 2017

@hotforfeature Sorry for no update for days. Just opened another issue and moved to it: #33.
Please review it and let me know if any questions.
I am leaving this issue now.
Many thanks for your support so far.

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

2 participants