Repository navigation
Conversation
MarginBottomSpan makes a lone "\n" spacer line exactly marginBottom tall, and LineHeightSpan grows any line with spare leading up to lineHeight. Both edit the line's FontMetricsInt in place, so the spacer's height depends on which span runs last. Android does not call them in the same order for the shadow node's StaticLayout and for the TextView's own layout, so Yoga can size the view from short spacers while the TextView draws tall ones, and the last line ends up below the view's bottom edge. LineHeightSpan now leaves spacer lines alone, which makes the result independent of span order.
3 tasks done
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Fixes #1060
What/Why?
A spacer line (a lone
"\n"carrying aMarginBottomSpan) getsLineHeightSpanfrom the surrounding text as well. Both spans rewrite the sameFontMetricsIntinchooseHeight, so the spacer's height depends on which one runs last. Android does not call them in the same order for the shadow node'sStaticLayoutand for theTextView's own layout. Yoga then sizes the view withmarginBottom-tall spacers while theTextViewdraws them almost a full line tall, and the last line is drawn below the view's bottom edge. The details are in #1060.LineHeightSpan.chooseHeightnow returns early for spacer lines, soMarginBottomSpanalone decides their height and span order no longer matters. Other lines behave as before, including the existing early return for lines already taller thanlineHeight(#643).The change is limited to the React Native package.
packages/enriched-markdown-android/uihas an identicalLineHeightSpan, but I have not checked whether its layout paths ever call the two spans in different orders, so I left it unchanged.Testing
SpacerLineHeightTestruns both spans over a spacer line in both orders and checks that the spacer ismarginBottomtall each time. It also checks that ordinary lines still getlineHeight. Onmain, the order-swap case fails: the spacer comes out 60 px instead of 10 px. With this change both tests pass.:react-native-enriched-markdown:testDebugUnitTestthe usual way, because it needs the example app's Gradle build, and that needsnode_modules, which I did not install. Instead I compiled the two span files and the test in a minimal standalone Android library project: AGP 9.2.1, Kotlin 2.2.10,react-android0.88.0, Robolectric 4.14.1, offline. Result:SpacerLineHeightTest, 2 tests, 0 failures. I usedsdk = [28]in the test config, asEmptyListAnchorFormattingTestdoes, because that Robolectric runtime was already available offline here. I am happy to switch it to 30 to match the other tests.PR Checklist