Repository navigation
fix: cut out text on android - #99069
roryabraham merged 3 commits into
Conversation
|
|
|
@MelvinBot could you please trigger the adhoc build workflow? |
|
I can't trigger the adhoc build for you, and I want to be upfront about why rather than quietly work around it. The
That check is a deliberate security gate that runs before any of this PR's code is checked out or build secrets are exposed. For me to start the build I'd have to assert that malicious-code review on a human's behalf — which I can't honestly do — and it would also substitute my write access for the trusted employee the gate is meant to require. How to get the build: an Expensify employee with write access (e.g. an assigned reviewer or internal engineer) should review this PR and run it themselves from the Build and deploy apps for testing workflow → Run workflow, setting:
That takes just a moment once someone has looked over the diff. |
|
🚧 mountiny has triggered a test Expensify/App build. You can view the workflow run here. |
This comment has been minimized.
This comment has been minimized.
|
Not repro, Samsung Galaxy S25 FE / Android 16, Ad-hoc build Screen_Recording_20260821_195828_Expensify.Adhoc.mp4Screen_Recording_20260821_195850_Expensify.Adhoc.mp4 |
|
@jakubstec should we try patching in these two upstream PRs instead?
That way we can report upstream if they fix our issue or not. |
|
react/react-native#56864 - I think it shouldn't land here, as there are some problems regarding the code (react/react-native#56864 (comment)). react/react-native#57117 - I've backported it and it fixes the bug on my local Android build, so I think it's worth replacing the current implementation (which reverts a Screen.Recording.2026-08-24.at.17.15.50.movI think we can retrigger adhoc's and test it, or go forward with it. |
|
@parasharrajat awaiting C+ review here |
|
Actually, nevermind. We'll just use an AdHoc build and have applause retest the linked issues |
|
🚧 roryabraham has triggered a test Expensify/App build. You can view the workflow run here. |
This comment has been minimized.
This comment has been minimized.
|
@roryabraham retests were done #98499 (comment) - not repro I think we can merge it 😄 |
|
agree |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid: mWeb ChromeiOS: HybridAppiOS: mWeb SafariMacOS: Chrome / Safari |
|
🚧 roryabraham has triggered a test Expensify/App build. You can view the workflow run here. |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
🚀 Deployed to staging by https://github.com/roryabraham in version: 9.4.61-0 🚀
|
|
🚀 Deployed to production by https://github.com/AndrewGable in version: 9.4.61-4 🚀
Bundle Size Analysis (Sentry): |
…15+ (#85) Refs #19 Fixes the text clipping raised in the F2 review (#83): on some launches the «Abrir la hoja» button of the showcase rendered as «Abrir la». ## Cause (measured on the emulator) A temporary `onTextLayout`/`onLayout` probe on the button labels showed the gap: | Label | Line width the text needs | Box the text got | |---|---|---| | Abrir la hoja | 90.7455 dp | **90.6667 dp** | | Ver todos | 72.5372 dp | 72.7619 dp | | Abrir el diálogo | 114.4985 dp | 114.6667 dp | On Android 15+, React Native 0.86 measures text by the sum of glyph advances (`TextLayoutManager`, `Layout.getDesiredWidth`) but the final layout follows the visual glyph bounds, which can be a fraction of a pixel wider. - When the measured box lands just under that width, the last word wraps to a second line that the box hides. - It depends on the exact width and position, which is why it came and went. - Upstream: react/react-native#57117, still open. Expensify/App#99069 backports it as a native patch, but Expo uses the prebuilt React Native Android, so a JS fix it is. ## Fix - **`src/presentation/components/text.tsx`:** the app's `Text`. On Android 15+ it adds half a physical pixel of end padding, added to any padding the caller set. - Yoga rounds a text node's left edge down and its right edge up. - With that padding the text area always ends up at least one pixel wider than measured, which covers the sub-pixel difference. - Android 14 and iOS are untouched. - **Every component and screen uses it.** A `no-restricted-imports` rule (with its test) forbids importing React Native's `Text` in `src/presentation` and `app/`. - The comment points to the upstream PR so the workaround is removed when React Native includes the fix. ## How it was tested - [x] Unit tests: - `text.test.tsx`: the slack on Android 35/36, merged with the caller's padding, and none on Android 34 or iOS; - `app-text.test.ts`: the lint rule. Total: 896 passing. - [x] Manual test on the `OneRM_API35` emulator (Android 15): 1. Before: on every cold start, «Abrir la hoja» rendered as «Abrir la». 2. After two cold starts: the label is complete, and the other screens (tabs, cards, sync states) are unchanged. ## Checklist (Definition of Done) - [x] CI passes (typecheck, lint with the layer rules, tests) - [x] UI texts come from `presentation/strings` - [x] No specification change - [x] `CHANGELOG.md`: not user-visible yet (no real screen) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Explanation of Change
On Android, text was losing its trailing word after a screen was re-mounted —
Submit expenses to my employerrendered asSubmit expenses to my,Your data is secureasYour data is— with no ellipsis and no change in row height. Reproducible on staging, never on production.Root cause
TextLayoutManagersizes text layouts fromceil(Layout.getDesiredWidth(...)), which sums glyph advances only. On Android 15+ a line's visual glyph bounds can exceed that sum, so the layout ends up a fraction of a pixel narrower than the text actually needs. Line breaking is a step function, so that shortfall is enough to push a trailing word onto a second line inside a view whose height was computed for one line. That line is clipped, so the word simply disappears.This is upstream #56402, still open.
Why it surfaced on 0.86 and not on 0.85.3
RN 0.86 turned
Paint.LINEAR_TEXT_FLAGon for all Android text: upstream #56409 deleted theenableAndroidLinearTextfeature flag and made both of its call sites unconditional, and that flag defaulted tofalsein OSS.LINEAR_TEXT_FLAGdisables font hinting, which shifts glyph advance widths by a fraction of a pixel and widens the advance-vs-visual-bounds gap — enough to push borderline strings over the wrapping threshold.So the faulty measurement predates 0.86; 0.86 only made it reachable for many more strings.
Upstream has attempted this fix twice already. #54721 $ #53286 in December 2025 by calling
setUseBoundsForWidth(true)on the finalStaticLayout, gated behindfixTextClippingAndroid15useBoundsForWidth— which defaulted tofalsein OSS, so 0.85.3 never ran it either. RN 0.86.0 then removed the flag and both call sites after multiline and wrapping regressions, leaving only an unusedsetUseBoundsForWidthMethodreflection handle inTextLayoutManager. The bug was refiled as #56402.Why it looked device-specific
Whether a given string lands on the wrong side of the wrapping boundary depends on the fractional residues of the layout, which come from screen width, density and font scale. On the reported Galaxy S25 FE the display density is overridden to 420 while the physical density is 450: at 450 dpi a 1080 px screen is exactly 384 dp, at 420 dpi it is 411.428571… dp, so every derived width carries a fraction. Devices left at their physical density mostly do not reproduce it, which is why most retests came back clean.
Worth calling out: #98507 (
037+fix-stale-font-scale) was previously assumed to address #98499, but both issues still reproduced on 9.4.53-3 and 9.4.54-4, which already contained that patch. It addresses a different mechanism on the C++/Yoga side and is unrelated to this one.The change
One new patch,
react-native+0.86.0+040+fix-android15-text-clipping.patch, backporting upstream #57117. It adds agetDesiredWidthhelper that, on API 35+, builds a probeStaticLayoutwithsetUseBoundsForWidth(true)and returnsmax(advanceWidth, ceil(visualBoundsWidth)), so the desired width can never shrink below the previous value. Only theAT_MOSTandUNDEFINEDmeasure modes are affected:EXACTLYstill uses the width Yoga supplies, and the final layout keeps the existing advance-based bounds mode. That narrowness is the whole point — it is what separates #57117 from the two attempts that were reverted.The patch reuses the
setUseBoundsForWidthMethodhandle that 0.86 already ships unused. The upstream diff'sTextLayoutManagerAbsoluteLayoutWithFractionalPixelTest.kthunks are omitted, because the npm package does not shipReactAndroid/src/test.Trade-offs for review
patches/react-native/details.mdwith a note to re-evaluate on the RN 0.87 upgrade and drop it once the fix lands upstream.StaticLayoutbuild per text measure on API 35+ inAT_MOST/UNDEFINEDmodes, which is a hot path.An earlier revision of this PR instead reverted
LINEAR_TEXT_FLAG(040+restore-non-linear-text). That also fixed the reported cases and was verified on the reported device, but it only removed the amplifier — the advance-based measurement stayed wrong, so borderline strings could still wrap — and it was a deliberate deviation from upstream that we would have carried indefinitely. This backport fixes the measurement itself and can be dropped when upstream lands the fix.Fixed Issues
$ #98499
$ #98690
Tests
Preconditions:
I
II
Offline tests
N/A
QA Steps
Same as Tests.
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectionAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari