fix(ios): keep padding out of textContainerInset and write it on the label's own layout pass - #33
Open
edusperoni wants to merge 1 commit into
Conversation
…label's own layout pass NLabel.drawText already insets by padding and borderThickness, which core sets synchronously. textContainerInset now only carries the vertical-alignment correction on the UILabel path, and onLayout forces the update because core's isLayoutValid is false for the whole pass a label requested itself. Fixes nativescript-community#29
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 #29. Supersedes #32, which removes the wrong half of the duplication (see below).
Symptoms
On the
UILabelpath (non-selectable labels) padding is drawn once on the first render and twice after certain relayouts: recycledCollectionViewcells (#29), or siblings shifting when a neighbouring label is restyled. With #32 applied the failure flips: the same labels draw with no padding until such a relayout happens, which is very visible on left-aligned text.Root cause
There are two inset sources on the native
NLabel, and only one of them is reliable.padding/borderThicknessare written synchronously by@nativescript/core'sLabelsetters (paddingInternalProperty,border*WidthProperty), which this plugin forwards tosuperwhen it is not usingUITextView.NLabel.swiftdeclares the same properties asTNSLabel, so core's setters land on it andtextRect(forBounds:)sizes with them.textContainerInsetis written byupdateVerticalAlignment()fromonLayoutaspadding + border (+ vertical-alignment correction).Since 2.0.0
NLabel.drawText(in:)insets by all three, so whenever (2) has been written the padding is applied twice.But (2) is only written on some layout passes.
updateVerticalAlignment()returns early when!this.isLayoutValid. On iOS core definesisLayoutValidas "the force-layout flag is clear", andView.layout()(ui/core/view/index.ios.ts) clears that flag afteronLayoutreturns. So on any pass the label requested itself, including its first one,onLayoutseesisLayoutValid === falseandtextContainerInsetstays at its previous value (zero for a fresh label). It only gets written when an ancestor drives the relayout and the label's bounds or measure spec change without the label having calledrequestLayout(). That is exactly the recycled-cell and sibling-shift cases.So:
textContainerInset+padding+border)textContainerInsetonly)Change
updateTextContainerInset(): on theUILabelpath the returned inset now carries only the vertical-alignment correction;paddingandborderare already applied natively from core's synchronous setters. TheUITextViewpath is unchanged, since theretextContainerInsetis the real native property and the only inset source.updateVerticalAlignment(applyVerticalTextAlignment, force): newforceflag that bypasses theisLayoutValidguard.onLayoutpasses it, because the frame is already applied at that point and it is the only reliable place to write the inset. The guard still protects the early calls fromonResumeNativeUpdatesand the property setters, where bounds are zero.onLayoutnow always ends with a forcedupdateVerticalAlignment(afterupdateAutoFontSizewhen auto font size is on), so the correction is also refreshed for auto-sized labels.NLabel.swiftis untouched: the 2.0.x draw path is correct once the JS side stops duplicating padding intotextContainerInset.Testing
Applied as a
patch-packagepatch to@nativescript-community/ui-label@2.0.1in a NativeScript 9.1 / Angular app that hit both symptoms (tab-set labels ellipsizing after a tap, left-aligned padded labels losing their inset with #32). Device verification of the patch is in progress; I'll report back here. No vertical-text-align usage in that app, so review of thetop/bottomcorrection math would be appreciated from anyone relying on it.