factor1 / prelude-wp

Prelude is a WordPress starter theme that helps you craft custom themes.
GNU General Public License v3.0
14 stars 3 forks source link

CSS Task Proposal #94

Closed bebaps closed 7 years ago

bebaps commented 7 years ago

The primary gulp task for CSS is styles, which calls minify-css, which calls sass. The end result is theme.css, theme.css.map, and theme.min.css. The theme enqueues only theme.min.css though, meaning theme.css and theme.css.map are kinda pointless by default unless you change the theme to enqueue theme.css. Also, the minified CSS does not have a generated source map, which makes sense for a production environment but during development there is no benefit because any inspected styles are all on line 1 of theme.min.css.

Proposal: remove the styles task to keep the sass and minify-css tasks separate. Then change the theme to enqueue theme.css by default, and in gulpfile.js set sass in the default gulp task. This way the theme is set up for more of a development workflow from the beginning, and the developer has the option to optimize the CSS when they are ready to (and thus enqueue the minified file). Also, more of our sites are running WP Rocket, which has the option to concatenate and minify CSS/JS files. This opens up yet more options for the developer to decide to either use CSSNano for optimization, or just to let WP Rocket handle it.

erwstout commented 7 years ago

I understand where you're coming from, but would prefer to keep it the same. When developing a theme you can just enqueue theme.css for your source maps and while its under development and then when ready to go to production, enqueue theme.min.css. While this would be the default functionality a developer could always choose to only enqueue theme.css and modify their gulpfile as they see fit for how they want to run their project.

I wouldn't and don't want us to rely on WP Rocket to handle the concatenation and minifcation because there have been many times where their optimization in these areas have straight up broken our sites, so we should not be using that in any form.

@mattada any thoughts on this?

bebaps commented 7 years ago

I disagree. I personally have never had WP Rocket break any site and I run it on all sites I have made since @mattada purchased it, and that is regardless of if I run it with either a minified or standard version of the theme CSS. Keep in mind WP Rocket will try to optimize all CSS and JS files found on the site, and you can alter this behavior as needed so maybe that is what broke it.

I also disagree because leaving it as is doesn't really allow a developer to build upon what is in place. Instead, this promotes undoing what is in place to get a desired effect. I think it would be better to leave options open from the very beginning.

erwstout commented 7 years ago

As for the prelude gulp tasks....

I don't really even see what would change here aside from enqueueing theme.css into the theme by default.

The tasks are broken up, so if you don't want to run both at once you can just run gulp minify-css or gulp-sass... gulp styles is just a wrapper to do all the style based tasks.

Prelude is a WordPress starter theme, and we've built it to work in this manner, I'm not really sure what the issue is and what you mean by "leaving options open."

To me this is a simple idea of, if you need to see source maps or want unmininifed css, then enqueue theme.css and if you want minified then you do nothing. I don't really see it needing to expand farther then that.

As a side note, I do believe we can modify the minified sourcemap to be "correct" and show what line it lives on in the scss file but last time I tried to get that part working it was unsuccessful. But if thats the issue we are trying to solve for maybe that should be the open issue.

mattada commented 7 years ago

Re: WP-Rocket. I don't want to depend on that for minifying our css. Yes we use it often, but its not always a guarantee. Ancora for example doesn't / cant use it because the server set up with mem-cache and nginx.

So I think using theme.css for development / troubleshooting as needed is ideal and the minified file for production.

To possibly ease that issue of modifying code on the functions each time, we could look to a environment set up option.

I wonder if there is a way to set a global variable that would dictate if a project is in development or production. Similar to how the maintenance.txt file tells wp public access the site is down for maintenance. I think if there was a way we could set a file of config status for the project that would flip which files are enqued based on status.

Or possibly a lower tech solution, if you are logged in, you get theme.css, and not logged in theme.min.css?