Repository navigation
Apply the reported embedded content height in the JS component - #778
Merged
Merged
Conversation
Holds the height reported by native in state and applies it as the view's height when the caller has not stated one. A style that fixes the height in any form - height, minHeight, maxHeight, aspectRatio, flex or flexBasis - keeps it, and the view fills that box as before. Converts the component from a class to a function so the measured height can live in state. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
crow
approved these changes
Oct 14, 2026
| config={JSON.stringify({ embeddedId, selection })} | ||
| /> | ||
| ); | ||
| export function isSelfSizing(style?: ViewStyle): boolean { |
Contributor
There was a problem hiding this comment.
This makes sizing to content the default whenever no height is set. I think that changes layouts that get their height from the parent, like a stretched child in a fixed-height row, or absolute with top and bottom, since they'd now get pinned to the content height. Might be safer as an opt-in prop?
Contributor
Author
There was a problem hiding this comment.
Switched to an opt-in sizeToContent prop, default off, so behaviour is unchanged for existing callers. Widening the detection first — alignSelf: 'stretch', absolute top and bottom — didn't hold up, because a height from alignItems on the parent isn't visible from the child's style at all, so the inference is gone rather than extended.
A style can get its height from the parent without stating one: alignSelf stretch, or absolute positioning pinned top and bottom. Both were treated as self sizing, so the content height overrode the height the parent gave them. Also adds flexGrow, which was missed alongside flex and flexBasis. A height the parent imposes from its own side, such as alignItems stretch on the row above, is not visible from the child's style and is still not detected. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Self sizing was inferred from the style: if no height was stated, the view sized itself. The inference cannot be made correct. A height can come from the parent - alignItems stretch on the row above - and that is not visible from the child's style, so a view the layout had already sized was pinned to its content height instead. Callers now ask for it with sizeToContent, and the default leaves behaviour as it was. Two fixes that came out of the same pass: - The style is typed StyleProp<ViewStyle> and the height is appended as an array entry rather than spread into an object. Spreading an array of styles produced numeric keys and dropped every property the caller set. - The component is wrapped in forwardRef. It was exported as a class, so a caller holding a ref got null once it became a function. isSelfSizing and resolveStyle are no longer exported; nothing outside this module used them, and src/index.tsx re-exports with a star. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The inferred ref type resolves to a react-native internal path that cannot be named from the emitted .d.ts, so bob's typescript target failed with TS2742. Naming the instance type and annotating the component keeps the emit portable. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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
2 participants
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.
What do these changes do?
Add a
sizeToContentprop. When it is set, the view takes the height of itscontent instead of filling the box it is given, using the height native
reports through
onSizeChange.The default is off, so an app that does not set it behaves exactly as before.
Why are these changes necessary?
Native can now measure the embedded content, but nothing consumed the height,
so the view still filled whatever box it was given.
How did you verify these changes?
Ran the example app on an iOS simulator and an Android emulator against a
Scene that renders at its natural height. The view matches the content on both,
and dismissing the content collapses it instead of leaving a stale gap behind.
Typecheck passes with no errors in
src.Anything else a reviewer should know?
An earlier version of this branch inferred self sizing from the style: no
height stated meant size to content. That is not decidable from the child's
style, because a height can come from the parent —
alignItems: 'stretch'onthe row above — and overriding it changed layouts nobody had touched. Hence
the explicit prop.
Two fixes from the same pass:
The
styleprop is nowStyleProp<ViewStyle>and the height is appended as anarray entry instead of spread into an object. Spreading an array of styles
turned the caller's entries into numeric keys and dropped them all.
The component is wrapped in
forwardRef. It was exported as a class, so acaller holding a ref would have got null once it became a function.
🤖 Generated with Claude Code