-
-
Notifications
You must be signed in to change notification settings - Fork 724
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
(Fix) chore(deps): bump wicked_pdf from 2.6.3 to 2.8.0 [OFN-12214] #12743
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @rioug for reviewing |
locally |
This should happen locally since
Then I noticed the Can you please checkout this conversation I had with Maikel |
I had missed that sorry!
It looks like we'll need to do this, do you want to have a go at it ? |
@wandji20 There is a a failing spec, could you have a look at it ? |
Bumps [wicked_pdf](https://github.com/mileszs/wicked_pdf) from 2.6.3 to 2.8.0. - [Release notes](https://github.com/mileszs/wicked_pdf/releases) - [Changelog](https://github.com/mileszs/wicked_pdf/blob/master/CHANGELOG.md) - [Commits](https://github.com/mileszs/wicked_pdf/commits) --- updated-dependencies: - dependency-name: wicked_pdf dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <[email protected]>
…y parsed when converting pdf to text [OFN-12214]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not so positive this is a good change.
So, I noticed a strange output in extracted text from pdf.
This is caused due to multi-line table header values.
Sadly, I could not figure out why it works when the mail.scss
is compiled with Webpacker (before changes).
For an invoice with a header (see screenshot)
I get this
Seems to me it reads the header as two lines instead of one due to the longer header values spanning two lines
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can happen in the conversion to text. It's very unfortunate but I don't think it can be easily solved. So we have to work with it for now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can happen in the conversion to text. It's very unfortunate but I don't think it can be easily solved. So we have to work with it for now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good 👍
Notes to self for testing:
|
@drummer83 BUU is about to be displayed to 75% of the users. And also dev and testers environment are displayed by default on admin style v3. For invoice feature, I don't know when we will have budget to move forward, so you can also only focus on invoice with toggle OFF. |
Hi @wandji20, Here are my testing notes:
The invoices are sent and printed correctly, the corresponding layout is applied. ✔️ I came across some other issues while testing, but those are not related to this PR. This one is ready to go! Great work! 🎉 |
What? Why?
Remove webpack compiled CSS from order pdf files
What should we test?
Generated order pdf from (print order) should work and look the same as before
http://localhost:3000/admin/orders/
Release notes
(Fix) chore(deps): bump wicked_pdf from 2.6.3 to 2.8.0 [OFN-12214]
(Fix) chore(deps): bump wicked_pdf from 2.6.3 to 2.8.0
Dependencies
wicked-pdf
Documentation updates
N/A