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

use "inline styles" and "styles urls" #13

Closed
wants to merge 3 commits into from
Closed

use "inline styles" and "styles urls" #13

wants to merge 3 commits into from

Conversation

rdlabo
Copy link

@rdlabo rdlabo commented Apr 14, 2016

now pages's css is called from "theme/app.core.scss".
But this can other style pollution. And Angular2 @component have function prevent (and Ionic2 @page too).
I think starter-package had better use this.

This request is pair "update watch ''app/pages/*/.scss" #9"( ionic-team/ionic-gulp-tasks#9 ).

I think starter-package using many functions is good.

thanks.

@adamdbradley
Copy link
Contributor

I think this is really great, and we're absolutely interested in pursuing this, but we'd like to do some more testing using this method first.

Each css file will be a new ajax request, and each css update is going to insert the new css into the head tag, which will cause an entire app repaint. On desktop I'm sure this works great, but my concern is on slow android, so this requires some real world tests on real world apps first.

Additionally, because it's a tooling change we'll have to make sure everything can update smoothly, and it's documented well so developers understand how everything is coming together.

What's great is that this way is still possible now on any ionic2 app since the sass has been modular from the beginning. So for what it's worth, we're absolutely for going this route, but we'd like to do more testing to ensure we keep up performance and thoroughly review how tooling would work around this.

Thanks

@rdlabo
Copy link
Author

rdlabo commented Apr 28, 2016

I agree this method need testing. thanks!

@brandyscarney
Copy link
Member

Thanks for the PR! Sorry for not taking a look at this sooner. There have been a lot of changes to the process since this so there are some merge conflicts now. This should work with css files, but still something we need to look into more.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants