Repository navigation
fix: TextInput styles can't crash a field, and NitroInput matches TextInput's layout - #70
Merged
Merged
Conversation
`TextInput` takes fontSize, fontWeight, fontFamily, color, letterSpacing,
lineHeight, textAlign and textAlignVertical from `style`. NitroInput read
them only as props and handed `style` to the view as it was, and Fabric
gives a view its style's keys as props: these share names with the
view's own, so `style={{ fontWeight: '700' }}` threw ("NitroInputView
.fontWeight: Value is a string, expected a number") and a numeric one was
fought over by the prop and the style.
The keys are now read from the flattened style under the props of the
same name (which win) and taken out of the style the view gets. textAlign
'start' is the start edge, 'end' the far one for the layout direction,
'justify' the start edge, as on a one-line TextInput.
Found by the visual comparison with TextInput.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Fabric hands a view its style's keys as props. Each view had native
props named like text style keys (fontSize, fontWeight, fontFamily,
color, letterSpacing, lineHeight, textAlign, textAlignVertical), so a
TextInput style on NitroInput, NitroNumber or NitroText reached them:
`style={{ fontWeight: '700' }}` threw "Value is a string, expected a
number", and a numeric fontSize or color was fought over by the style
and the prop.
The native props are renamed (textSize, textWeight, textFamily,
textColor, textLetterSpacing, textLineHeight, textAlignment,
textVerticalAlignment), on all three views and the measure's sizing
struct; the components' public props keep their names. With that, the
style no longer has to be stripped of those keys, so it is not.
NitroInput's `style` is typed as TextInput's, and it reads its
typography from it (the props of the same name win).
Guards:
- PropNameCollisions.test now also reads every style key React Native
knows (ReactNativeStyleAttributes) and fails if a native prop of any
view shares one, with a check that it would have caught fontWeight.
- A Harness test renders all three with a full TextInput style (string
fontWeight included) on a device.
The visual gallery is pixel-identical before and after the rename on
both platforms. All suites pass (iOS 145, Android 141); Jest 187.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Three differences from TextInput found by comparing the two side by side in a visual gallery on both platforms: - Android ignored textAlign center / right on a plain field: the text alignment set when the field became plain overrode the gravity that textAlign set later in the same commit. Both are now applied whenever either changes. - A style padding or border did not make a self-sized field bigger: an autoWidth field with paddingHorizontal 8 squeezed "blue" to "…" on iOS (React Native lays the content out inside the padding there), and on Android the padding never reached the view at all (Fabric does not hand a Nitro view its padding). JS now resolves padding plus border per edge as Yoga does and adds them to the measured size, and sends them as textInset* props, by which the Android view insets its edit text, overlay and frame. Percentages count as 0 (they need the parent). - iOS sized a single line by the font's own line box whatever lineHeight said; it now uses lineHeight, as Android does (sizing only; the drawing is unchanged). Kept, and written down in the README: digits are always tabular, and Android gets no default padding. The gallery, re-measured: Android's centred text starts where TextInput's does (81.9 vs 81.1 dp; it was 1.9), the padded field is inset like TextInput's on both, and iOS's lineHeight 30 field is 30 tall. Nothing else on the five pages moved. New Harness and Jest tests; all suites pass (iOS 147, Android 143), Jest 189. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
ronickg
changed the title
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
1 participant
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.
Found by the visual regression run that compares
NitroInputwith RN'sTextInputside by side, in a 5-page gallery on both platforms.1. A
TextInputstyle crashed a fieldstyle={{ fontWeight: '700' }}onNitroInput,NitroNumberorNitroTextthrew:Cause. Fabric hands a view its style's keys as props, and our native props had the same names as text style keys.
Fix.
textSize,textWeight,textFamily,textColor,textLetterSpacing,textLineHeight,textAlignment,textVerticalAlignment. The public props are unchanged.PropNameCollisions.testnow checks every native prop of every view against every React Native style key and view prop, with a check that it would have caughtfontWeight.TextInputstyle on a device.NitroInputnow reads its typography fromstylelikeTextInput, and the props of the same name win. Itsstyleis typed asTextInput's.2. Layout differences from
TextInputtextAlign="center"/"right"on a plain fieldTextInputat 81.1 dpstylepadding or border on a self-sized fieldTextInput'slineHeight={30}How the padding fix works:
textInset*props. React Native already insets the iOS view, but never passes padding to a Nitro view on Android, so the Android view now insets its text, overlay and frame itself.Kept as they are, now written in the README:
TextInputhas none.Tests
lineHeightfield being exactly that tall.🤖 Generated with Claude Code