cryogen-project / cryogen-core

Cryogen's core
Eclipse Public License 1.0
69 stars 62 forks source link

Sass doesn't apply to themes' css/ directory #90

Closed KingMob closed 7 years ago

KingMob commented 7 years ago

I was a bit confused why the Sass files in my theme's css folder weren't compiling. The docs just say the Sass directory config defaults to css, and since there isn't one under resources/templates/ by default, I assumed it was referring to the css directory in the template instead.

I could compile Sass to CSS in the theme in advance, but I'd prefer to build everything at once, if possible. I also tried specifying the Sass paths to use public/blog/css, but Cryogen only looks under templates/ at the moment.

I'm happy to do this if you want a PR. Perhaps using :sass-theme-src and :sass-theme-dest to indicate Sass dirs relative to the theme? Or make :sass-src and :sass-dest accept multiple directories?

lacarmen commented 7 years ago

Hmm I didn't think about this when I added the theming support... I think it would make sense to make the currently existing config keys accept multiple directories. A PR would be great!

KingMob commented 7 years ago

Getting into the thick of it, and I had a question. How much do you want to avoid breaking changes?

In particular, there's an undocumented effect of how sass is currently called. The configuration doc says you need to add the css directory to the :resources list, but the way sass is called, it works even if you forget to do that. Likewise, even though :sass-src is set to css by default, and the css directory doesn't exist by default, it doesn't currently produce an error. (For any other resource directory, not existing is an error.) Should I fix that, if people might be relying on it, or preserve that behavior?

Mostly, my aim is to separate compiling Sass files from copying them to the public directory, as they're currently combined in a way that makes it hard to add more dirs. The idea is to compile all Sass files first, then copy the resources over.

lacarmen commented 7 years ago

I'd rather clear up all the inconsistencies than avoid breaking changes. If your approach feels intuitive then I'd say go for it.

Mostly, my aim is to separate compiling Sass files from copying them to the public directory, as they're currently combined in a way that makes it hard to add more dirs. The idea is to compile all Sass files first, then copy the resources over.

I think this will definitely clear up any confusion in the future.

KingMob commented 7 years ago

In that case, to preserve the current behavior, how about I leave the default :sass-src as css, but add a css directory to the main cryogen repo, so it works correctly by default. Plus, that'll show people where to place non-theme Sass/CSS anyway.

lacarmen commented 7 years ago

Sounds good to me.