Skip to content
This repository was archived by the owner on Mar 31, 2020. It is now read-only.

Conversation

jonathanpmartins
Copy link
Contributor

I know this isn't the most elegant solution for the (custom styles) autoprefixer case, but for me it works great! If you guys have a nicer solution, please teach me so I can do better! I know too that this is not the right place to put a autoprefixer service. (because this is a combiner task file). But it works so beautiful, sorry that the code looks inconsistent and so ugly.

I know this isn't the best solution for the (custom styles) autoprefixer case, but for me it works great! If you guys have a nicer solution, please teach me so I can do better!
@carmanchris31-old
Copy link

It would probably be much better to break the chain to insert some conditional code rather than repeating so much code this way. You could also make the prefixing optional since you were concerned about this being primarily a concatenation task. Something like:

stream = gulp.src(set.src)
    .pipe(plugins.concat(fileName));

if( options.taskName == "styles" && options.autoprefix === true )
  stream
      .pipe(plugins.autoprefixer());

stream
    .pipe(plugins.if(config.production, options.minifier.call(this)))
    .pipe(gulp.dest(set.to));

Could probably take it one step further and actually just allow for specification of plugins through the options and call them dynamically like the minifier. Would be a lot more flexible, but it is still a question of if this is the best place for it given its purpose. We could just as easily perform things like prefixing before/after the process and keep the concatenation as simply that.

@jonathanpmartins
Copy link
Contributor Author

Thank you for the tips, for the time being, I change the code and make a new commit.

@carmanchris31-old
Copy link

Nice, thanks. :)

I had another thought, looking further at the minification line, realizing that it, too, is conditional. It makes use of gulp-if, which says the condition can be anything that is accepted by gulp-match (including booleans and functions).

Thus, rather than breaking the chain at all, we could probably just wrap the pipe content with a plugins.if() condition as with the minification, like this:

stream = gulp.src(set.src)
        .pipe(plugins.concat(fileName));
        .pipe(plugins.if(options.taskName == "styles", plugins.autoprefixer()));
        .pipe(plugins.if(config.production, options.minifier.call(this)))
        .pipe(gulp.dest(set.to));

CSS Autoprefixer that not break the chain. Credits to @carmanchris31
@jonathanpmartins
Copy link
Contributor Author

Man, your are the master! ehehehhe
How could I din't see that.
I make a new commit. Thank you for your time! Very helpful!

@jonathanpmartins
Copy link
Contributor Author

Nicer and cleaner!

@jonathanpmartins
Copy link
Contributor Author

Hope it get merged soon! 👍

@tymondesigns
Copy link
Contributor

this has already been implemented here

@jonathanpmartins
Copy link
Contributor Author

Yes @tymondesigns, but its only if you use Less or Sass to compile css.
If you use custom css like me, this autoprefixer feature don't work. This code ".pipe(plugins.autoprefixer())" on line 24 of the GulpCssCompiler nevers get triggered.

You can see it here: https://github.com/laravel/elixir/blob/master/ingredients/styles.js
....only combining, when using "mix.styles(...)"

@tymondesigns
Copy link
Contributor

I see your point, when not using a preprocessor... Although this does feel a little hacky to me... I don't think GulpCombiner.js should be concerned with any styling specific logic.

Maybe instead it would be better to have a css ingredient that does this?

@jonathanpmartins
Copy link
Contributor Author

Yeah, I agree with you!
But, it works so easy to me. Make the job with a little tweak. "When combining styles, do autoprefix. And the task is done.

I'm very busy lately, but If you help me, I we can done this better!

@tymondesigns
Copy link
Contributor

To be honest, my advice to you would be to use sass. You could just change the extension to .scss and still use plain old CSS if you dont want to learn the syntax... then you will get the autoprefixing and minification etc

@jonathanpmartins
Copy link
Contributor Author

Ok, yeah, I understood you, and your point if perfectly valid, but for me this is not the way I like to do it.
To me, changing the extension of files are more hacky than simply autoprefix styles. Otherwise I have to change a lot of my code, heheheheheh
Simple is better!

@jonathanpmartins
Copy link
Contributor Author

Yep | Nope
👍 | 👎
?

@JeffreyWay
Copy link
Contributor

Going to close for now.

Not sure that stylesheet concatenation should include autoprefixing by default.

@JeffreyWay JeffreyWay closed this Nov 19, 2014
@jonathanpmartins
Copy link
Contributor Author

Hey @JeffreyWay , I create a new Pull Request to resolve the autoprefixer issue on custom styles https://github.com/laravel/elixir/pull/27/files

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.

4 participants