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

4.1.3 grid classes don't work with wicked_pdf's wkhtmltopdf 0.12.5 #27642

Closed
RudyOnRails opened this issue Nov 9, 2018 · 12 comments
Closed

4.1.3 grid classes don't work with wicked_pdf's wkhtmltopdf 0.12.5 #27642

RudyOnRails opened this issue Nov 9, 2018 · 12 comments
Labels

Comments

@RudyOnRails
Copy link

@RudyOnRails RudyOnRails commented Nov 9, 2018

bootstrap_v4.0.0.pdf
bootstrap_v4.1.3.pdf

I can confirm that upgrading to bootstrap 4.1.3 breaks grid layouts in wicked_pdf, which uses wkhtmltopdf, which uses Qt WebKit.

This may just be that QTWebKit hasn't kept up with cutting edge browser technologies but thought I would at least start here to register the issue, as I did not find it in my due-dilligence searching before posting.

I'm not a front-end pro, so I can't point out the cause of the bug (that is if it something that can even be resolved in BS). All I can say is that I heard that in BS v4, the grid system was revamped to use flexbox. I thought maybe QTWebkit couldn't hande that, but it can in my example with 4.0.0, but breaks when switching to 4.1.3.

Here's my supporting info, if it helps. Note the reduced case with the showterm and the repo.

Best, and thanks for all the work you do.
Kevin

@RudyOnRails RudyOnRails changed the title 4.1.3 grid classes don't work with wicked_pdf's wkhtmltopdf 4.1.3 grid classes don't work with wicked_pdf's wkhtmltopdf 0.12.5 Nov 9, 2018
@XhmikosR
Copy link
Member

@XhmikosR XhmikosR commented Nov 11, 2018

@galoberlyn
Copy link

@galoberlyn galoberlyn commented Dec 20, 2018

Hi I've got the same issue, however I found something here and it's quite helpful:
https://stackoverflow.com/questions/19705880/converting-twitter-bootstrap-page-into-pdf-with-wkhtmltopdf-span-issue
, just use col-xs-n, not sure why this works but it's definitely worth a try.

@RudyOnRails
Copy link
Author

@RudyOnRails RudyOnRails commented Dec 28, 2018

@galoberlyn thanks for chiming in. back when I was dealing with this I recall coming across that post - but I don't think it solved the problem.

Anywho, I tried it again, and here's my new commit if you want to look at the new pdf generated. In summary, after looking at the docu, Bootstrap 4* does not implement col-xs-*/n anymore, so it didn't work; they just use col-*/n instead now, which I also tried locally and it doesn't solve the problem.

@XhmikosR
Copy link
Member

@XhmikosR XhmikosR commented Dec 28, 2018

Yeah, these classes have been removed in Bootstrap 4. You'll have to use v3.x if you want these.

@XhmikosR XhmikosR closed this Dec 28, 2018
@RudyOnRails
Copy link
Author

@RudyOnRails RudyOnRails commented Dec 28, 2018

@XhmikosR why was this closed? It's still an issue...

@XhmikosR
Copy link
Member

@XhmikosR XhmikosR commented Dec 28, 2018

@RudyOnRails
Copy link
Author

@RudyOnRails RudyOnRails commented Dec 28, 2018

Thanks for writing back @XhmikosR. You did miss a little something. It's best to disregard the last few comments recently from @galoberlyn and i as they just add confusion.

@galoberlyn was just sharing a stack overflow answer to see if it would be a workaround but it was outdated as those xs classes don't even exist anymore in v4 like you said, so I don't think @galoberlyn read this issue fully in detail, and was thinking this was all v3 related. I still tested out @galoberlyn's suggestion, because I'm sick on vacation and have a slow brain and wanted to be respectful and actually respond to their guess of an outdated workaround.

The grid still breaks from 4.0 to 4.1 like I originally explained in my issue above.

Hope this helps clear things up.

@mdo
Copy link
Member

@mdo mdo commented Dec 29, 2018

Looking at the diff between v4.0.0 and v4.1.0, the only changes I see to the grid are the addition of some WebKit classes. See
https://github.com/twbs/bootstrap/compare/v4.1.0..v4.0.0#diff-af96efa4270ab5ad379f4965e12235a1. I’m assuming this was added when we tweaked some browser support somewhere? Could’ve been the addition of not dead?
13b8b9f#diff-b9cfc7f2cdf78a7f4b91a753d10865a2

@MartijnCuppens
Copy link
Member

@MartijnCuppens MartijnCuppens commented Dec 29, 2018

We 've upgraded to autoprefixer 8 which uses Browserlist 3.0 which has dropped support for older webkit engines.

You can solve this in 4.1 by changing this line:

"Chrome >= 45",

to "Chrome > 27",

or in 4.2 by changing this line:

Chrome >= 45

to Chrome > 27

And rebuild the sass afterwards. We'll not be changing this by default, because we don't support Qt WebKit by default due to its limited usage and adding this would increase our default file size.

I hope I helped you out with that, @RudyOnRails

@RudyOnRails
Copy link
Author

@RudyOnRails RudyOnRails commented Dec 31, 2018

@MartijnCuppens yes, that is a very suffice and exact explanation of the reasoning for the issue. Thank you for the information and the proposed workaround.

Now I agree that this issue should remain closed by @XhmikosR since bootstrap >= 4.1's grid layout will not work with wicked_pdf, since wicked_pdf uses wkhtmltopdf which uses Qt WebKit, which is an older Webkit engine which is no longer supported by Browserslist 3.0 which is used by autoprefixer 8 which is the version of autoprefixer that bootstrap upgraded to in 4.1.

Happy new year 🎊and thanks for y'all's time.

@pokono
Copy link

@pokono pokono commented Mar 21, 2021

Would be the same for bootstrap v4.6?
I'm trying to recompile it and it's not working for me.

@mralston
Copy link

@mralston mralston commented Dec 13, 2021

This should fix it.

.row
{
    display: -webkit-box;
}

Credit goes to https://github.com/eyupatis

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

Successfully merging a pull request may close this issue.

None yet
8 participants