Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixup hack for flex line size calculation #1380

Closed
wants to merge 1 commit into from

Conversation

NickGerleman
Copy link
Contributor

Summary:
X-link: facebook/react-native#39433

Back when rolling out flex gap, we encountered a bug where gap was added to the end of the main axis when a size was not specified.

During flex line justification/sizing, we calculate the amount of space that should be in between children. We erroneously add this, even after the last child element.

For justify-content, this space between children is derived from free space along the axis. The only time we have free space is if we had a dimension/dimension constraint already set on the parent. In this case, the extra space added to the end of the flex line is never noticed, becasue we bound maxLineMainDim to container dimension constraints at the end of layout, and the error doesn't effect how any children are positioned or sized.

betweenMainDim may still be set for gap even if we don't have a sized parent, which makes the extra space propagated to maxLineMainDim effect parent size.

Because we were in a code freeze, I opted to have us go with a solution just effecting flex gap, instead of the right one, in case there were any side effects. This cleans up the code to use the right calculation everywhere, and fixes a separate bug, where endOfLineIndex and startOfLineIndex may not be the last/first in the line if they are out of the layout flow (absolutely positioned, or display: none_

See the original conversation on #1188

Differential Revision: D49260049

@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D49260049

@NickGerleman NickGerleman mentioned this pull request Sep 14, 2023
NickGerleman added a commit to NickGerleman/yoga that referenced this pull request Sep 14, 2023
Summary:

X-link: facebook/react-native#39433

Back when rolling out flex gap, we encountered a bug where gap was added to the end of the main axis when a size was not specified.

During flex line justification/sizing, we calculate the amount of space that should be in between children. We erroneously add this, even after the last child element.

For `justify-content`, this space between children is derived from free space along the axis. The only time we have free space is if we had a dimension/dimension constraint already set on the parent. In this case, the extra space added to the end of the flex line is never noticed, becasue we bound `maxLineMainDim` to container dimension constraints at the end of layout, and the error doesn't effect how any children are positioned or sized.

`betweenMainDim` may still be set for `gap` even if we don't have a sized parent, which makes the extra space propagated to `maxLineMainDim` effect parent size.

Because we were in a code freeze, I opted to have us go with a solution just effecting flex gap, instead of the right one, in case there were any side effects. This cleans up the code to use the right calculation everywhere, and fixes a separate bug, where `endOfLineIndex` and `startOfLineIndex` may not be the last/first in the line if they are out of the layout flow (absolutely positioned, or display: none_

See the original conversation on facebook#1188

Reviewed By: javache

Differential Revision: D49260049
NickGerleman added a commit to NickGerleman/react-native that referenced this pull request Sep 14, 2023
Summary:
X-link: facebook/yoga#1380


Back when rolling out flex gap, we encountered a bug where gap was added to the end of the main axis when a size was not specified.

During flex line justification/sizing, we calculate the amount of space that should be in between children. We erroneously add this, even after the last child element.

For `justify-content`, this space between children is derived from free space along the axis. The only time we have free space is if we had a dimension/dimension constraint already set on the parent. In this case, the extra space added to the end of the flex line is never noticed, becasue we bound `maxLineMainDim` to container dimension constraints at the end of layout, and the error doesn't effect how any children are positioned or sized.

`betweenMainDim` may still be set for `gap` even if we don't have a sized parent, which makes the extra space propagated to `maxLineMainDim` effect parent size.

Because we were in a code freeze, I opted to have us go with a solution just effecting flex gap, instead of the right one, in case there were any side effects. This cleans up the code to use the right calculation everywhere, and fixes a separate bug, where `endOfLineIndex` and `startOfLineIndex` may not be the last/first in the line if they are out of the layout flow (absolutely positioned, or display: none_

See the original conversation on facebook/yoga#1188

Reviewed By: javache

Differential Revision: D49260049
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D49260049

NickGerleman added a commit to NickGerleman/yoga that referenced this pull request Sep 14, 2023
Summary:

X-link: facebook/react-native#39433

Back when rolling out flex gap, we encountered a bug where gap was added to the end of the main axis when a size was not specified.

During flex line justification/sizing, we calculate the amount of space that should be in between children. We erroneously add this, even after the last child element.

For `justify-content`, this space between children is derived from free space along the axis. The only time we have free space is if we had a dimension/dimension constraint already set on the parent. In this case, the extra space added to the end of the flex line is never noticed, becasue we bound `maxLineMainDim` to container dimension constraints at the end of layout, and the error doesn't effect how any children are positioned or sized.

`betweenMainDim` may still be set for `gap` even if we don't have a sized parent, which makes the extra space propagated to `maxLineMainDim` effect parent size.

Because we were in a code freeze, I opted to have us go with a solution just effecting flex gap, instead of the right one, in case there were any side effects. This cleans up the code to use the right calculation everywhere, and fixes a separate bug, where `endOfLineIndex` and `startOfLineIndex` may not be the last/first in the line if they are out of the layout flow (absolutely positioned, or display: none_

See the original conversation on facebook#1188

Reviewed By: javache

Differential Revision: D49260049
NickGerleman added a commit to NickGerleman/react-native that referenced this pull request Sep 14, 2023
Summary:
X-link: facebook/yoga#1380


Back when rolling out flex gap, we encountered a bug where gap was added to the end of the main axis when a size was not specified.

During flex line justification/sizing, we calculate the amount of space that should be in between children. We erroneously add this, even after the last child element.

For `justify-content`, this space between children is derived from free space along the axis. The only time we have free space is if we had a dimension/dimension constraint already set on the parent. In this case, the extra space added to the end of the flex line is never noticed, becasue we bound `maxLineMainDim` to container dimension constraints at the end of layout, and the error doesn't effect how any children are positioned or sized.

`betweenMainDim` may still be set for `gap` even if we don't have a sized parent, which makes the extra space propagated to `maxLineMainDim` effect parent size.

Because we were in a code freeze, I opted to have us go with a solution just effecting flex gap, instead of the right one, in case there were any side effects. This cleans up the code to use the right calculation everywhere, and fixes a separate bug, where `endOfLineIndex` and `startOfLineIndex` may not be the last/first in the line if they are out of the layout flow (absolutely positioned, or display: none_

See the original conversation on facebook/yoga#1188

Reviewed By: javache

Differential Revision: D49260049
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D49260049

NickGerleman added a commit to NickGerleman/react-native that referenced this pull request Sep 15, 2023
Summary:
X-link: facebook/yoga#1380


Back when rolling out flex gap, we encountered a bug where gap was added to the end of the main axis when a size was not specified.

During flex line justification/sizing, we calculate the amount of space that should be in between children. We erroneously add this, even after the last child element.

For `justify-content`, this space between children is derived from free space along the axis. The only time we have free space is if we had a dimension/dimension constraint already set on the parent. In this case, the extra space added to the end of the flex line is usually never noticed, because we bound `maxLineMainDim` to container dimension constraints at the end of layout, and the error doesn't effect how any children are positioned or sized.

There was at least one screenshot test where this issue showed up though, and I was able to add a slightly different repro where we may have free space without a definite dimension by enforcing a min dimension and not stretching.

{F1091401183}

The new reference is correct, and looking back at diffs, is what this seemed to originally look like when added three years ago. Seems like there may have been a potential regression, but I didn't spot anything suspicious when I looked around the code history.


`betweenMainDim` may still be set for `gap` even if we don't have a sized parent, which makes the extra space propagated to `maxLineMainDim` effect parent size.

Because we were in a code freeze, I opted to have us go with a solution just effecting flex gap, instead of the right one, in case there were any side effects. This cleans up the code to use the right calculation everywhere, and fixes a separate bug, where `endOfLineIndex` and `startOfLineIndex` may not be the last/first in the line if they are out of the layout flow (absolutely positioned, or display: none_

See the original conversation on facebook/yoga#1188

Reviewed By: javache

Differential Revision: D49260049
NickGerleman added a commit to NickGerleman/yoga that referenced this pull request Sep 15, 2023
Summary:

X-link: facebook/react-native#39433

Back when rolling out flex gap, we encountered a bug where gap was added to the end of the main axis when a size was not specified.

During flex line justification/sizing, we calculate the amount of space that should be in between children. We erroneously add this, even after the last child element.

For `justify-content`, this space between children is derived from free space along the axis. The only time we have free space is if we had a dimension/dimension constraint already set on the parent. In this case, the extra space added to the end of the flex line is usually never noticed, because we bound `maxLineMainDim` to container dimension constraints at the end of layout, and the error doesn't effect how any children are positioned or sized.

There was at least one screenshot test where this issue showed up though, and I was able to add a slightly different repro where we may have free space without a definite dimension by enforcing a min dimension and not stretching.

{F1091401183}

The new reference is correct, and looking back at diffs, is what this seemed to originally look like when added three years ago. Seems like there may have been a potential regression, but I didn't spot anything suspicious when I looked around the code history.


`betweenMainDim` may still be set for `gap` even if we don't have a sized parent, which makes the extra space propagated to `maxLineMainDim` effect parent size.

Because we were in a code freeze, I opted to have us go with a solution just effecting flex gap, instead of the right one, in case there were any side effects. This cleans up the code to use the right calculation everywhere, and fixes a separate bug, where `endOfLineIndex` and `startOfLineIndex` may not be the last/first in the line if they are out of the layout flow (absolutely positioned, or display: none_

See the original conversation on facebook#1188

Reviewed By: javache

Differential Revision: D49260049
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D49260049

NickGerleman added a commit to NickGerleman/yoga that referenced this pull request Sep 15, 2023
Summary:

X-link: facebook/react-native#39433

Back when rolling out flex gap, we encountered a bug where gap was added to the end of the main axis when a size was not specified.

During flex line justification/sizing, we calculate the amount of space that should be in between children. We erroneously add this, even after the last child element.

For `justify-content`, this space between children is derived from free space along the axis. The only time we have free space is if we had a dimension/dimension constraint already set on the parent. In this case, the extra space added to the end of the flex line is usually never noticed, because we bound `maxLineMainDim` to container dimension constraints at the end of layout, and the error doesn't effect how any children are positioned or sized.

There was at least one screenshot test where this issue showed up though, and I was able to add a slightly different repro where we may have free space without a definite dimension by enforcing a min dimension and not stretching.

{F1091401183}

The new reference is correct, and looking back at diffs, is what this seemed to originally look like when added three years ago. Seems like there may have been a potential regression, but I didn't spot anything suspicious when I looked around the code history.


`betweenMainDim` may still be set for `gap` even if we don't have a sized parent, which makes the extra space propagated to `maxLineMainDim` effect parent size.

Because we were in a code freeze, I opted to have us go with a solution just effecting flex gap, instead of the right one, in case there were any side effects. This cleans up the code to use the right calculation everywhere, and fixes a separate bug, where `endOfLineIndex` and `startOfLineIndex` may not be the last/first in the line if they are out of the layout flow (absolutely positioned, or display: none_

See the original conversation on facebook#1188

Reviewed By: javache

Differential Revision: D49260049
NickGerleman added a commit to NickGerleman/react-native that referenced this pull request Sep 15, 2023
Summary:
X-link: facebook/yoga#1380


Back when rolling out flex gap, we encountered a bug where gap was added to the end of the main axis when a size was not specified.

During flex line justification/sizing, we calculate the amount of space that should be in between children. We erroneously add this, even after the last child element.

For `justify-content`, this space between children is derived from free space along the axis. The only time we have free space is if we had a dimension/dimension constraint already set on the parent. In this case, the extra space added to the end of the flex line is usually never noticed, because we bound `maxLineMainDim` to container dimension constraints at the end of layout, and the error doesn't effect how any children are positioned or sized.

There was at least one screenshot test where this issue showed up though, and I was able to add a slightly different repro where we may have free space without a definite dimension by enforcing a min dimension and not stretching.

{F1091401183}

The new reference is correct, and looking back at diffs, is what this seemed to originally look like when added three years ago. Seems like there may have been a potential regression, but I didn't spot anything suspicious when I looked around the code history.


`betweenMainDim` may still be set for `gap` even if we don't have a sized parent, which makes the extra space propagated to `maxLineMainDim` effect parent size.

Because we were in a code freeze, I opted to have us go with a solution just effecting flex gap, instead of the right one, in case there were any side effects. This cleans up the code to use the right calculation everywhere, and fixes a separate bug, where `endOfLineIndex` and `startOfLineIndex` may not be the last/first in the line if they are out of the layout flow (absolutely positioned, or display: none_

See the original conversation on facebook/yoga#1188

Reviewed By: javache

Differential Revision: D49260049
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D49260049

NickGerleman added a commit to NickGerleman/yoga that referenced this pull request Sep 15, 2023
Summary:

X-link: facebook/react-native#39433

Back when rolling out flex gap, we encountered a bug where gap was added to the end of the main axis when a size was not specified.

During flex line justification/sizing, we calculate the amount of space that should be in between children. We erroneously add this, even after the last child element.

For `justify-content`, this space between children is derived from free space along the axis. The only time we have free space is if we had a dimension/dimension constraint already set on the parent. In this case, the extra space added to the end of the flex line is usually never noticed, because we bound `maxLineMainDim` to container dimension constraints at the end of layout, and the error doesn't effect how any children are positioned or sized.

There was at least one screenshot test where this issue showed up though, and I was able to add a slightly different repro where we may have free space without a definite dimension by enforcing a min dimension and not stretching.

{F1091401183}

The new reference is correct, and looking back at diffs, is what this seemed to originally look like when added three years ago. Seems like there may have been a potential regression, but I didn't spot anything suspicious when I looked around the code history.


`betweenMainDim` may still be set for `gap` even if we don't have a sized parent, which makes the extra space propagated to `maxLineMainDim` effect parent size.

Because we were in a code freeze, I opted to have us go with a solution just effecting flex gap, instead of the right one, in case there were any side effects. This cleans up the code to use the right calculation everywhere, and fixes a separate bug, where `endOfLineIndex` and `startOfLineIndex` may not be the last/first in the line if they are out of the layout flow (absolutely positioned, or display: none_

See the original conversation on facebook#1188

Reviewed By: javache

Differential Revision: D49260049
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D49260049

NickGerleman added a commit to NickGerleman/react-native that referenced this pull request Sep 15, 2023
Summary:
X-link: facebook/yoga#1380


Back when rolling out flex gap, we encountered a bug where gap was added to the end of the main axis when a size was not specified.

During flex line justification/sizing, we calculate the amount of space that should be in between children. We erroneously add this, even after the last child element.

For `justify-content`, this space between children is derived from free space along the axis. The only time we have free space is if we had a dimension/dimension constraint already set on the parent. In this case, the extra space added to the end of the flex line is usually never noticed, because we bound `maxLineMainDim` to container dimension constraints at the end of layout, and the error doesn't effect how any children are positioned or sized.

There was at least one screenshot test where this issue showed up though, and I was able to add a slightly different repro where we may have free space without a definite dimension by enforcing a min dimension and not stretching.

{F1091401183}

The new reference is correct, and looking back at diffs, is what this seemed to originally look like when added three years ago. Seems like there may have been a potential regression, but I didn't spot anything suspicious when I looked around the code history.


`betweenMainDim` may still be set for `gap` even if we don't have a sized parent, which makes the extra space propagated to `maxLineMainDim` effect parent size.

Because we were in a code freeze, I opted to have us go with a solution just effecting flex gap, instead of the right one, in case there were any side effects. This cleans up the code to use the right calculation everywhere, and fixes a separate bug, where `endOfLineIndex` and `startOfLineIndex` may not be the last/first in the line if they are out of the layout flow (absolutely positioned, or display: none_

See the original conversation on facebook/yoga#1188

Reviewed By: javache

Differential Revision: D49260049
NickGerleman added a commit to NickGerleman/yoga that referenced this pull request Sep 15, 2023
Summary:

X-link: facebook/react-native#39433

Back when rolling out flex gap, we encountered a bug where gap was added to the end of the main axis when a size was not specified.

During flex line justification/sizing, we calculate the amount of space that should be in between children. We erroneously add this, even after the last child element.

For `justify-content`, this space between children is derived from free space along the axis. The only time we have free space is if we had a dimension/dimension constraint already set on the parent. In this case, the extra space added to the end of the flex line is usually never noticed, because we bound `maxLineMainDim` to container dimension constraints at the end of layout, and the error doesn't effect how any children are positioned or sized.

There was at least one screenshot test where this issue showed up though, and I was able to add a slightly different repro where we may have free space without a definite dimension by enforcing a min dimension and not stretching.

{F1091401183}

The new reference is correct, and looking back at diffs, is what this seemed to originally look like when added three years ago. Seems like there may have been a potential regression, but I didn't spot anything suspicious when I looked around the code history.


`betweenMainDim` may still be set for `gap` even if we don't have a sized parent, which makes the extra space propagated to `maxLineMainDim` effect parent size.

Because we were in a code freeze, I opted to have us go with a solution just effecting flex gap, instead of the right one, in case there were any side effects. This cleans up the code to use the right calculation everywhere, and fixes a separate bug, where `endOfLineIndex` and `startOfLineIndex` may not be the last/first in the line if they are out of the layout flow (absolutely positioned, or display: none_

See the original conversation on facebook#1188

Reviewed By: javache

Differential Revision: D49260049
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D49260049

NickGerleman added a commit to NickGerleman/react-native that referenced this pull request Sep 15, 2023
Summary:
X-link: facebook/yoga#1380


Back when rolling out flex gap, we encountered a bug where gap was added to the end of the main axis when a size was not specified.

During flex line justification/sizing, we calculate the amount of space that should be in between children. We erroneously add this, even after the last child element.

For `justify-content`, this space between children is derived from free space along the axis. The only time we have free space is if we had a dimension/dimension constraint already set on the parent. In this case, the extra space added to the end of the flex line is usually never noticed, because we bound `maxLineMainDim` to container dimension constraints at the end of layout, and the error doesn't effect how any children are positioned or sized.

There was at least one screenshot test where this issue showed up though, and I was able to add a slightly different repro where we may have free space without a definite dimension by enforcing a min dimension and not stretching.

{F1091401183}

The new reference is correct, and looking back at diffs, is what this seemed to originally look like when added three years ago. Seems like there may have been a potential regression, but I didn't spot anything suspicious when I looked around the code history.


`betweenMainDim` may still be set for `gap` even if we don't have a sized parent, which makes the extra space propagated to `maxLineMainDim` effect parent size.

Because we were in a code freeze, I opted to have us go with a solution just effecting flex gap, instead of the right one, in case there were any side effects. This cleans up the code to use the right calculation everywhere, and fixes a separate bug, where `endOfLineIndex` and `startOfLineIndex` may not be the last/first in the line if they are out of the layout flow (absolutely positioned, or display: none_

See the original conversation on facebook/yoga#1188

Reviewed By: javache

Differential Revision: D49260049
Summary:

X-link: facebook/react-native#39433

Back when rolling out flex gap, we encountered a bug where gap was added to the end of the main axis when a size was not specified.

During flex line justification/sizing, we calculate the amount of space that should be in between children. We erroneously add this, even after the last child element.

For `justify-content`, this space between children is derived from free space along the axis. The only time we have free space is if we had a dimension/dimension constraint already set on the parent. In this case, the extra space added to the end of the flex line is usually never noticed, because we bound `maxLineMainDim` to container dimension constraints at the end of layout, and the error doesn't effect how any children are positioned or sized.

There was at least one screenshot test where this issue showed up though, and I was able to add a slightly different repro where we may have free space without a definite dimension by enforcing a min dimension and not stretching.

{F1091401183}

The new reference is correct, and looking back at diffs, is what this seemed to originally look like when added three years ago. Seems like there may have been a potential regression, but I didn't spot anything suspicious when I looked around the code history.


`betweenMainDim` may still be set for `gap` even if we don't have a sized parent, which makes the extra space propagated to `maxLineMainDim` effect parent size.

Because we were in a code freeze, I opted to have us go with a solution just effecting flex gap, instead of the right one, in case there were any side effects. This cleans up the code to use the right calculation everywhere, and fixes a separate bug, where `endOfLineIndex` and `startOfLineIndex` may not be the last/first in the line if they are out of the layout flow (absolutely positioned, or display: none_

See the original conversation on facebook#1188

Reviewed By: javache

Differential Revision: D49260049
NickGerleman added a commit to NickGerleman/react-native that referenced this pull request Sep 15, 2023
Summary:
X-link: facebook/yoga#1380


Back when rolling out flex gap, we encountered a bug where gap was added to the end of the main axis when a size was not specified.

During flex line justification/sizing, we calculate the amount of space that should be in between children. We erroneously add this, even after the last child element.

For `justify-content`, this space between children is derived from free space along the axis. The only time we have free space is if we had a dimension/dimension constraint already set on the parent. In this case, the extra space added to the end of the flex line is usually never noticed, because we bound `maxLineMainDim` to container dimension constraints at the end of layout, and the error doesn't effect how any children are positioned or sized.

There was at least one screenshot test where this issue showed up though, and I was able to add a slightly different repro where we may have free space without a definite dimension by enforcing a min dimension and not stretching.

{F1091401183}

The new reference is correct, and looking back at diffs, is what this seemed to originally look like when added three years ago. Seems like there may have been a potential regression, but I didn't spot anything suspicious when I looked around the code history.


`betweenMainDim` may still be set for `gap` even if we don't have a sized parent, which makes the extra space propagated to `maxLineMainDim` effect parent size.

Because we were in a code freeze, I opted to have us go with a solution just effecting flex gap, instead of the right one, in case there were any side effects. This cleans up the code to use the right calculation everywhere, and fixes a separate bug, where `endOfLineIndex` and `startOfLineIndex` may not be the last/first in the line if they are out of the layout flow (absolutely positioned, or display: none_

See the original conversation on facebook/yoga#1188

Reviewed By: javache

Differential Revision: D49260049
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D49260049

facebook-github-bot pushed a commit to facebook/litho that referenced this pull request Sep 15, 2023
Summary:
X-link: facebook/yoga#1380

X-link: facebook/react-native#39433

Back when rolling out flex gap, we encountered a bug where gap was added to the end of the main axis when a size was not specified.

During flex line justification/sizing, we calculate the amount of space that should be in between children. We erroneously add this, even after the last child element.

For `justify-content`, this space between children is derived from free space along the axis. The only time we have free space is if we had a dimension/dimension constraint already set on the parent. In this case, the extra space added to the end of the flex line is usually never noticed, because we bound `maxLineMainDim` to container dimension constraints at the end of layout, and the error doesn't effect how any children are positioned or sized.

There was at least one screenshot test where this issue showed up though, and I was able to add a slightly different repro where we may have free space without a definite dimension by enforcing a min dimension and not stretching.

{F1091401183}

The new reference is correct, and looking back at diffs, is what this seemed to originally look like when added three years ago. Seems like there may have been a potential regression, but I didn't spot anything suspicious when I looked around the code history.

`betweenMainDim` may still be set for `gap` even if we don't have a sized parent, which makes the extra space propagated to `maxLineMainDim` effect parent size.

Because we were in a code freeze, I opted to have us go with a solution just effecting flex gap, instead of the right one, in case there were any side effects. This cleans up the code to use the right calculation everywhere, and fixes a separate bug, where `endOfLineIndex` and `startOfLineIndex` may not be the last/first in the line if they are out of the layout flow (absolutely positioned, or display: none_

See the original conversation on facebook/yoga#1188

Reviewed By: javache

Differential Revision: D49260049

fbshipit-source-id: 218552c5ff938668b9f257df7a1493e13ded4d0d
@facebook-github-bot
Copy link
Contributor

This pull request has been merged in c60050d.

facebook-github-bot pushed a commit to facebook/react-native that referenced this pull request Sep 15, 2023
Summary:
X-link: facebook/yoga#1380

Pull Request resolved: #39433

Back when rolling out flex gap, we encountered a bug where gap was added to the end of the main axis when a size was not specified.

During flex line justification/sizing, we calculate the amount of space that should be in between children. We erroneously add this, even after the last child element.

For `justify-content`, this space between children is derived from free space along the axis. The only time we have free space is if we had a dimension/dimension constraint already set on the parent. In this case, the extra space added to the end of the flex line is usually never noticed, because we bound `maxLineMainDim` to container dimension constraints at the end of layout, and the error doesn't effect how any children are positioned or sized.

There was at least one screenshot test where this issue showed up though, and I was able to add a slightly different repro where we may have free space without a definite dimension by enforcing a min dimension and not stretching.

{F1091401183}

The new reference is correct, and looking back at diffs, is what this seemed to originally look like when added three years ago. Seems like there may have been a potential regression, but I didn't spot anything suspicious when I looked around the code history.

`betweenMainDim` may still be set for `gap` even if we don't have a sized parent, which makes the extra space propagated to `maxLineMainDim` effect parent size.

Because we were in a code freeze, I opted to have us go with a solution just effecting flex gap, instead of the right one, in case there were any side effects. This cleans up the code to use the right calculation everywhere, and fixes a separate bug, where `endOfLineIndex` and `startOfLineIndex` may not be the last/first in the line if they are out of the layout flow (absolutely positioned, or display: none_

See the original conversation on facebook/yoga#1188

Reviewed By: javache

Differential Revision: D49260049

fbshipit-source-id: 218552c5ff938668b9f257df7a1493e13ded4d0d
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants