Skip to content
This repository has been archived by the owner on Feb 22, 2018. It is now read-only.

Allow using multiple css files in NgComponent #315

Closed

Conversation

mvuksano
Copy link
Contributor

@mvuksano mvuksano commented Dec 3, 2013

Addresses issue #313

This pull request adds support for specifying css files as a list. This allows for better separation of css styles across files as demonstrated in https://github.com/markovuksanovic/angular.dart.playground

This pull request changes synax of NgComponent a bit:

@NgComponent( selector: 'my-widget',
    templateUrl: 'packages/helloworld/mywidget/mywidget.html',
    cssUrl: const ['packages/helloworld/base/base.css',  'packages/helloworld/mywidget/mywidget.css']
    )
class MyWidget {

}

This way base.css can be included in multiple NgComponents, as in AwesomeWidget in the demo mentioned above.

@ghost ghost assigned mhevery Dec 9, 2013
@mhevery
Copy link
Contributor

mhevery commented Dec 10, 2013

Thanks for your contribution! In order for us to be able to accept it, we ask you to sign our CLA (contributor's license agreement). CLA is important for us to be able to avoid legal troubles down the road.

For individuals (a simple click-through form): http://code.google.com/legal/individual-cla-v1.0.html For corporations (print, sign and scan+email, fax or mail): http://code.google.com/legal/corporate-cla-v1.0.html

@mvuksano
Copy link
Contributor Author

@mhevery - I signed the CLA electronically. Please let me know if there's anything else I need to do.

@mhevery
Copy link
Contributor

mhevery commented Dec 13, 2013

I have spent some time thinking about it, and I am wondering why do we need this feature. Is @import not sufficient?

https://developer.mozilla.org/en-US/docs/Web/CSS/@import

@mvuksano
Copy link
Contributor Author

The reasoning was that @import results in additional HTTP request. From
performance point of view it's better to combine these by the tools rather
then let users (the ones who use the app) pay the price.

Also I assume most of these files will be small, usually a few bytes, end
establishing a new TCP connection to get them will also introduce a penalty
due to three way handshake. This doesn't apply if the TCP connection is
reused.

P.S. I actually thought about using @import initially but read somewhere that it wouldn't work correctly with shadowDom. Apparently @import can also work.

@mvuksano
Copy link
Contributor Author

Ok, @mhevery you got me thinking on this one, so I did some tests to figure out if @import can work.

The answer is, yes it can work and the result is very similar (both approaches will still fetch all the css files), but...

If @import is used, stylesheets are fetched after load event is fired.
screen shot 2013-12-13 at 5 05 26 pm

If css are combined, all css files are first fetched and load event is fired afterwards.
screen shot 2013-12-13 at 5 06 04 pm

The net effect is that with @import sometimes people will see a flicker (I've noticed it when testing) while combined css result in a smooth rendering.

@mhevery
Copy link
Contributor

mhevery commented Dec 13, 2013

Sold, MERGED

@mhevery mhevery closed this Dec 13, 2013
@mvuksano mvuksano deleted the feature/multiple-css-files branch March 26, 2014 02:19
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Development

Successfully merging this pull request may close these issues.

None yet

2 participants