Skip to content
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

Use autoprefixer for Customizer builds #14980

Merged
merged 1 commit into from
Nov 4, 2014
Merged

Conversation

hnrch02
Copy link
Collaborator

@hnrch02 hnrch02 commented Nov 3, 2014

@hnrch02 hnrch02 added this to the v3.3.1 milestone Nov 3, 2014
@hnrch02
Copy link
Collaborator Author

hnrch02 commented Nov 3, 2014

I think this also solves #14750. The actual Less compilation is async and it was treated as sync before.

@hnrch02 hnrch02 force-pushed the customizer-autoprefixer branch 2 times, most recently from b2e5586 to d370168 Compare November 3, 2014 13:50
@XhmikosR
Copy link
Member

XhmikosR commented Nov 3, 2014

@hnrch02: isn't there a minified autoprefixer browser build?

@hnrch02
Copy link
Collaborator Author

hnrch02 commented Nov 3, 2014

I don't think so. But the deps get uglified anyway when building for GHP so does it matter?

@XhmikosR
Copy link
Member

XhmikosR commented Nov 3, 2014

BTW, this reminds me, it's in my TODO list to add a grunt task to generate those browser JS files from the node_modules we use so that we are in sync. I haven't got the time to get to it yet.

@XhmikosR
Copy link
Member

XhmikosR commented Nov 3, 2014

Not my decision @hnrch02... It was @mdo who preferred having the minified assets in the repo.

@hnrch02
Copy link
Collaborator Author

hnrch02 commented Nov 3, 2014

In any case it just slows grunt down when the already compressed files get compressed again with no benefit whatsoever.

@XhmikosR
Copy link
Member

XhmikosR commented Nov 3, 2014

Again, not my decision.

@XhmikosR
Copy link
Member

XhmikosR commented Nov 3, 2014

As for this PR, it breaks download with Firefox again.

@XhmikosR
Copy link
Member

XhmikosR commented Nov 3, 2014

Other than, the generated CSS looks good :)

@hnrch02
Copy link
Collaborator Author

hnrch02 commented Nov 3, 2014

The setTimeout workaround doesn't fix it for me though...

@XhmikosR
Copy link
Member

XhmikosR commented Nov 3, 2014

You mean if you add it in this branch or it didn't work for you before too?

@hnrch02
Copy link
Collaborator Author

hnrch02 commented Nov 3, 2014

The download on the current getbootstrap.com doesn't work either in Firefox.

@XhmikosR
Copy link
Member

XhmikosR commented Nov 3, 2014

I'm pretty sure it work(ed), because I use the customizer myself and I updated to 3.3.0 successfully the other day. Which Firefox version do you use?

@hnrch02
Copy link
Collaborator Author

hnrch02 commented Nov 3, 2014

33.0.2

@XhmikosR
Copy link
Member

XhmikosR commented Nov 3, 2014

33.1 x64 and works fine here. Also worked with 33.0.2 x64.

@hnrch02
Copy link
Collaborator Author

hnrch02 commented Nov 3, 2014

It tried it a couple of times and it seems kinda random. Sometimes it works other times it doesn't. Anyhow, I'll add the timeout back.

try {
compileLESS(bsLessSource, 'bootstrap', result)
var prefixer = autoprefixer({
browsers: [
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps we could move this data to a separate file to avoid the duplication between here and the Gruntfile?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See #14982.

@cvrebert
Copy link
Collaborator

cvrebert commented Nov 3, 2014

Probably time to file a Firefox bug, IMO, although we'd need to get a more reduced testcase, chopping out the irrelevant parts of FileSaver and Blob.js.

@XhmikosR
Copy link
Member

XhmikosR commented Nov 3, 2014

I confirm it works fine now with FF.

@hnrch02
Copy link
Collaborator Author

hnrch02 commented Nov 4, 2014

@cvrebert Maybe open a separate issue for that?

hnrch02 added a commit that referenced this pull request Nov 4, 2014
Use autoprefixer for Customizer builds
@hnrch02 hnrch02 merged commit d6a99cb into master Nov 4, 2014
@hnrch02 hnrch02 deleted the customizer-autoprefixer branch November 4, 2014 18:35
@cvrebert
Copy link
Collaborator

cvrebert commented Nov 4, 2014

Opened #15016 regarding the ongoing Firefox Customizer issue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Customizer doesn't run autoprefixer
3 participants