Skip to content

Commit 2462efd

Browse files
sammy-SCfacebook-github-bot
sammy-SC
authored and
facebook-github-bot
committed
Fix flex-growing content sizing in <ScrollView>
Summary: Preserve available height for flex-growing subtrees nested inside scroll content. Apply optimized flex-basis measurement only when a conservative proof shows that the subtree is height-independent and connected to the nearest scroll ancestor through column-stretch layout edges. Keep authored flex-basis, overflow, padding, absolute-positioning, wrapping, and percentage-owner semantics intact across layout and measurement passes. Caching this eligibility proof is intentionally left to a separately gated follow-up. Changelog: [General][Fixed] - Preserve nested `<ScrollView>` sizing during optimized flex-basis measurement Differential Revision: D112353599
1 parent 9f3108d commit 2462efd

4 files changed

Lines changed: 554 additions & 78 deletions

File tree

‎packages/react-native/Libraries/Components/View/__tests__/View-flexBasisFitContent-itest.js‎

Lines changed: 322 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,328 @@ test('auto-height wrapper around a feed ScrollView stays bounded by the viewport
6565
expect(feedContentRect.height).toBe(FEED_CONTENT_HEIGHT);
6666
});
6767

68+
test('auto-height wrapper gives remaining ScrollView space to a flex-growing body', () => {
69+
const viewportHeight = 200;
70+
const headerHeight = 50;
71+
const root = Fantom.createRoot({
72+
viewportHeight,
73+
viewportWidth: 100,
74+
});
75+
76+
const wrapperRef = createRef<HostInstance>();
77+
const growingBodyRef = createRef<HostInstance>();
78+
79+
Fantom.runTask(() => {
80+
root.render(
81+
<ScrollView
82+
contentContainerStyle={{alignItems: 'flex-start', flexGrow: 1}}>
83+
<View collapsable={false} ref={wrapperRef}>
84+
<View collapsable={false} style={{height: headerHeight, width: 50}} />
85+
<View
86+
collapsable={false}
87+
ref={growingBodyRef}
88+
style={{flexGrow: 1, height: 40, width: 50}}
89+
/>
90+
</View>
91+
</ScrollView>,
92+
);
93+
});
94+
95+
const wrapperRect = nullthrows(wrapperRef.current).getBoundingClientRect();
96+
const growingBodyRect = nullthrows(
97+
growingBodyRef.current,
98+
).getBoundingClientRect();
99+
100+
const layout = {
101+
growingBodyHeight: growingBodyRect.height,
102+
wrapperHeight: wrapperRect.height,
103+
};
104+
105+
expect(layout).toEqual({
106+
growingBodyHeight: viewportHeight - headerHeight,
107+
wrapperHeight: viewportHeight,
108+
});
109+
});
110+
111+
test('positive flex-basis preserves content height in an unbounded ScrollView axis', () => {
112+
const root = Fantom.createRoot({
113+
viewportHeight: 200,
114+
viewportWidth: 100,
115+
});
116+
117+
const wrapperRef = createRef<HostInstance>();
118+
const contentRef = createRef<HostInstance>();
119+
120+
Fantom.runTask(() => {
121+
root.render(
122+
<ScrollView>
123+
<View collapsable={false} ref={wrapperRef} style={{flexBasis: 50}}>
124+
<View collapsable={false} ref={contentRef} style={{height: 100}} />
125+
</View>
126+
</ScrollView>,
127+
);
128+
});
129+
130+
expect({
131+
contentHeight: nullthrows(contentRef.current).getBoundingClientRect()
132+
.height,
133+
wrapperHeight: nullthrows(wrapperRef.current).getBoundingClientRect()
134+
.height,
135+
}).toEqual({
136+
contentHeight: 100,
137+
wrapperHeight: 100,
138+
});
139+
});
140+
141+
test('positive flex-basis larger than content preserves content height in an unbounded ScrollView axis', () => {
142+
const root = Fantom.createRoot({
143+
viewportHeight: 200,
144+
viewportWidth: 100,
145+
});
146+
147+
const wrapperRef = createRef<HostInstance>();
148+
const contentRef = createRef<HostInstance>();
149+
150+
Fantom.runTask(() => {
151+
root.render(
152+
<ScrollView>
153+
<View collapsable={false} ref={wrapperRef} style={{flexBasis: 100}}>
154+
<View collapsable={false} ref={contentRef} style={{height: 40}} />
155+
</View>
156+
</ScrollView>,
157+
);
158+
});
159+
160+
const layout = {
161+
contentHeight: nullthrows(contentRef.current).getBoundingClientRect()
162+
.height,
163+
wrapperHeight: nullthrows(wrapperRef.current).getBoundingClientRect()
164+
.height,
165+
};
166+
167+
expect(layout).toEqual({
168+
contentHeight: 40,
169+
wrapperHeight: 40,
170+
});
171+
});
172+
173+
test('nested padding respects an exhausted ScrollView height constraint', () => {
174+
const viewportHeight = 100;
175+
const root = Fantom.createRoot({
176+
viewportHeight,
177+
viewportWidth: 100,
178+
});
179+
180+
const wrapperRef = createRef<HostInstance>();
181+
const nestedRef = createRef<HostInstance>();
182+
const paddedRef = createRef<HostInstance>();
183+
184+
Fantom.runTask(() => {
185+
root.render(
186+
<ScrollView contentContainerStyle={{height: viewportHeight}}>
187+
<View
188+
collapsable={false}
189+
ref={wrapperRef}
190+
style={{paddingBottom: 50, paddingTop: 50}}>
191+
<View collapsable={false} ref={nestedRef}>
192+
<View
193+
collapsable={false}
194+
ref={paddedRef}
195+
style={{paddingBottom: 10, paddingTop: 10}}
196+
/>
197+
</View>
198+
</View>
199+
</ScrollView>,
200+
);
201+
});
202+
203+
expect({
204+
nestedHeight: nullthrows(nestedRef.current).getBoundingClientRect().height,
205+
paddedHeight: nullthrows(paddedRef.current).getBoundingClientRect().height,
206+
wrapperHeight: nullthrows(wrapperRef.current).getBoundingClientRect()
207+
.height,
208+
}).toEqual({
209+
nestedHeight: 0,
210+
paddedHeight: 20,
211+
wrapperHeight: viewportHeight,
212+
});
213+
});
214+
215+
test('clipped ScrollView inside a horizontal pager stays scrollable', () => {
216+
const root = Fantom.createRoot({
217+
viewportHeight: 150,
218+
viewportWidth: 320,
219+
});
220+
221+
const outerScrollRef = createRef<HostInstance>();
222+
const boundedContainerRef = createRef<HostInstance>();
223+
const autoHeightWrapperRef = createRef<HostInstance>();
224+
const innerScrollRef = createRef<HostInstance>();
225+
226+
Fantom.runTask(() => {
227+
root.render(
228+
<ScrollView horizontal pagingEnabled ref={outerScrollRef}>
229+
<View
230+
collapsable={false}
231+
ref={boundedContainerRef}
232+
style={{height: 150, overflow: 'hidden', width: 320}}>
233+
<View collapsable={false} ref={autoHeightWrapperRef}>
234+
<ScrollView ref={innerScrollRef}>
235+
<View style={{height: 240}} />
236+
</ScrollView>
237+
</View>
238+
</View>
239+
<View style={{height: 150, width: 320}} />
240+
</ScrollView>,
241+
);
242+
});
243+
244+
const outerScroll = nullthrows(outerScrollRef.current);
245+
const innerScroll = nullthrows(innerScrollRef.current);
246+
const layout = {
247+
autoHeightWrapperHeight: nullthrows(
248+
autoHeightWrapperRef.current,
249+
).getBoundingClientRect().height,
250+
boundedContainerHeight: nullthrows(
251+
boundedContainerRef.current,
252+
).getBoundingClientRect().height,
253+
innerContentHeight: innerScroll.scrollHeight,
254+
innerHeight: innerScroll.getBoundingClientRect().height,
255+
innerScrollRange: innerScroll.scrollHeight - innerScroll.clientHeight,
256+
outerHorizontalScrollRange:
257+
outerScroll.scrollWidth - outerScroll.clientWidth,
258+
outerVerticalScrollRange:
259+
outerScroll.scrollHeight - outerScroll.clientHeight,
260+
};
261+
262+
expect(layout).toEqual({
263+
autoHeightWrapperHeight: 150,
264+
boundedContainerHeight: 150,
265+
innerContentHeight: 240,
266+
innerHeight: 150,
267+
innerScrollRange: 90,
268+
outerHorizontalScrollRange: 320,
269+
outerVerticalScrollRange: 0,
270+
});
271+
});
272+
273+
test('percentage padding tracks an updated owner width', () => {
274+
const root = Fantom.createRoot({
275+
viewportHeight: 300,
276+
viewportWidth: 300,
277+
});
278+
279+
function render(updated: boolean) {
280+
Fantom.runTask(() => {
281+
root.render(
282+
<View
283+
collapsable={false}
284+
style={{
285+
height: 200,
286+
overflow: 'scroll',
287+
width: updated ? 200 : 100,
288+
}}>
289+
<View collapsable={false} style={updated ? {height: 150} : undefined}>
290+
<View
291+
collapsable={false}
292+
nativeID="candidate"
293+
style={{maxWidth: 50, paddingLeft: '25%'}}
294+
/>
295+
</View>
296+
</View>,
297+
);
298+
});
299+
}
300+
301+
render(false);
302+
render(true);
303+
304+
expect(
305+
root
306+
.getRenderedOutput({
307+
includeLayoutMetrics: true,
308+
props: ['layoutMetrics-contentInsets', 'nativeID'],
309+
})
310+
.toJSX(),
311+
).toEqual(
312+
<rn-view layoutMetrics-contentInsets="{top:0,right:0,bottom:0,left:0}">
313+
<rn-view layoutMetrics-contentInsets="{top:0,right:0,bottom:0,left:0}">
314+
<rn-view
315+
layoutMetrics-contentInsets="{top:0,right:0,bottom:0,left:50}"
316+
nativeID="candidate"
317+
/>
318+
</rn-view>
319+
</rn-view>,
320+
);
321+
});
322+
323+
test('native box sizing update invalidates cached intrinsic width', () => {
324+
const root = Fantom.createRoot({
325+
viewportHeight: 280,
326+
viewportWidth: 240,
327+
});
328+
const scrollRef = createRef<HostInstance>();
329+
const leafRef = createRef<HostInstance>();
330+
331+
const render = (siblingHeight: number) => {
332+
Fantom.runTask(() => {
333+
root.render(
334+
<View
335+
collapsable={false}
336+
style={{
337+
alignItems: 'flex-start',
338+
flexDirection: 'row',
339+
height: 240,
340+
width: 181,
341+
}}>
342+
<View
343+
collapsable={false}
344+
style={{alignItems: 'flex-start', alignSelf: 'flex-start'}}>
345+
<ScrollView ref={scrollRef} style={{height: 200}}>
346+
<View collapsable={false} style={{height: 100}}>
347+
<View
348+
collapsable={false}
349+
style={{flexBasis: 100, flexShrink: 1}}>
350+
<View collapsable={false}>
351+
<View
352+
collapsable={false}
353+
ref={leafRef}
354+
style={{
355+
borderLeftWidth: 1,
356+
borderRightWidth: 2,
357+
paddingHorizontal: 3,
358+
width: 83,
359+
}}
360+
/>
361+
</View>
362+
</View>
363+
<View style={{height: siblingHeight, width: 17}} />
364+
</View>
365+
</ScrollView>
366+
</View>
367+
</View>,
368+
);
369+
});
370+
};
371+
372+
render(27);
373+
Fantom.runTask(() => {
374+
nullthrows(leafRef.current).setNativeProps({
375+
style: {boxSizing: 'content-box'},
376+
});
377+
});
378+
render(40);
379+
render(53);
380+
381+
expect({
382+
leafWidth: nullthrows(leafRef.current).getBoundingClientRect().width,
383+
scrollWidth: nullthrows(scrollRef.current).getBoundingClientRect().width,
384+
}).toEqual({
385+
leafWidth: 92,
386+
scrollWidth: 92,
387+
});
388+
});
389+
68390
const styles = StyleSheet.create({
69391
feed: {
70392
width: VIEWPORT_WIDTH,

‎packages/react-native/ReactCommon/react/renderer/components/view/YogaLayoutableShadowNode.cpp‎

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,18 @@ static int FabricDefaultYogaLog(
6464

6565
thread_local LayoutContext threadLocalLayoutContext;
6666

67+
static bool yogaStylesEqualForConfig(
68+
const yoga::Node& node,
69+
const yoga::Style& first,
70+
const yoga::Style& second) {
71+
const auto config = node.getConfig();
72+
const bool compareBoxSizing = config != nullptr &&
73+
config->isExperimentalFeatureEnabled(
74+
yoga::ExperimentalFeature::FixFlexBasisFitContent);
75+
return first == second &&
76+
(!compareBoxSizing || first.boxSizing() == second.boxSizing());
77+
}
78+
6779
YogaLayoutableShadowNode::YogaLayoutableShadowNode(
6880
const ShadowNodeFragment& fragment,
6981
const ShadowNodeFamily::Shared& family,
@@ -378,7 +390,10 @@ void YogaLayoutableShadowNode::updateYogaChildren() {
378390
yogaLayoutableChildren_.at(yogaChildIndex)->yogaNode_;
379391

380392
isClean = isClean && !newYogaChildNode.isDirty() &&
381-
(newYogaChildNode.style() == oldYogaChildNode.style());
393+
yogaStylesEqualForConfig(
394+
newYogaChildNode,
395+
newYogaChildNode.style(),
396+
oldYogaChildNode.style());
382397
}
383398
}
384399
}
@@ -397,7 +412,8 @@ void YogaLayoutableShadowNode::updateYogaProps() {
397412

398413
// Resetting `dirty` flag only if `yogaStyle` portion of `Props` was
399414
// changed.
400-
if (!YGNodeIsDirty(&yogaNode_) && (styleResult != yogaNode_.style())) {
415+
if (!YGNodeIsDirty(&yogaNode_) &&
416+
!yogaStylesEqualForConfig(yogaNode_, styleResult, yogaNode_.style())) {
401417
yogaNode_.setDirty(true);
402418
}
403419

‎packages/react-native/ReactCommon/yoga/yoga/YGNodeStyle.cpp‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,13 +33,25 @@ void updateStyle(YGNodeRef node, IdxT idx, ValueT value) {
3333
}
3434
}
3535

36+
bool stylesEqualForConfig(
37+
const Node* node,
38+
const Style& first,
39+
const Style& second) {
40+
const auto config = node->getConfig();
41+
const bool compareBoxSizing = config != nullptr &&
42+
config->isExperimentalFeatureEnabled(
43+
ExperimentalFeature::FixFlexBasisFitContent);
44+
return first == second &&
45+
(!compareBoxSizing || first.boxSizing() == second.boxSizing());
46+
}
47+
3648
} // namespace
3749

3850
void YGNodeCopyStyle(YGNodeRef dstNode, YGNodeConstRef srcNode) {
3951
auto dst = resolveRef(dstNode);
4052
auto src = resolveRef(srcNode);
4153

42-
if (dst->style() != src->style()) {
54+
if (!stylesEqualForConfig(dst, dst->style(), src->style())) {
4355
dst->setStyle(src->style());
4456
dst->markDirtyAndPropagate();
4557
}

0 commit comments

Comments
 (0)