autoprefixer support #472

Merged
merged 4 commits into from Aug 27, 2014

Conversation

Projects
None yet
6 participants
@mente
Contributor

mente commented Jul 20, 2013

Hello,

This PR adds support of great lib autoprefixer that helps to solve down vendor-specific nightmare in css. I know there are mixins, but not everyone uses sass, less or other css compilers.

Looking forward for comments.

@ai

This comment has been minimized.

Show comment
Hide comment
@ai

ai Aug 8, 2013

CSS Tricks publish good article, why Autoprefixer is cool and how it works: http://css-tricks.com/autoprefixer/

ai commented Aug 8, 2013

CSS Tricks publish good article, why Autoprefixer is cool and how it works: http://css-tricks.com/autoprefixer/

@mente mente referenced this pull request Aug 8, 2013

Closed

Added AutoprefixerFilter #483

+ /**
+ * @param string $browser
+ */
+ public function setBrowsers($browser)

This comment has been minimized.

@stof

stof Aug 8, 2013

Collaborator

I would find it more logical to configure the browsers as an array in PHP and build the comma separated string only when calling the binary.

Note that when calling autoprefixer programmatically in JS, browsers are also passed as an array

@stof

stof Aug 8, 2013

Collaborator

I would find it more logical to configure the browsers as an array in PHP and build the comma separated string only when calling the binary.

Note that when calling autoprefixer programmatically in JS, browsers are also passed as an array

This comment has been minimized.

@mente

mente Aug 8, 2013

Contributor

Yes, I think it would be more clear

@mente

mente Aug 8, 2013

Contributor

Yes, I think it would be more clear

+ public function filterLoad(AssetInterface $asset)
+ {
+ $input = $asset->getContent();
+ $pb = $this->createProcessBuilder(array($this->autoprefixerBin, '-o', '-'));

This comment has been minimized.

@stof

stof Aug 8, 2013

Collaborator

you should use an output file instead of stdout because proc_open on windows has issues when the output is too large. See other filters to see how it is done

@stof

stof Aug 8, 2013

Collaborator

you should use an output file instead of stdout because proc_open on windows has issues when the output is too large. See other filters to see how it is done

This comment has been minimized.

@mente

mente Aug 8, 2013

Contributor

arghhh. Ok

@mente

mente Aug 8, 2013

Contributor

arghhh. Ok

This comment has been minimized.

@mente

mente Aug 8, 2013

Contributor

I believe this is the issue. But there are other filters that use same proc_open, i.e. CoffeeScriptFilter. They should be fixed too?

@mente

mente Aug 8, 2013

Contributor

I believe this is the issue. But there are other filters that use same proc_open, i.e. CoffeeScriptFilter. They should be fixed too?

This comment has been minimized.

@mente

mente Aug 8, 2013

Contributor

Same problem exists for SassFilter.

@mente

mente Aug 8, 2013

Contributor

Same problem exists for SassFilter.

@mente

This comment has been minimized.

Show comment
Hide comment
@mente

mente Aug 8, 2013

Contributor

Should I squash commits into 1?

Contributor

mente commented Aug 8, 2013

Should I squash commits into 1?

@ai

This comment has been minimized.

Show comment
Hide comment
@ai

ai Aug 8, 2013

What How To I should post on Autoprefixer README.md?

ai commented Aug 8, 2013

What How To I should post on Autoprefixer README.md?

@mente

This comment has been minimized.

Show comment
Hide comment
@mente

mente Aug 8, 2013

Contributor

@ai i'll provide PR to your repo. Also there will be PR for Symfony2 Bundle after this PR is merged. Hopefully soon enough :)

Contributor

mente commented Aug 8, 2013

@ai i'll provide PR to your repo. Also there will be PR for Symfony2 Bundle after this PR is merged. Hopefully soon enough :)

@guill3m

This comment has been minimized.

Show comment
Hide comment
@guill3m

guill3m Jan 14, 2014

What's the status on this PR?

Is it planned to be merged to core or should we use it as a custom filter?

Thank you.

guill3m commented Jan 14, 2014

What's the status on this PR?

Is it planned to be merged to core or should we use it as a custom filter?

Thank you.

@patchampoux

This comment has been minimized.

Show comment
Hide comment
@patchampoux

patchampoux Aug 27, 2014

Please merge this?

Thank you.

Please merge this?

Thank you.

kriswallsmith added a commit that referenced this pull request Aug 27, 2014

@kriswallsmith kriswallsmith merged commit 7c9639d into kriswallsmith:master Aug 27, 2014

1 check passed

default The Travis CI build passed
Details
@kriswallsmith

This comment has been minimized.

Show comment
Hide comment
@kriswallsmith

kriswallsmith Aug 27, 2014

Owner

Thank you!

Owner

kriswallsmith commented Aug 27, 2014

Thank you!

@mente

This comment has been minimized.

Show comment
Hide comment
@mente

mente Aug 27, 2014

Contributor

Awesome, thanks!

Contributor

mente commented Aug 27, 2014

Awesome, thanks!

@stof

This comment has been minimized.

Show comment
Hide comment
@stof

stof Aug 27, 2014

Collaborator

tests are broken on Travis

Collaborator

stof commented Aug 27, 2014

tests are broken on Travis

@mente

This comment has been minimized.

Show comment
Hide comment
@mente

mente Aug 27, 2014

Contributor

PR was made more than year ago, might be outdated. Checking it

Contributor

mente commented Aug 27, 2014

PR was made more than year ago, might be outdated. Checking it

@ai

This comment has been minimized.

Show comment
Hide comment
@ai

ai Aug 27, 2014

Autoprefixer was rewritten several times since this PR ;).

ai commented Aug 27, 2014

Autoprefixer was rewritten several times since this PR ;).

mente added a commit to mente/assetic that referenced this pull request Aug 27, 2014

stof added a commit that referenced this pull request Sep 16, 2014

Merge pull request #644 from mente/autoprefix-path
fixed autoprefixer unit tests problem introduced in #472
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment