We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for the commit author(s). If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.

CLA is because I forgot to setup my email when I did the commit. Should be fixed on the next rebase.

@jakemac53 could you review? or is someone else more appropriate?

A few remarks:

• Do we also need to support styles as an input ? ie the user provided some style in the annotation - this is supported when transformers are not used
• Fetching the styleUrls is one thing but then you need:
  • to resolve the urls in the css (see StyleUrlResolver),
  • to inline @import rules, ie recursively inline imported CSS (see StyleInliner)

#2566 should be closed only when all that is implemented.

Thanks.

Sorry yes I was reading through the issue too fast and was missing some of the requirements. This will only support CSS inlining.

• styles is supported as an input. Any attributes not specifically handled are passed through in the attribute. So they are there I just didn't need to do any processing for this.
• StyleUrlResolver should be pretty simple. I think we just need to stop swallowing the templateUrl when we write out the annotation. I don't know if angular is going to favor template over templateUrl. Maybe we can just set moduleId in the transformer and use the same system TS will be using. I wouldn't want to rewrite all the CSS inline because it doesn't allow an application to specify another StyleUrlResolver if they wish.
• I didn't know we were going to still support inlined @import rules and styleUrls. Probably that should be a separate transformer that can take any html file and inline the style tags. @jakemac53 might already have something that was done for polymer.

styles is supported as an input. Any attributes not specifically handled are passed through in the attribute. So they are there I just didn't need to do any processing for this.

You need to resolve urls, inline imports in user provided styles

StyleUrlResolver should be pretty simple. I think we just need to stop swallowing the templateUrl when we write out the annotation. I don't know if angular is going to favor template over templateUrl. Maybe we can just set moduleId in the transformer and use the same system TS will be using. I wouldn't want to rewrite all the CSS inline because it doesn't allow an application to specify another StyleUrlResolver if they wish.

transformers code must inline templateUrl to template

I didn't know we were going to still support inlined @import rules and styleUrls. Probably that should be a separate transformer that can take any html file and inline the style tags. @jakemac53 might already have something that was done for polymer.

The code already exists in the project. It should be possible to call it the transformers.

it doesn't allow an application to specify another StyleUrlResolver if they wish

Good point, there should be an option to pass a custom UrlResolver to the transformer (I don't think there is a need to pass a custom **Style**UrlResolver.

/cc @mhevery

Good point, there should be an option to pass a custom UrlResolver to the transformer (I don't think there is a need to pass a custom StyleUrlResolver.

Unfortunately for users this would require them to write their own transformer (it can't be done through a simple option in the pubspec). We probably do need to support it though regardless, but will need to create a good wiki page for how to set it up.

I didn't know we were going to still support inlined @import rules and styleUrls. Probably that should be a separate transformer that can take any html file and inline the style tags. @jakemac53 might already have something that was done for polymer.

The code already exists in the project. It should be possible to call it the transformers.

It seems reasonable to just drop support for this and remove the code from the project entirely? What benefit do we get by supporting both?

@vicb talked to Misko about this. I'm not closing #2566 but that work is not going to be part of this pull request. We need this internally so going to get it as part of the next release.

Will make another pull request supporting baseUrl, but we are going to leave it as a Runtime step for now. Also the inlining of styles from the html page is low priority for us so we leave that for another time too.

merged

It seems reasonable to just drop support for this and remove the code from the project entirely? What benefit do we get by supporting both?

If you are referring to dropping support for @import, I don't think this would be reasonable: it is a css standard and 3rd party libs might use this. Also note that @import rules could be handy as they support adding a media query.

I'm not closing #2566 but that work is not going to be part of this pull request. We need this internally so going to get it as part of the next release.

It has been closed by merging the PR which I think is reasonable. Should we create a new issue to track remaining work ?

Sounds reasonable created: #2643, #2646. Feel free to add any other features you think are needed.

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}