-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
fix: rework hyphenation #3188
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
base: master
Are you sure you want to change the base?
fix: rework hyphenation #3188
Conversation
The hyphenation algorithm may change the string (e.g. by removing some characters, namely soft hyphens). Therefore, calculating the glyphs must come after hyphenation, so that the glyphs match the final string. Fixes diegomura#3018 This was probably broken in diegomura#2600.
The Hyphenation algorithm should be able to leave soft hyphens in, to indicate that a hyphen should be placed there if the word breaks there.
The line breaking algorithm needs to distinguish syllables which end with a soft hyphen from syllables that do not, and only mark a syllable for adding a hyphen in the former case.
For the line breaking algorithm, soft hyphens should be considered to have a width of zero, since they are never printed directly (they can only lead to an inserted hyphen if at the end of a line). The font package was already doing this correctly, but the pdfkit package considered the soft hyphen to be the same as a normal hyphen with an advanceWidth of 333 in Helvetica. Without this change, in some edge cases the pdfkit would break apart lines already broken apart by the line breaking algorithm in textkit. Added tests for both packages to make sure they remain compatible in the future.
In the best fit line breaking algorithm, the width of the hyphen must be taken into account, in case one is to be inserted at the end of the line. This is the most readable change I was able to find to acheive the goal. Maybe the bestFit algorithm could be optimized in the future, along with writing extensive tests for corner cases.
Therefore, we remove all soft hyphens from the attributed string after linebreaking is completed, and recalculate the glyphs afterwards. This way, pdfkit never sees the soft hyphens, and does not mistake them for normal hyphens.
Tests the functionality of custom word splitting functions
🦋 Changeset detectedLatest commit: e343b3d The changes in this PR will be included in the next version bump. This PR includes changesets to release 13 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Insane work. Looks good to me! |
.changeset/ninety-dogs-grow.md
Outdated
|
|
||
| This allows you to break correctly on normal hyphens or other special characters in your text. For example, to use the default english-language syllable breaking built into react-pdf, but also break after hyphens naturally occurring in your text (such as is often present in hyperlinks), you could use the following hyphenation callback: | ||
| ```js | ||
| import { wordHyphenation } from '@react-pdf/textkit'; |
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 means @react-pdf/textkit will now be something users shuold import from? I rather keep this "internal", and all public apis being exported from @react-pdf/renderer
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.
Makes sense, I'll prepare a fix
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.
The import is not necessary anymore with e343b3d
.changeset/ninety-dogs-grow.md
Outdated
| import { Font } from '@react-pdf/renderer'; | ||
|
|
||
| const originalHyphenationCallback = wordHyphenation() | ||
| Font.registerHyphenationCallback((word) => { |
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.
Maybe we should consider passing the default hyphenation fn as argument of the callback? Less things to care as a user
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.
Done in e343b3d
|
@carlobeltrame great work man! Sorry for the late response here. Can you guide me on how (or if) this fixes solving hyphenation in different languages? I always thought we should export some hyphenation "language specifications" (if that even exists) that people can pass to |
This allows library users to avoid importing the callback themselves, which probably most of the implementations will want to do.
@diegomura see also diegomura/react-pdf-site#150 for a documentation PR which attempts to do just that. After this PR here, a full example for German and for splitting at hyphens in the source text could look like this: import { Font } from "@react-pdf/renderer";
import { hyphenateSync as hyphenateDe } from "hyphen/de";
const SOFT_HYPHEN = '\u00ad';
const hyphenationCallback = (word) => {
// technically, null may be passed as argument
if (word === null) return [];
// take care to leave the soft hyphens in the split parts, so a hyphen is
// rendered in the pdf if the string is split at that position
const syllables = hyphenateDe(word).split(new RegExp(`(?<=${SOFT_HYPHEN})`));
// also allow line splitting at dashes in the text, e.g. inside URLs
return syllables.flatMap((syllable) => syllable.split(/(?<=-)/))
};
Font.registerHyphenationCallback(hyphenationCallback);I don't think react-pdf should be responsible for providing the hyphenation rules for all languages. I think users should be responsible for selecting and installing only the necessary language-specific and domain-specific hyphenation dictionaries which they use. The other languages of the If react-pdf ever starts to include "presets" for hyphenation in common western languages, the customizable hyphenationCallback will still be necessary. Imagine if a company named "foobar" never wants to have its company name hyphenated in their pdfs, they could implement this using the hyphenationCallback. Also, please note that I only have experience with western languages such as English, German, French, Italian, etc., which share pretty similar hyphenation strategies. I have no idea how hyphenation works in e.g. middle eastern or asian languages, and cannot consult on how to implement support for that. I think if there is a need for hyphenation in such a language, we need contributors from these regions to describe the required rules in an issue first. |
|
Hello. I'm a Japanese speaker. Next, regarding hyphenation rules in Japanese, I personally think they are unnecessary. It's sufficient for each person to break lines at appropriate positions. Currently, using React-PDF inserts hyphens in Japanese text, which is extremely disruptive. (Even following the steps to disable it doesn't remove them.) I hope this is helpful. Thank you. (Please note that this translation was generated by a translation tool and may contain unnatural expressions.) |
|
〔Addendum〕Is the fact that entering meaningless characters (e.g., “aaaa”) doesn't cause a line break related to hyphenation? |
Yes. The current version of react-pdf adds line breaks on
Since |


Fixes #3018, was broken since #2600
Fixes #1642
Fixes #2456
Fixes #2564
Fixes #2739
Enables better solutions for the following issues:
Fixes #1238
Fixes #1380
Fixes #1416
Fixes #1662
I added each change in a separate commit, with accompanying tests to make sure the intention of the code change will not as easily be broken again in the future. Especially the textkit layout integration test took a while to write, but should be of great importance to detect unwanted changes in the future.
This is technically a breaking change for people who have previously used custom word wrapping functions, but one could argue this feature has been quite broken for a long time now, as described in #3018. I can also update the documentation website and possibly some of the examples once this is merged.