react-native: [0.71.0-rc.0] Gap, flexWrap & alignContent renders wrong size
Description
When using gap, flexWrap and alignContent, size is miscalculated, because gap isn’t taken into account when computing the sizes, so if any wrapping happens because of gap, it is not computed properly
Please note that I have tried with and without legacyStretchBehavior enabled (by patching react-native).
Version
0.71.0-rc.0
Output of npx react-native info
System: OS: macOS 12.6 CPU: (8) arm64 Apple M1 Memory: 155.78 MB / 16.00 GB Shell: 5.8.1 - /bin/zsh Binaries: Node: 18.11.0 - /usr/local/bin/node Yarn: 1.22.17 - /opt/homebrew/bin/yarn npm: 8.19.2 - /usr/local/bin/npm Watchman: 2022.10.10.00 - /opt/homebrew/bin/watchman Managers: CocoaPods: 1.11.3 - /opt/homebrew/bin/pod SDKs: iOS SDK: Platforms: DriverKit 21.4, iOS 15.5, macOS 12.3, tvOS 15.4, watchOS 8.5 Android SDK: Not Found IDEs: Android Studio: 2021.3 AI-213.7172.25.2113.9123335 Xcode: 13.4/13F17a - /usr/bin/xcodebuild Languages: Java: 11.0.15 - /usr/bin/javac npmPackages: @react-native-community/cli: Not Found react: 18.2.0 => 18.2.0 react-native: 0.71.0-rc.0 => 0.71.0-rc.0 react-native-macos: Not Found npmGlobalPackages: react-native: Not Found
Steps to reproduce
In order to reproduce you need to have:
- 1 container with flexWrap: ‘wrap’ & alignContent: ‘stretch’ & gap: someValue
- Children that wrap only because of the gap property (they would fit in the container otherwise)
When that happens the size calculated by alignContent: ’stretch’ doesn’t take into account the wrap, so each child will have the full container size instead of half .
Example:
- Container of 300 x 300 direction ‘row’, flexWrap: ‘wrap’, alignContent: ‘stretch’
- 5 children of width: 60, flexGrow: 1
Before adding gap: 1 After adding gap: 1
Note that if the child where already wrapping (example width: 61) this would not happen. It only happens if the wrapping is caused by gap
Snack, code example, screenshot, or link to a repository
Web equivalent: https://jsfiddle.net/t8q6xevj/1/ Native Code used to test:
Feel free to alter gap of container / width of children
import React from 'react';
import {
SafeAreaView,
View,
} from 'react-native';
function App(): JSX.Element {
return (
<SafeAreaView style={{flexGrow: 1, alignItems: 'center', justifyContent: 'center'}}>
<View
style={{
height: 300,
width: 300,
backgroundColor: 'blue',
flexWrap: 'wrap',
flexDirection: 'row',
alignContent: 'stretch',
gap: 1,
}}>
<View
style={redChild}></View>
<View
style={greenChild}></View>
<View
style={redChild}></View>
<View
style={greenChild}></View>
<View
style={redChild}></View>
</View>
</SafeAreaView>
);
}
const child = {
flexGrow: 1,
width: 60,
}
const redChild = {
...child,
backgroundColor: 'red',
};
const greenChild = {
...child,
backgroundColor: 'green',
}
export default App;
About this issue
- Original URL
- State: closed
- Created 2 years ago
- Reactions: 2
- Comments: 19 (15 by maintainers)
Commits related to this issue
- Incorporate gap space into main axis overflow flag Summary: In https://github.com/facebook/react-native/issues/35351 we see incorrect child item height when the flex-wrap is enabled, the cross-axis i... — committed to NickGerleman/yoga by NickGerleman 2 years ago
- Incorporate gap space into main axis overflow flag (#1173) Summary: X-link: https://github.com/facebook/yoga/pull/1173 In https://github.com/facebook/react-native/issues/35351 we see incorrect child... — committed to facebook/litho by NickGerleman 2 years ago
- Incorporate gap space into main axis overflow flag (#1173) Summary: Pull Request resolved: https://github.com/facebook/yoga/pull/1173 In https://github.com/facebook/react-native/issues/35351 we see ... — committed to facebook/yoga by NickGerleman 2 years ago
- Incorporate gap space into main axis overflow flag (#1173) Summary: X-link: https://github.com/facebook/yoga/pull/1173 In https://github.com/facebook/react-native/issues/35351 we see incorrect child... — committed to facebook/react-native by NickGerleman 2 years ago
- Incorporate gap space into main axis overflow flag (#1173) Summary: X-link: https://github.com/facebook/yoga/pull/1173 In https://github.com/facebook/react-native/issues/35351 we see incorrect child... — committed to facebook/react-native by NickGerleman 2 years ago
- Incorporate gap space into main axis overflow flag (#1173) Summary: X-link: https://github.com/facebook/yoga/pull/1173 In https://github.com/facebook/react-native/issues/35351 we see incorrect child... — committed to facebook/react-native by NickGerleman 2 years ago
- Incorporate gap space into main axis overflow flag (#1173) Summary: X-link: https://github.com/facebook/yoga/pull/1173 In https://github.com/facebook/react-native/issues/35351 we see incorrect child... — committed to OlimpiaZurek/react-native by NickGerleman 2 years ago
I also think we should not gate it, but warn users that if it doesn’t match the spec there will be breaking changes might be a good idea
PR here: https://github.com/facebook/yoga/pull/1173 New UT and previous ones are now passing.
Maybe it’s worth adding a disclaimer about
gap
in the release notes too. Warn people it may have bugs, and that fixing those bugs later on may break their layoutsYeah, it looks like
totalOuterFlexBasis
, used for overflow logic, is more than just flex basis, it also includes margins already. It is also used for setting main axis measure mode to parent width instead of minimum size if we have overflow.I think the right solution here is to rename this, and incorporate gap spacing into its checks.
Adding this fixes the issue!
Still happens with just
columnGap
. I have a fixture working which causes a failing unit test.Planning to dig in a bit, since I know running these UTs is currently a giant pain in OSS (I thought I would have added the CMake baed UTs a while back but have been awful about maintaining focus recently). But should be manually reproable as well.