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 Simulator Screen Shot - iPhone 13 - 2022-11-15 at 15 06 01 After adding gap: 1 Simulator Screen Shot - iPhone 13 - 2022-11-15 at 15 08 14

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 Simulator Screen Shot - iPhone 13 - 2022-11-15 at 15 09 40

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

Most upvoted comments

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 layouts

Yeah, 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.

 currentRelativeChild->getGapForAxis(crossAxis, availableInnerCrossDim).isUndefined()

Adding this fixes the issue!

Screenshot 2022-11-15 at 10 59 52 PM

Still happens with just columnGap. I have a fixture working which causes a failing unit test.

image

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.