Skip to content

Commit 56029aa

Browse files
authored
fix(📝): wordSpacing property not working in Paragraph text styles (Shopify#3653)
1 parent 6db3dc5 commit 56029aa

5 files changed

Lines changed: 158 additions & 0 deletions

File tree

packages/skia/cpp/api/JsiSkTextStyle.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,10 @@ class JsiSkTextStyle {
143143
auto propValue = object.getProperty(runtime, "letterSpacing");
144144
retVal.setLetterSpacing(propValue.asNumber());
145145
}
146+
if (object.hasProperty(runtime, "wordSpacing")) {
147+
auto propValue = object.getProperty(runtime, "wordSpacing");
148+
retVal.setWordSpacing(propValue.asNumber());
149+
}
146150
if (object.hasProperty(runtime, "locale")) {
147151
auto propValue = object.getProperty(runtime, "locale");
148152
retVal.setLocale(SkString(propValue.asString(runtime).utf8(runtime)));
14.1 KB
Loading
38.8 KB
Loading

packages/skia/src/renderer/__tests__/e2e/Paragraphs.spec.tsx

Lines changed: 154 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,10 @@ const Noto = Array.from(
3131
resolveFile("skia/__tests__/assets/NotoColorEmoji.ttf")
3232
);
3333

34+
const Amiri = Array.from(
35+
resolveFile("skia/__tests__/assets/Amiri-Regular.ttf")
36+
);
37+
3438
describe("Paragraphs", () => {
3539
it("Should build the first example from the documentation", async () => {
3640
const img = await surface.drawOffscreen(
@@ -315,6 +319,156 @@ describe("Paragraphs", () => {
315319
);
316320
});
317321

322+
it("should apply letterSpacing and wordSpacing to RTL Arabic text", async () => {
323+
const img = await surface.drawOffscreen(
324+
(Skia, canvas, ctx) => {
325+
const amiri = Skia.Typeface.MakeFreeTypeFaceFromData(
326+
Skia.Data.fromBytes(new Uint8Array(ctx.Amiri))
327+
)!;
328+
const provider = Skia.TypefaceFontProvider.Make();
329+
provider.registerFont(amiri, "Amiri");
330+
331+
// Paragraph without spacing (reference)
332+
const paraNoSpacing = Skia.ParagraphBuilder.Make(
333+
{
334+
textAlign: ctx.textAlign,
335+
textDirection: ctx.textDirection,
336+
},
337+
provider
338+
)
339+
.pushStyle({
340+
fontFamilies: ["Amiri"],
341+
fontSize: 24,
342+
color: Skia.Color("black"),
343+
})
344+
.addText("بِسْمِ اللَّهِ")
345+
.pop()
346+
.build();
347+
paraNoSpacing.layout(ctx.width);
348+
paraNoSpacing.paint(canvas, 0, 0);
349+
350+
// Paragraph with letter spacing
351+
const paraLetterSpacing = Skia.ParagraphBuilder.Make(
352+
{
353+
textAlign: ctx.textAlign,
354+
textDirection: ctx.textDirection,
355+
},
356+
provider
357+
)
358+
.pushStyle({
359+
fontFamilies: ["Amiri"],
360+
fontSize: 24,
361+
color: Skia.Color("black"),
362+
letterSpacing: 10,
363+
})
364+
.addText("بِسْمِ اللَّهِ")
365+
.pop()
366+
.build();
367+
paraLetterSpacing.layout(ctx.width);
368+
paraLetterSpacing.paint(canvas, 0, 50);
369+
370+
// Paragraph with word spacing
371+
const paraWordSpacing = Skia.ParagraphBuilder.Make(
372+
{
373+
textAlign: ctx.textAlign,
374+
textDirection: ctx.textDirection,
375+
},
376+
provider
377+
)
378+
.pushStyle({
379+
fontFamilies: ["Amiri"],
380+
fontSize: 24,
381+
color: Skia.Color("black"),
382+
wordSpacing: 20,
383+
})
384+
.addText("بِسْمِ اللَّهِ")
385+
.pop()
386+
.build();
387+
paraWordSpacing.layout(ctx.width);
388+
paraWordSpacing.paint(canvas, 0, 100);
389+
},
390+
{
391+
Amiri,
392+
textAlign: TextAlign.Right,
393+
textDirection: TextDirection.RTL,
394+
width: surface.width,
395+
}
396+
);
397+
checkImage(
398+
img,
399+
`snapshots/paragraph/paragraph-rtl-arabic-spacing-bug-${surface.OS}.png`
400+
);
401+
});
402+
403+
it("should compare wordSpacing between LTR and RTL mixed text", async () => {
404+
const img = await surface.drawOffscreen(
405+
(Skia, canvas, ctx) => {
406+
const wordSpacing = 40;
407+
const roboto = Skia.Typeface.MakeFreeTypeFaceFromData(
408+
Skia.Data.fromBytes(new Uint8Array(ctx.RobotoRegular))
409+
)!;
410+
const amiri = Skia.Typeface.MakeFreeTypeFaceFromData(
411+
Skia.Data.fromBytes(new Uint8Array(ctx.Amiri))
412+
)!;
413+
const provider = Skia.TypefaceFontProvider.Make();
414+
provider.registerFont(roboto, "Roboto");
415+
provider.registerFont(amiri, "Amiri");
416+
417+
const mixedText =
418+
"Lorem ipsum dolor sit amet, טקסט ללא רווח בין מילים, نص مع عدم وجود مسافات بين الكلمات";
419+
420+
// LTR with wordSpacing
421+
const paraLTR = Skia.ParagraphBuilder.Make(
422+
{
423+
textDirection: ctx.LTR,
424+
},
425+
provider
426+
)
427+
.pushStyle({
428+
fontFamilies: ["Roboto", "Amiri"],
429+
fontSize: 16,
430+
color: Skia.Color("black"),
431+
wordSpacing,
432+
})
433+
.addText(mixedText)
434+
.pop()
435+
.build();
436+
paraLTR.layout(ctx.width);
437+
paraLTR.paint(canvas, 0, 0);
438+
439+
// RTL with wordSpacing
440+
const paraRTL = Skia.ParagraphBuilder.Make(
441+
{
442+
textDirection: ctx.RTL,
443+
},
444+
provider
445+
)
446+
.pushStyle({
447+
fontFamilies: ["Roboto", "Amiri"],
448+
fontSize: 16,
449+
color: Skia.Color("black"),
450+
wordSpacing,
451+
})
452+
.addText(mixedText)
453+
.pop()
454+
.build();
455+
paraRTL.layout(ctx.width);
456+
paraRTL.paint(canvas, 0, 80);
457+
},
458+
{
459+
RobotoRegular,
460+
Amiri,
461+
LTR: TextDirection.LTR,
462+
RTL: TextDirection.RTL,
463+
width: surface.width,
464+
}
465+
);
466+
checkImage(
467+
img,
468+
`snapshots/paragraph/paragraph-word-spacing-ltr-rtl-${surface.OS}.png`
469+
);
470+
});
471+
318472
itRunsE2eOnly(
319473
"should show ellipse when line count is above max lines",
320474
async () => {
539 KB
Binary file not shown.

0 commit comments

Comments
 (0)