Split text into graphemes correctly #9646
Merged
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Use
graphemeSplit
to correctly split chars into graphemes correctly for textStyles conversion.This bug doesn't cause actually any issue to fabric, which I why I could not write any test. Fabric uses the textStyles utils only for
toObject
andfromObject
, but since the bug is present in both directionsfromObject(toObject(textStyles)) === textStyles
, but it's technically wrong. If you use a text like👨👩👦👦
, it will be looped through as 4 distinct chars, but it's a single grapheme.I know we should use
object.graphemeSplit
to actually use the overriden implementation from the consumer, but there's a comment that says there should be a way to provide thegraphemeSplit
implementation directly to fabric. So for now I think this is enough as reminder thattextStyles
should do grapheme splitting as well. If needed, we can change the functions to accepttextLines: string[][]
as array of grapheme-split text lines.Also nobody reported this issue. I knew about it only because at work we use the
TextStyleArray
format both for serialization and for the React UI state, e.g. to show the text formatting buttons for the selected text.