-
Notifications
You must be signed in to change notification settings - Fork 1.4k
issue(docs): Resolve issues with extending seed to sass #746
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
Comments
Definitely makes sense! We can list them here, as well as in the docs. |
@mgechev - Do you happen to remember what version of the seed those docs were based off of? I'd like to roll my own fork back to that version, and then use the update process to write out a proposal for the validation / notification workflow. |
We can compare with the wiki's revision history, not sure if all the revisions were actually valid. |
Fair enough. I'll start from scratch and just build it myself. |
@mgechev @ludohenin - This is going to require a change to either the testing scripts or the dev / prod build. Without modifying the seed you can either get the tests working or you can get the sass working. Question is, which route do you guys prefer we go? |
What are the changes that need to be performed? Change of the testing output directory? |
Get the sass working and modify the test I suppose but don't really see the implications from there. |
The crux of the problem right now is related to the components.
It's not a major change either way but before I make a change to the seed portion that will have side effects for others, i'm curious which way we want to go with it. |
@ludohenin @mgechev - So I had a thought. I've used sass/scss for quite a while & I love some of the functionality. Variables, mixins, partials, imports and so on. Issues
We already use postcss in the seed as part of our build process ( autoprefixer / cssnano ). Extending the existing postcss implementation to allow people to pick and chose the "sass-like" features they want without getting the baggage that comes with it should be far easier to support and more beneficial to the community as a whole. Opinions? |
Not sure, I don't do much css. But if I understand correctly, it's either sass or postcss but not both. If such is the case I'm fine if we ship with postcss by default. In the other and it would be nice to be able to swap that part of the build pipeline "easily". |
It might also be good to wait for offline template compiler (not sure it's there in beta.15), it's going to change the general picture. |
Postcss and sass can live in the same place. Any implementation of sass ontop of the seed is using both now.
This is being built using the project convention, I wasn't intending and trying to merge this into the seed itself. That said, the seed master is already using postcss which in essence is just an API to add plugins for stylesheet related functionality. Autoprefixer & cssnano are both postcss.
Using a function in the So after all the additions required in the wiki page for scss and then adding the styleUrl function to process the scss in the components, you end up taking a ~ 3 second performance hit. I guess what i'm saying is though I initially though it would be a good idea to validate merges against a sass enabled fork of the seed, all of the effort is really producing negative value for the maintainers as well as the people who would use it. I would think it a better solution overall to provide people with the functionality they need with a simplified implementation and faster execution. |
Short version, the sass/scss steps work but the styles aren't applied.
I'll deal with getting it working / correcting the docs.
Question: Does it make sense to pick a few very common configurations and then maintain them.
ex - sass/scss : bootstrap : material2 ( "supported extensions" from here down )
People already maintain these as projects but there is no merge validation against them so we end up inadvertently breaking common extensions to the project.
It may be enough to add validating supported extensions to our merge process and note a breaking change then create an issue to investigate the fix / update docs in either the seed or extension project.
//cc @mgechev @ludohenin
The text was updated successfully, but these errors were encountered: