feat(android): honor textDecorationStyle on Text decorations#56768
feat(android): honor textDecorationStyle on Text decorations#56768quantizor wants to merge 2 commits into
Conversation
`textDecorationStyle` is declared on `TextStyleAndroid` in the public types but `TA_KEY_TEXT_DECORATION_STYLE` was a no-op handler: every value silently rendered as a solid line. This PR wires the prop through the existing C++ → Kotlin pipeline and implements `solid`, `double`, `dotted`, `dashed`, and `wavy` for both underlines and strikethroughs. Background: Android's `Layout.draw` paints the underline produced by `setUnderlineText(true)` using `paint.color`, ignoring `paint.underlineColor` on every API level, and offers no native way to draw a dotted / dashed / wavy decoration. The same applies to strikethrough. `ReactUnderlineSpan` and `ReactStrikethroughSpan` now extend `DrawCommandSpan` and paint the decoration themselves in `onDraw` via `Canvas.drawLine` / `Canvas.drawPath`, dispatching by style. This also makes `textDecorationColor` reach the paint as a side effect, closing a separate long-standing gap (see react#4579 from 2015). `TextDecorationStyle::Wavy` is added to the Fabric C++ primitives / conversions so the JS value flows through instead of being rejected with an `Unsupported value` log; the same enum is shared with iOS. The wavy curve uses Chromium/Blink's formula from `decoration_line_painter.cc` (`wavelength = 1 + 2 * round(2 * thickness + 0.5)`, `controlPointDistance = 0.5 + round(3 * thickness + 0.5)`, one cubic Bezier per wavelength with both control points at the midpoint, one above and one below the y-axis). The minimum stroke thickness is density-aware (1.5 dp) so decorations read consistently across display densities. The drawing loop iterates `while x < x2` so the final cycle continues through the last character (including trailing punctuation that would otherwise be visually uncovered when the run width is not an integer multiple of the wavelength). `ReactTextView.onDraw` invokes `DrawCommandSpan.onDraw` after `super.onDraw`, mirroring what `PreparedLayoutTextView.onDraw` already did. Without this, the new spans have no effect on the older view class, which is what some Text components on the new architecture still route through. ## Changelog: [GENERAL] [ADDED] - `textDecorationStyle: 'wavy'` for `<Text>` (see corresponding iOS PR for the iOS counterpart) [ANDROID] [ADDED] - Text decorations honor `textDecorationStyle` (`solid`, `double`, `dotted`, `dashed`, `wavy`) ## Test Plan: Rendered `<Text>` components with `textDecorationLine` set to `"underline"` or `"line-through"` and `textDecorationStyle` cycling through `solid` / `double` / `dotted` / `dashed` / `wavy`. On stock 0.85.2 every value renders as a solid line and `wavy` logs an `Unsupported value` warning; with this patch each style renders with the requested stroke geometry. Verified single-line and wrapped multi-line cases on an Android API 36 emulator: each visual line within a wrapped block receives its own correctly-styled decoration that starts and ends at the line's content boundaries. ```tsx <Text style={{ color: 'black', textDecorationLine: 'underline', textDecorationStyle: 'wavy', }}> Hello </Text> ```
|
@CalixTang has imported this pull request. If you are a Meta employee, you can view this in D104680895. |
|
Warning JavaScript API change detected This PR commits an update to
This change was flagged as: |
Three CI breakages were introduced by changes that landed on `main` between the original PR push and now: 1. PR react#56705 renamed `DrawCommandSpan` to `CanvasEffectSpan` and dropped the `ReactSpan` / `UpdateAppearance` interfaces from the base class. `ReactUnderlineSpan` and `ReactStrikethroughSpan` were extending the old name; rename them and re-declare `ReactSpan` so they remain valid `SetSpanOperation` arguments. `ReactTextView.onDraw` updated to import the new name. 2. Adding `Wavy` to `facebook::react::TextDecorationStyle` (this PR) left `RCTNSUnderlineStyleFromTextDecorationStyle` non-exhaustive, tripping `-Werror,-Wreturn-type` on iOS builds. Add a `Wavy` case that falls back to a solid underline (the actual wavy rendering ships in companion PR react#56769). 3. `validate_cxx_api_snapshots` flagged the missing `Wavy` entry in all six snapshots under `scripts/cxx-api/api-snapshots/`. Regenerate. No behavior change beyond what was already in the feature commit; this is purely "rebase the implementation onto current `main`."
3b58384 to
676ed17
Compare
|
@CalixTang following up, is any further action needed here? What's the typical merge timeline for RN fixes? |
|
thanks for the follow-up. Let me check in with someone on the team. |
@quantizor would you be able to rebase this one to start with? |
|
@cortinico merged this pull request in 87184c8. |
|
This pull request was successfully merged by @quantizor in 87184c8 When will my fix make it into a release? | How to file a pick request? |
Summary: `textDecorationStyle` is declared on `TextStyleIOS` in the public types but `wavy` is silently dropped: Fabric's C++ enum doesn't include `Wavy`, and UIKit's `NSUnderlineStyle` has no native wavy pattern bit. Separately, `dotted` and `dashed` map to `NSUnderlineStylePatternDot` / `NSUnderlineStylePatternDash` which don't match browser geometry on iOS. This PR adds `TextDecorationStyle::Wavy` to the shared Fabric primitives / conversions (also unblocks the same value on Android, see companion PR #56768) and renders wavy / dotted / dashed decorations with custom Core Graphics paths. **Implementation:** - Wavy ranges are tagged with a custom `RCTCustomDecorationAttributeName` (storing the line kinds, stroke color, and style key) in `RCTAttributedTextUtils.mm` and painted by `RCTTextLayoutManager.mm` after `drawGlyphsForGlyphRange:`. Wavy uses an adaptation of WebKit's formula from `Source/WebCore/style/InlineTextBoxStyle.cpp` (`controlPointDistance = thickness * 1.5 + 0.5`, one cubic Bezier per wavelength, control points at the midpoint above and below the y-axis). At iOS point sizes the literal Blink amplitude renders as a very pronounced wave because Core Graphics paints in points (not device pixels), so the constants are dialed back to read as a clear-but-subtle browser-style wave at typical text sizes. - Dotted uses a custom CG path with a zero-length dash + round line caps, producing actual circular dots at `2 * thickness` spacing. - Dashed uses a custom CG path with `[2 * thickness, thickness]` intervals — short rectangular dashes with a tight gap, closer to Safari's geometry than UIKit's default. - Solid and double continue to use UIKit's native `NSUnderlineStyle` pattern bits, so this PR does not touch the long-standing iOS Arial+bold solid-underline rendering bug tracked in #53935. - The wavy drawing loop iterates `while x < x2` so the final cycle continues through the last character (including trailing punctuation that would otherwise be visually uncovered when the run width is not an integer multiple of the wavelength). Companion PRs (independent, also targeting `main`): - #56767 — fix(android): textDecorationColor on underlines + strikethroughs. Resolves #4579 (2015). - #56768 — feat(android): textDecorationStyle solid/double/dotted/dashed/wavy. Shares the `TextDecorationStyle::Wavy` enum addition; whichever lands first leaves the other with a trivial conflict to resolve. ## Changelog: [IOS] [ADDED] - `textDecorationStyle: 'wavy'` for `<Text>` (custom CoreGraphics path) [IOS] [CHANGED] - `textDecorationStyle: 'dotted'` and `'dashed'` for `<Text>` render with custom CoreGraphics paths instead of UIKit pattern bits, matching browser geometry more closely Pull Request resolved: #56769 Test Plan: See the screenshot comparisons here: https://www.internalfb.com/compare-screenshots-from-diff/D104680636 {F1990979243} ---- Side-by-side comparison on iPhone 17 sim (iOS 26.4) of a `<Text>` with `textDecorationLine="underline"` and `textDecorationStyle` cycling through `solid` / `double` / `dotted` / `dashed` / `wavy`, verified against Safari rendering of the same CSS. Trailing periods now fall under the wavy stroke. Verified with `textDecorationColor` set distinct from the foreground color. ```tsx <Text style={{ color: 'black', textDecorationLine: 'underline', textDecorationStyle: 'wavy', textDecorationColor: '#ff00aa', }}> Hello </Text> ``` Reviewed By: cipolleschi Differential Revision: D104680636 Pulled By: cortinico fbshipit-source-id: ac96e5b36530f7d243a4b85a67c576b62fe99866
Summary:
textDecorationStyleis declared onTextStyleAndroidin the public types butTA_KEY_TEXT_DECORATION_STYLEwas a no-op handler: every value silently rendered as a solid line, andwavywas additionally rejected at the Fabric C++ enum boundary with anUnsupported valuelog. This PR wires the prop through the existing C++ → Kotlin pipeline and implementssolid,double,dotted,dashed, andwavyfor both underlines and strikethroughs.Android's
Layout.drawpaints the underline produced bysetUnderlineText(true)usingpaint.coloronly and offers no native way to draw a dotted / dashed / wavy decoration.ReactUnderlineSpanandReactStrikethroughSpannow extendDrawCommandSpanand paint the decoration themselves inonDrawviaCanvas.drawLine/Canvas.drawPath, dispatching by style. As a side effect this also makestextDecorationColorreach the paint, closing the separate long-standing gap filed as #4579 in 2015 — the companion color-focused PR #56767 isolates that fix for reviewers who only want the color change.TextDecorationStyle::Wavyis added to the Fabric C++ primitives / conversions so thewavyJS value flows through; the same enum is shared with iOS (see companion iOS PR #56769).The wavy curve uses Chromium/Blink's formula from
decoration_line_painter.cc(wavelength = 1 + 2 * round(2 * thickness + 0.5),controlPointDistance = 0.5 + round(3 * thickness + 0.5), one cubic Bezier per wavelength with both control points at the midpoint, one above and one below the y-axis). The minimum stroke thickness is density-aware (1.5 dp) so decorations read consistently across display densities. The drawing loop iterateswhile x < x2so the final cycle continues through the last character (including trailing punctuation that would otherwise be visually uncovered when the run width is not an integer multiple of the wavelength).ReactTextView.onDrawinvokesDrawCommandSpan.onDrawaftersuper.onDraw, mirroring whatPreparedLayoutTextView.onDrawalready did. Without this, the new spans have no effect on the older view class, which is what some Text components on the new architecture still route through.Companion PRs (independent, also targeting
main):TextDecorationStyle::Wavyenum addition; whichever lands first leaves the other with a trivial conflict to resolve.Changelog:
[GENERAL] [ADDED] -
textDecorationStyle: 'wavy'for<Text>(see corresponding iOS PR for the iOS counterpart)[ANDROID] [ADDED] - Text decorations honor
textDecorationStyle(solid,double,dotted,dashed,wavy)Test Plan:
Rendered
<Text>components withtextDecorationLineset to"underline"or"line-through"andtextDecorationStylecycling throughsolid/double/dotted/dashed/wavy. On stock 0.85.2 every value renders as a solid line andwavylogs anUnsupported valuewarning; with this patch each style renders with the requested stroke geometry. Verified single-line and wrapped multi-line cases on an Android API 36 emulator: each visual line within a wrapped block receives its own correctly-styled decoration that starts and ends at the line's content boundaries.