-
Notifications
You must be signed in to change notification settings - Fork 9.3k
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 Arabic and Hebrew in invoices #27887
Fix Arabic and Hebrew in invoices #27887
Conversation
Revert MC-13880 and bring back MAGETWO-95816 This reverts commit 1c0549e
Add model for processing RTL texts
Hi @ihor-sviziev. Thank you for your contribution
For more details, please, review the Magento Contributor Guide documentation. |
5ada5c6
to
6d6cc47
Compare
Use rtl text handler in invoice PDF generation
6d6cc47
to
27ec0ae
Compare
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 fine, nice catch ✔️
Hi @lbajsarowicz, thank you for the review.
|
Hi @ihor-sviziev, thank you for your contribution! |
@ihor-sviziev thank you i used your fix and it already make the text be from right to left but the words are separated ?! |
O> @ihor-sviziev thank you i used your fix and it already make the text be from right to left but the words are separated ?!
As I said above - with the current library, we can't fix it. We need to use another library for that. So far, it's quite a significant effort to implement it. You could try to install some custom extension like https://store.fooman.co.nz/magento-extension-pdf-customiser-m2.html |
@ihor-sviziev thanks alot i will try 👍 |
Description (*)
I found fix that was reverted in 1c0549e due to backward compatibility issues (from @lenaorobei). I just re-applied these changes again, extracted whole logic with working with RTL texts to the separate model, covered it with unit tests and used it during invoice generation, so now it should be fully backward compatible.
Also I found that that solution didn't worked fine with Hebrew, so I also fixed it.
Note: for preparing test data I used some sample data generation service.
Related Pull Requests
Fixed Issues (if relevant)
Manual testing scenarios (*)
Questions or comments
Contribution checklist (*)