-
Notifications
You must be signed in to change notification settings - Fork 50
fix(android): unify line break strategy across measuring and drawing, add line break props #374
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
7d49682
b495446
57b21fe
3fd78eb
5d2a438
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,39 @@ | ||
| package com.swmansion.enriched.markdown.utils.common | ||
|
|
||
| import android.text.Layout | ||
|
|
||
| /** | ||
| * Singleton that resolves the break strategy for StaticLayout and TextView. | ||
| * | ||
| * Both Measurement (via MeasurementStore) and the render (via rendered TextView) | ||
| * must use the same value - a mismatch causes the measured line count to differ | ||
| * from the rendered line count, which results in the view being sized incorrectly | ||
| * and ScrollingMovementMethod (inherited via LinkMovementMethod) silently | ||
| * scrolling the overflow. | ||
| * | ||
| * The strategy is set from the `textBreakStrategy` prop via the view's setter, | ||
| * which updates this object before invalidating measurement and triggering a | ||
| * re-render. Both MeasurementStore (StaticLayout.Builder) and TextViewSetup | ||
| * (TextView.breakStrategy) call resolveBreakStrategy(), so updating it here | ||
| * is enough for both paths. | ||
| * | ||
| * Note: call sites in StaticLayout.Builder suppress "WrongConstant" lint. This is | ||
| * intentional - Layout.BREAK_STRATEGY_SIMPLE and LineBreaker.BREAK_STRATEGY_SIMPLE | ||
| * are the same integer (0), but the @IntDef annotation on StaticLayout.Builder | ||
| * .setBreakStrategy() was changed from Layout.* to LineBreaker.* in API 29. | ||
| * The suppression is safe; the Layout.* constants share the same integer values. | ||
| */ | ||
| object BreakStrategyUtils { | ||
| private var strategy: String = "simple" | ||
|
|
||
|
eszlamczyk marked this conversation as resolved.
|
||
| fun setStrategy(newStrategy: String?) { | ||
| strategy = newStrategy ?: "simple" | ||
| } | ||
|
eszlamczyk marked this conversation as resolved.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
RN Could we follow the existing |
||
|
|
||
| fun resolveBreakStrategy(): Int = | ||
| when (strategy) { | ||
| "highQuality" -> Layout.BREAK_STRATEGY_HIGH_QUALITY | ||
| "balanced" -> Layout.BREAK_STRATEGY_BALANCED | ||
| else -> Layout.BREAK_STRATEGY_SIMPLE | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,6 +3,20 @@ | |
|
|
||
| NSAttributedString *kNewlineAttributedString; | ||
| static NSParagraphStyle *kBlockSpacerTemplate; | ||
| static NSLineBreakStrategy gLineBreakStrategy = NSLineBreakStrategyNone; | ||
|
|
||
| void ENRMSetLineBreakStrategy(NSString *strategy) | ||
| { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This file already has statics like
Font scaling is already stored per view here; RN |
||
| if ([strategy isEqualToString:@"standard"]) { | ||
|
eszlamczyk marked this conversation as resolved.
|
||
| gLineBreakStrategy = NSLineBreakStrategyStandard; | ||
| } else if ([strategy isEqualToString:@"hangul-word"]) { | ||
| gLineBreakStrategy = NSLineBreakStrategyHangulWordPriority; | ||
| } else if ([strategy isEqualToString:@"push-out"]) { | ||
| gLineBreakStrategy = NSLineBreakStrategyPushOut; | ||
| } else { | ||
| gLineBreakStrategy = NSLineBreakStrategyNone; | ||
| } | ||
| } | ||
|
|
||
| __attribute__((constructor)) static void initParagraphStyleUtils(void) | ||
| { | ||
|
|
@@ -25,6 +39,7 @@ NSWritingDirection currentWritingDirection(void) | |
| NSParagraphStyle *existing = [output attribute:NSParagraphStyleAttributeName atIndex:index effectiveRange:NULL]; | ||
| NSMutableParagraphStyle *style = existing ? [existing mutableCopy] : [[NSMutableParagraphStyle alloc] init]; | ||
| style.baseWritingDirection = currentWritingDirection(); | ||
| style.lineBreakStrategy = gLineBreakStrategy; | ||
| return style; | ||
|
eszlamczyk marked this conversation as resolved.
|
||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -213,4 +213,26 @@ export interface EnrichedMarkdownTextProps extends Omit<ViewProps, 'style'> { | |
| * @platform web | ||
| */ | ||
| dir?: 'ltr' | 'rtl' | 'auto'; | ||
| /** | ||
| * Sets the text break strategy on Android (API 23+). | ||
| * - `'simple'`: no hyphenation, minimal line-break work. | ||
| * - `'highQuality'` (default): full paragraph optimization with hyphenation. | ||
| * - `'balanced'`: balances line lengths, no hyphenation. | ||
|
eszlamczyk marked this conversation as resolved.
|
||
| * | ||
| * Both the measurement pass and the render pass use this value so that | ||
| * measured line counts match rendered line counts. | ||
| * @default 'highQuality' | ||
| * @platform android | ||
| */ | ||
| textBreakStrategy?: 'simple' | 'highQuality' | 'balanced'; | ||
| /** | ||
| * Sets the line break strategy on iOS (iOS 14+). | ||
| * - `'none'` (default): no additional line break strategy. | ||
| * - `'standard'`: standard line breaking rules. | ||
| * - `'hangul-word'`: Korean word-boundary breaking. | ||
| * - `'push-out'`: pushes text out to avoid orphaned words. | ||
| * @default 'none' | ||
| * @platform ios | ||
| */ | ||
| lineBreakStrategyIOS?: 'none' | 'standard' | 'hangul-word' | 'push-out'; | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could we update API_RENCE doc with these props? 🙏 |
||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The default/fallback here is still
"simple", but managers, codegen, and the JS wrapper all default to"highQuality". Let's use 'highQuality' as default.