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

Fix issue where marginStart and marginEnd were not working with rowReverse flex direction #1420

Closed
wants to merge 2 commits into from

Conversation

joevilches
Copy link
Contributor

Summary:
This stack is ultimately aiming to solve #1208

The problem
Turns out that we do not even check direction when determining which edge is the leading (start) and trailing (end) edges. This is not how web does it as the start/end is based on the writing direction NOT the flex direction: https://developer.mozilla.org/en-US/docs/Web/CSS/CSS_flexible_box_layout/Basic_concepts_of_flexbox#start_and_end_lines. While web does not have marginStart and marginEnd, they do have margin-inline-start/end which relies on the writing mode to determine the "start"/"end": https://developer.mozilla.org/en-US/docs/Web/CSS/margin-inline-start.

This means that if you do something like

export default function Playground(props: Props): React.Node {
  return (
    <View style={styles.container}>
      <View style={styles.item} />
    </View>
  );
}

const styles = StyleSheet.create({
  container: {
    marginEnd: 100,
    flexDirection: 'row-reverse',
    backgroundColor: 'red',
    display: 'flex',
    width: 100,
    height: 100,
  },
  item: {
    backgroundColor: 'blue',
    width: 10,
  },
});

You get {F1116264350}
As you can see the margin gets applied to the left edge even thought the direction is ltr and it should be applied to the right edge.

The solution
I ended up fixing this by creating a new leadingLayoutEdge and trailingLayoutEdge function that take the flex direction as well as the direction. Based on the errata, the a few functions will use these new functions to determine which YGEdge is the starting/ending.

You might be wondering why I did not put this logic inside of leadingEdge(flexDirection) / trailingEdge(flexDirection) since other areas could potentially have the same bug like getLeadingPadding. These functions are a bit overloaded and there are cases where we actually want to use the flexDirection to get the edge in question. For example, many of the calls to setLayoutPosition in CalculateLayout.cpp call leadingEdge() / trailingEdge() to set the proper position for cases like row-reverse where items need to line up in a different direction.

Differential Revision: D50140503

@facebook-github-bot
Copy link
Contributor

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

@facebook-github-bot
Copy link
Contributor

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

joevilches added a commit to joevilches/yoga that referenced this pull request Oct 12, 2023
…verse flex direction (facebook#1420)

Summary:
X-link: facebook/litho#962

X-link: facebook/react-native#40804

Pull Request resolved: facebook#1420

This stack is ultimately aiming to solve facebook#1208

**The problem**
Turns out that we do not even check direction when determining which edge is the leading (start) and trailing (end) edges. This is not how web does it as the start/end is based on the writing direction NOT the flex direction: https://developer.mozilla.org/en-US/docs/Web/CSS/CSS_flexible_box_layout/Basic_concepts_of_flexbox#start_and_end_lines. While web does not have marginStart and marginEnd, they do have margin-inline-start/end which relies on the writing mode to determine the "start"/"end": https://developer.mozilla.org/en-US/docs/Web/CSS/margin-inline-start.

This means that if you do something like
```
export default function Playground(props: Props): React.Node {
  return (
    <View style={styles.container}>
      <View style={styles.item} />
    </View>
  );
}

const styles = StyleSheet.create({
  container: {
    marginEnd: 100,
    flexDirection: 'row-reverse',
    backgroundColor: 'red',
    display: 'flex',
    width: 100,
    height: 100,
  },
  item: {
    backgroundColor: 'blue',
    width: 10,
  },
});
```

You get  {F1116264350}
As you can see the margin gets applied to the left edge even thought the direction is ltr and it should be applied to the right edge.

**The solution**
I ended up fixing this by creating a new `leadingLayoutEdge` and `trailingLayoutEdge` function that take the flex direction as well as the direction. Based on the errata, the a few functions will use these new functions to determine which `YGEdge` is the starting/ending.

You might be wondering why I did not put this logic inside of `leadingEdge(flexDirection)` / `trailingEdge(flexDirection)` since other areas could potentially have the same bug like `getLeadingPadding`. These functions are a bit overloaded and there are cases where we actually want to use the flexDirection to get the edge in question. For example, many of the calls to `setLayoutPosition` in `CalculateLayout.cpp` call `leadingEdge()` / `trailingEdge()` to set the proper position for cases like row-reverse where items need to line up in a different direction.

Differential Revision: D50140503

fbshipit-source-id: a120a32f010eca5edc91a5f970356b8d3dbc1ebd
joevilches added a commit to joevilches/litho that referenced this pull request Oct 12, 2023
…verse flex direction (facebook#962)

Summary:
Pull Request resolved: facebook#962

X-link: facebook/react-native#40804

X-link: facebook/yoga#1420

This stack is ultimately aiming to solve facebook/yoga#1208

**The problem**
Turns out that we do not even check direction when determining which edge is the leading (start) and trailing (end) edges. This is not how web does it as the start/end is based on the writing direction NOT the flex direction: https://developer.mozilla.org/en-US/docs/Web/CSS/CSS_flexible_box_layout/Basic_concepts_of_flexbox#start_and_end_lines. While web does not have marginStart and marginEnd, they do have margin-inline-start/end which relies on the writing mode to determine the "start"/"end": https://developer.mozilla.org/en-US/docs/Web/CSS/margin-inline-start.

This means that if you do something like
```
export default function Playground(props: Props): React.Node {
  return (
    <View style={styles.container}>
      <View style={styles.item} />
    </View>
  );
}

const styles = StyleSheet.create({
  container: {
    marginEnd: 100,
    flexDirection: 'row-reverse',
    backgroundColor: 'red',
    display: 'flex',
    width: 100,
    height: 100,
  },
  item: {
    backgroundColor: 'blue',
    width: 10,
  },
});
```

You get  {F1116264350}
As you can see the margin gets applied to the left edge even thought the direction is ltr and it should be applied to the right edge.

**The solution**
I ended up fixing this by creating a new `leadingLayoutEdge` and `trailingLayoutEdge` function that take the flex direction as well as the direction. Based on the errata, the a few functions will use these new functions to determine which `YGEdge` is the starting/ending.

You might be wondering why I did not put this logic inside of `leadingEdge(flexDirection)` / `trailingEdge(flexDirection)` since other areas could potentially have the same bug like `getLeadingPadding`. These functions are a bit overloaded and there are cases where we actually want to use the flexDirection to get the edge in question. For example, many of the calls to `setLayoutPosition` in `CalculateLayout.cpp` call `leadingEdge()` / `trailingEdge()` to set the proper position for cases like row-reverse where items need to line up in a different direction.

Differential Revision: D50140503

fbshipit-source-id: e9888b97cf8f4b04d1f00e986110c2c68a674f04
joevilches added a commit to joevilches/react-native that referenced this pull request Oct 12, 2023
…verse flex direction (facebook#40804)

Summary:
X-link: facebook/litho#962

Pull Request resolved: facebook#40804

X-link: facebook/yoga#1420

This stack is ultimately aiming to solve facebook/yoga#1208

**The problem**
Turns out that we do not even check direction when determining which edge is the leading (start) and trailing (end) edges. This is not how web does it as the start/end is based on the writing direction NOT the flex direction: https://developer.mozilla.org/en-US/docs/Web/CSS/CSS_flexible_box_layout/Basic_concepts_of_flexbox#start_and_end_lines. While web does not have marginStart and marginEnd, they do have margin-inline-start/end which relies on the writing mode to determine the "start"/"end": https://developer.mozilla.org/en-US/docs/Web/CSS/margin-inline-start.

This means that if you do something like
```
export default function Playground(props: Props): React.Node {
  return (
    <View style={styles.container}>
      <View style={styles.item} />
    </View>
  );
}

const styles = StyleSheet.create({
  container: {
    marginEnd: 100,
    flexDirection: 'row-reverse',
    backgroundColor: 'red',
    display: 'flex',
    width: 100,
    height: 100,
  },
  item: {
    backgroundColor: 'blue',
    width: 10,
  },
});
```

You get  {F1116264350}
As you can see the margin gets applied to the left edge even thought the direction is ltr and it should be applied to the right edge.

**The solution**
I ended up fixing this by creating a new `leadingLayoutEdge` and `trailingLayoutEdge` function that take the flex direction as well as the direction. Based on the errata, the a few functions will use these new functions to determine which `YGEdge` is the starting/ending.

You might be wondering why I did not put this logic inside of `leadingEdge(flexDirection)` / `trailingEdge(flexDirection)` since other areas could potentially have the same bug like `getLeadingPadding`. These functions are a bit overloaded and there are cases where we actually want to use the flexDirection to get the edge in question. For example, many of the calls to `setLayoutPosition` in `CalculateLayout.cpp` call `leadingEdge()` / `trailingEdge()` to set the proper position for cases like row-reverse where items need to line up in a different direction.

Differential Revision: D50140503

fbshipit-source-id: 34fa8bd824a4289c21a74b9cdb41cd083bd493ff
@facebook-github-bot
Copy link
Contributor

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

joevilches added a commit to joevilches/yoga that referenced this pull request Oct 12, 2023
…verse flex direction (facebook#1420)

Summary:
X-link: facebook/litho#962

X-link: facebook/react-native#40804

Pull Request resolved: facebook#1420

This stack is ultimately aiming to solve facebook#1208

**The problem**
Turns out that we do not even check direction when determining which edge is the leading (start) and trailing (end) edges. This is not how web does it as the start/end is based on the writing direction NOT the flex direction: https://developer.mozilla.org/en-US/docs/Web/CSS/CSS_flexible_box_layout/Basic_concepts_of_flexbox#start_and_end_lines. While web does not have marginStart and marginEnd, they do have margin-inline-start/end which relies on the writing mode to determine the "start"/"end": https://developer.mozilla.org/en-US/docs/Web/CSS/margin-inline-start.

This means that if you do something like
```
export default function Playground(props: Props): React.Node {
  return (
    <View style={styles.container}>
      <View style={styles.item} />
    </View>
  );
}

const styles = StyleSheet.create({
  container: {
    marginEnd: 100,
    flexDirection: 'row-reverse',
    backgroundColor: 'red',
    display: 'flex',
    width: 100,
    height: 100,
  },
  item: {
    backgroundColor: 'blue',
    width: 10,
  },
});
```

You get  {F1116264350}
As you can see the margin gets applied to the left edge even thought the direction is ltr and it should be applied to the right edge.

**The solution**
I ended up fixing this by creating a new `leadingLayoutEdge` and `trailingLayoutEdge` function that take the flex direction as well as the direction. Based on the errata, the a few functions will use these new functions to determine which `YGEdge` is the starting/ending.

You might be wondering why I did not put this logic inside of `leadingEdge(flexDirection)` / `trailingEdge(flexDirection)` since other areas could potentially have the same bug like `getLeadingPadding`. These functions are a bit overloaded and there are cases where we actually want to use the flexDirection to get the edge in question. For example, many of the calls to `setLayoutPosition` in `CalculateLayout.cpp` call `leadingEdge()` / `trailingEdge()` to set the proper position for cases like row-reverse where items need to line up in a different direction.

Differential Revision: D50140503

fbshipit-source-id: 3843348b103868a241333d87190839db6ea03b73
joevilches added a commit to joevilches/litho that referenced this pull request Oct 12, 2023
…verse flex direction (facebook#962)

Summary:
Pull Request resolved: facebook#962

X-link: facebook/react-native#40804

X-link: facebook/yoga#1420

This stack is ultimately aiming to solve facebook/yoga#1208

**The problem**
Turns out that we do not even check direction when determining which edge is the leading (start) and trailing (end) edges. This is not how web does it as the start/end is based on the writing direction NOT the flex direction: https://developer.mozilla.org/en-US/docs/Web/CSS/CSS_flexible_box_layout/Basic_concepts_of_flexbox#start_and_end_lines. While web does not have marginStart and marginEnd, they do have margin-inline-start/end which relies on the writing mode to determine the "start"/"end": https://developer.mozilla.org/en-US/docs/Web/CSS/margin-inline-start.

This means that if you do something like
```
export default function Playground(props: Props): React.Node {
  return (
    <View style={styles.container}>
      <View style={styles.item} />
    </View>
  );
}

const styles = StyleSheet.create({
  container: {
    marginEnd: 100,
    flexDirection: 'row-reverse',
    backgroundColor: 'red',
    display: 'flex',
    width: 100,
    height: 100,
  },
  item: {
    backgroundColor: 'blue',
    width: 10,
  },
});
```

You get  {F1116264350}
As you can see the margin gets applied to the left edge even thought the direction is ltr and it should be applied to the right edge.

**The solution**
I ended up fixing this by creating a new `leadingLayoutEdge` and `trailingLayoutEdge` function that take the flex direction as well as the direction. Based on the errata, the a few functions will use these new functions to determine which `YGEdge` is the starting/ending.

You might be wondering why I did not put this logic inside of `leadingEdge(flexDirection)` / `trailingEdge(flexDirection)` since other areas could potentially have the same bug like `getLeadingPadding`. These functions are a bit overloaded and there are cases where we actually want to use the flexDirection to get the edge in question. For example, many of the calls to `setLayoutPosition` in `CalculateLayout.cpp` call `leadingEdge()` / `trailingEdge()` to set the proper position for cases like row-reverse where items need to line up in a different direction.

Differential Revision: D50140503

fbshipit-source-id: f12189ea03a309a1f131a308965943d91ab6e0ed
joevilches added a commit to joevilches/react-native that referenced this pull request Oct 12, 2023
…verse flex direction (facebook#40804)

Summary:
X-link: facebook/litho#962

Pull Request resolved: facebook#40804

X-link: facebook/yoga#1420

This stack is ultimately aiming to solve facebook/yoga#1208

**The problem**
Turns out that we do not even check direction when determining which edge is the leading (start) and trailing (end) edges. This is not how web does it as the start/end is based on the writing direction NOT the flex direction: https://developer.mozilla.org/en-US/docs/Web/CSS/CSS_flexible_box_layout/Basic_concepts_of_flexbox#start_and_end_lines. While web does not have marginStart and marginEnd, they do have margin-inline-start/end which relies on the writing mode to determine the "start"/"end": https://developer.mozilla.org/en-US/docs/Web/CSS/margin-inline-start.

This means that if you do something like
```
export default function Playground(props: Props): React.Node {
  return (
    <View style={styles.container}>
      <View style={styles.item} />
    </View>
  );
}

const styles = StyleSheet.create({
  container: {
    marginEnd: 100,
    flexDirection: 'row-reverse',
    backgroundColor: 'red',
    display: 'flex',
    width: 100,
    height: 100,
  },
  item: {
    backgroundColor: 'blue',
    width: 10,
  },
});
```

You get  {F1116264350}
As you can see the margin gets applied to the left edge even thought the direction is ltr and it should be applied to the right edge.

**The solution**
I ended up fixing this by creating a new `leadingLayoutEdge` and `trailingLayoutEdge` function that take the flex direction as well as the direction. Based on the errata, the a few functions will use these new functions to determine which `YGEdge` is the starting/ending.

You might be wondering why I did not put this logic inside of `leadingEdge(flexDirection)` / `trailingEdge(flexDirection)` since other areas could potentially have the same bug like `getLeadingPadding`. These functions are a bit overloaded and there are cases where we actually want to use the flexDirection to get the edge in question. For example, many of the calls to `setLayoutPosition` in `CalculateLayout.cpp` call `leadingEdge()` / `trailingEdge()` to set the proper position for cases like row-reverse where items need to line up in a different direction.

Reviewed By: NickGerleman

Differential Revision: D50140503

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

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

joevilches added a commit to joevilches/yoga that referenced this pull request Oct 12, 2023
…verse flex direction (facebook#1420)

Summary:
X-link: facebook/litho#962

X-link: facebook/react-native#40804

Pull Request resolved: facebook#1420

This stack is ultimately aiming to solve facebook#1208

**The problem**
Turns out that we do not even check direction when determining which edge is the leading (start) and trailing (end) edges. This is not how web does it as the start/end is based on the writing direction NOT the flex direction: https://developer.mozilla.org/en-US/docs/Web/CSS/CSS_flexible_box_layout/Basic_concepts_of_flexbox#start_and_end_lines. While web does not have marginStart and marginEnd, they do have margin-inline-start/end which relies on the writing mode to determine the "start"/"end": https://developer.mozilla.org/en-US/docs/Web/CSS/margin-inline-start.

This means that if you do something like
```
export default function Playground(props: Props): React.Node {
  return (
    <View style={styles.container}>
      <View style={styles.item} />
    </View>
  );
}

const styles = StyleSheet.create({
  container: {
    marginEnd: 100,
    flexDirection: 'row-reverse',
    backgroundColor: 'red',
    display: 'flex',
    width: 100,
    height: 100,
  },
  item: {
    backgroundColor: 'blue',
    width: 10,
  },
});
```

You get  {F1116264350}
As you can see the margin gets applied to the left edge even thought the direction is ltr and it should be applied to the right edge.

**The solution**
I ended up fixing this by creating a new `leadingLayoutEdge` and `trailingLayoutEdge` function that take the flex direction as well as the direction. Based on the errata, the a few functions will use these new functions to determine which `YGEdge` is the starting/ending.

You might be wondering why I did not put this logic inside of `leadingEdge(flexDirection)` / `trailingEdge(flexDirection)` since other areas could potentially have the same bug like `getLeadingPadding`. These functions are a bit overloaded and there are cases where we actually want to use the flexDirection to get the edge in question. For example, many of the calls to `setLayoutPosition` in `CalculateLayout.cpp` call `leadingEdge()` / `trailingEdge()` to set the proper position for cases like row-reverse where items need to line up in a different direction.

Reviewed By: NickGerleman

Differential Revision: D50140503

fbshipit-source-id: aaa53c0c0c301d60b4aec95c15a77d9ee551023b
joevilches added a commit to joevilches/litho that referenced this pull request Oct 12, 2023
…verse flex direction (facebook#962)

Summary:
Pull Request resolved: facebook#962

X-link: facebook/react-native#40804

X-link: facebook/yoga#1420

This stack is ultimately aiming to solve facebook/yoga#1208

**The problem**
Turns out that we do not even check direction when determining which edge is the leading (start) and trailing (end) edges. This is not how web does it as the start/end is based on the writing direction NOT the flex direction: https://developer.mozilla.org/en-US/docs/Web/CSS/CSS_flexible_box_layout/Basic_concepts_of_flexbox#start_and_end_lines. While web does not have marginStart and marginEnd, they do have margin-inline-start/end which relies on the writing mode to determine the "start"/"end": https://developer.mozilla.org/en-US/docs/Web/CSS/margin-inline-start.

This means that if you do something like
```
export default function Playground(props: Props): React.Node {
  return (
    <View style={styles.container}>
      <View style={styles.item} />
    </View>
  );
}

const styles = StyleSheet.create({
  container: {
    marginEnd: 100,
    flexDirection: 'row-reverse',
    backgroundColor: 'red',
    display: 'flex',
    width: 100,
    height: 100,
  },
  item: {
    backgroundColor: 'blue',
    width: 10,
  },
});
```

You get  {F1116264350}
As you can see the margin gets applied to the left edge even thought the direction is ltr and it should be applied to the right edge.

**The solution**
I ended up fixing this by creating a new `leadingLayoutEdge` and `trailingLayoutEdge` function that take the flex direction as well as the direction. Based on the errata, the a few functions will use these new functions to determine which `YGEdge` is the starting/ending.

You might be wondering why I did not put this logic inside of `leadingEdge(flexDirection)` / `trailingEdge(flexDirection)` since other areas could potentially have the same bug like `getLeadingPadding`. These functions are a bit overloaded and there are cases where we actually want to use the flexDirection to get the edge in question. For example, many of the calls to `setLayoutPosition` in `CalculateLayout.cpp` call `leadingEdge()` / `trailingEdge()` to set the proper position for cases like row-reverse where items need to line up in a different direction.

Reviewed By: NickGerleman

Differential Revision: D50140503

fbshipit-source-id: db6ea7af9d4dfbc481bb37b1bbad5a4fa60cb454
joevilches added a commit to joevilches/react-native that referenced this pull request Oct 12, 2023
…verse flex direction (facebook#40804)

Summary:
X-link: facebook/litho#962

Pull Request resolved: facebook#40804

X-link: facebook/yoga#1420

This stack is ultimately aiming to solve facebook/yoga#1208

**The problem**
Turns out that we do not even check direction when determining which edge is the leading (start) and trailing (end) edges. This is not how web does it as the start/end is based on the writing direction NOT the flex direction: https://developer.mozilla.org/en-US/docs/Web/CSS/CSS_flexible_box_layout/Basic_concepts_of_flexbox#start_and_end_lines. While web does not have marginStart and marginEnd, they do have margin-inline-start/end which relies on the writing mode to determine the "start"/"end": https://developer.mozilla.org/en-US/docs/Web/CSS/margin-inline-start.

This means that if you do something like
```
export default function Playground(props: Props): React.Node {
  return (
    <View style={styles.container}>
      <View style={styles.item} />
    </View>
  );
}

const styles = StyleSheet.create({
  container: {
    marginEnd: 100,
    flexDirection: 'row-reverse',
    backgroundColor: 'red',
    display: 'flex',
    width: 100,
    height: 100,
  },
  item: {
    backgroundColor: 'blue',
    width: 10,
  },
});
```

You get  {F1116264350}
As you can see the margin gets applied to the left edge even thought the direction is ltr and it should be applied to the right edge.

**The solution**
I ended up fixing this by creating a new `leadingLayoutEdge` and `trailingLayoutEdge` function that take the flex direction as well as the direction. Based on the errata, the a few functions will use these new functions to determine which `YGEdge` is the starting/ending.

You might be wondering why I did not put this logic inside of `leadingEdge(flexDirection)` / `trailingEdge(flexDirection)` since other areas could potentially have the same bug like `getLeadingPadding`. These functions are a bit overloaded and there are cases where we actually want to use the flexDirection to get the edge in question. For example, many of the calls to `setLayoutPosition` in `CalculateLayout.cpp` call `leadingEdge()` / `trailingEdge()` to set the proper position for cases like row-reverse where items need to line up in a different direction.

Reviewed By: NickGerleman

Differential Revision: D50140503

fbshipit-source-id: b387787a705cb928ba7b8565a599468abe3f33fc
joevilches added a commit to joevilches/react-native that referenced this pull request Oct 12, 2023
…verse flex direction (facebook#40804)

Summary:
X-link: facebook/litho#962

Pull Request resolved: facebook#40804

X-link: facebook/yoga#1420

This stack is ultimately aiming to solve facebook/yoga#1208

**The problem**
Turns out that we do not even check direction when determining which edge is the leading (start) and trailing (end) edges. This is not how web does it as the start/end is based on the writing direction NOT the flex direction: https://developer.mozilla.org/en-US/docs/Web/CSS/CSS_flexible_box_layout/Basic_concepts_of_flexbox#start_and_end_lines. While web does not have marginStart and marginEnd, they do have margin-inline-start/end which relies on the writing mode to determine the "start"/"end": https://developer.mozilla.org/en-US/docs/Web/CSS/margin-inline-start.

This means that if you do something like
```
export default function Playground(props: Props): React.Node {
  return (
    <View style={styles.container}>
      <View style={styles.item} />
    </View>
  );
}

const styles = StyleSheet.create({
  container: {
    marginEnd: 100,
    flexDirection: 'row-reverse',
    backgroundColor: 'red',
    display: 'flex',
    width: 100,
    height: 100,
  },
  item: {
    backgroundColor: 'blue',
    width: 10,
  },
});
```

You get  {F1116264350}
As you can see the margin gets applied to the left edge even thought the direction is ltr and it should be applied to the right edge.

**The solution**
I ended up fixing this by creating a new `leadingLayoutEdge` and `trailingLayoutEdge` function that take the flex direction as well as the direction. Based on the errata, the a few functions will use these new functions to determine which `YGEdge` is the starting/ending.

You might be wondering why I did not put this logic inside of `leadingEdge(flexDirection)` / `trailingEdge(flexDirection)` since other areas could potentially have the same bug like `getLeadingPadding`. These functions are a bit overloaded and there are cases where we actually want to use the flexDirection to get the edge in question. For example, many of the calls to `setLayoutPosition` in `CalculateLayout.cpp` call `leadingEdge()` / `trailingEdge()` to set the proper position for cases like row-reverse where items need to line up in a different direction.

Reviewed By: NickGerleman

Differential Revision: D50140503

fbshipit-source-id: 9dc87d072da2d504c3fa3cb6e8fb3077303dad37
@facebook-github-bot
Copy link
Contributor

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

joevilches added a commit to joevilches/yoga that referenced this pull request Oct 12, 2023
…verse flex direction (facebook#1420)

Summary:
X-link: facebook/litho#962

X-link: facebook/react-native#40804

Pull Request resolved: facebook#1420

This stack is ultimately aiming to solve facebook#1208

**The problem**
Turns out that we do not even check direction when determining which edge is the leading (start) and trailing (end) edges. This is not how web does it as the start/end is based on the writing direction NOT the flex direction: https://developer.mozilla.org/en-US/docs/Web/CSS/CSS_flexible_box_layout/Basic_concepts_of_flexbox#start_and_end_lines. While web does not have marginStart and marginEnd, they do have margin-inline-start/end which relies on the writing mode to determine the "start"/"end": https://developer.mozilla.org/en-US/docs/Web/CSS/margin-inline-start.

This means that if you do something like
```
export default function Playground(props: Props): React.Node {
  return (
    <View style={styles.container}>
      <View style={styles.item} />
    </View>
  );
}

const styles = StyleSheet.create({
  container: {
    marginEnd: 100,
    flexDirection: 'row-reverse',
    backgroundColor: 'red',
    display: 'flex',
    width: 100,
    height: 100,
  },
  item: {
    backgroundColor: 'blue',
    width: 10,
  },
});
```

You get  {F1116264350}
As you can see the margin gets applied to the left edge even thought the direction is ltr and it should be applied to the right edge.

**The solution**
I ended up fixing this by creating a new `leadingLayoutEdge` and `trailingLayoutEdge` function that take the flex direction as well as the direction. Based on the errata, the a few functions will use these new functions to determine which `YGEdge` is the starting/ending.

You might be wondering why I did not put this logic inside of `leadingEdge(flexDirection)` / `trailingEdge(flexDirection)` since other areas could potentially have the same bug like `getLeadingPadding`. These functions are a bit overloaded and there are cases where we actually want to use the flexDirection to get the edge in question. For example, many of the calls to `setLayoutPosition` in `CalculateLayout.cpp` call `leadingEdge()` / `trailingEdge()` to set the proper position for cases like row-reverse where items need to line up in a different direction.

Reviewed By: NickGerleman

Differential Revision: D50140503

fbshipit-source-id: 8c3d13958e519c6b1e1daafe1e1f4e30d04b13a0
joevilches added a commit to joevilches/litho that referenced this pull request Oct 12, 2023
…verse flex direction (facebook#962)

Summary:
Pull Request resolved: facebook#962

X-link: facebook/react-native#40804

X-link: facebook/yoga#1420

This stack is ultimately aiming to solve facebook/yoga#1208

**The problem**
Turns out that we do not even check direction when determining which edge is the leading (start) and trailing (end) edges. This is not how web does it as the start/end is based on the writing direction NOT the flex direction: https://developer.mozilla.org/en-US/docs/Web/CSS/CSS_flexible_box_layout/Basic_concepts_of_flexbox#start_and_end_lines. While web does not have marginStart and marginEnd, they do have margin-inline-start/end which relies on the writing mode to determine the "start"/"end": https://developer.mozilla.org/en-US/docs/Web/CSS/margin-inline-start.

This means that if you do something like
```
export default function Playground(props: Props): React.Node {
  return (
    <View style={styles.container}>
      <View style={styles.item} />
    </View>
  );
}

const styles = StyleSheet.create({
  container: {
    marginEnd: 100,
    flexDirection: 'row-reverse',
    backgroundColor: 'red',
    display: 'flex',
    width: 100,
    height: 100,
  },
  item: {
    backgroundColor: 'blue',
    width: 10,
  },
});
```

You get  {F1116264350}
As you can see the margin gets applied to the left edge even thought the direction is ltr and it should be applied to the right edge.

**The solution**
I ended up fixing this by creating a new `leadingLayoutEdge` and `trailingLayoutEdge` function that take the flex direction as well as the direction. Based on the errata, the a few functions will use these new functions to determine which `YGEdge` is the starting/ending.

You might be wondering why I did not put this logic inside of `leadingEdge(flexDirection)` / `trailingEdge(flexDirection)` since other areas could potentially have the same bug like `getLeadingPadding`. These functions are a bit overloaded and there are cases where we actually want to use the flexDirection to get the edge in question. For example, many of the calls to `setLayoutPosition` in `CalculateLayout.cpp` call `leadingEdge()` / `trailingEdge()` to set the proper position for cases like row-reverse where items need to line up in a different direction.

Reviewed By: NickGerleman

Differential Revision: D50140503

fbshipit-source-id: 023c1d1ad6a5f9b6551bfe975d02dca5fcaf5c8f
joevilches added a commit to joevilches/react-native that referenced this pull request Oct 12, 2023
…verse flex direction (facebook#40804)

Summary:
X-link: facebook/litho#962

Pull Request resolved: facebook#40804

X-link: facebook/yoga#1420

This stack is ultimately aiming to solve facebook/yoga#1208

**The problem**
Turns out that we do not even check direction when determining which edge is the leading (start) and trailing (end) edges. This is not how web does it as the start/end is based on the writing direction NOT the flex direction: https://developer.mozilla.org/en-US/docs/Web/CSS/CSS_flexible_box_layout/Basic_concepts_of_flexbox#start_and_end_lines. While web does not have marginStart and marginEnd, they do have margin-inline-start/end which relies on the writing mode to determine the "start"/"end": https://developer.mozilla.org/en-US/docs/Web/CSS/margin-inline-start.

This means that if you do something like
```
export default function Playground(props: Props): React.Node {
  return (
    <View style={styles.container}>
      <View style={styles.item} />
    </View>
  );
}

const styles = StyleSheet.create({
  container: {
    marginEnd: 100,
    flexDirection: 'row-reverse',
    backgroundColor: 'red',
    display: 'flex',
    width: 100,
    height: 100,
  },
  item: {
    backgroundColor: 'blue',
    width: 10,
  },
});
```

You get  {F1116264350}
As you can see the margin gets applied to the left edge even thought the direction is ltr and it should be applied to the right edge.

**The solution**
I ended up fixing this by creating a new `leadingLayoutEdge` and `trailingLayoutEdge` function that take the flex direction as well as the direction. Based on the errata, the a few functions will use these new functions to determine which `YGEdge` is the starting/ending.

You might be wondering why I did not put this logic inside of `leadingEdge(flexDirection)` / `trailingEdge(flexDirection)` since other areas could potentially have the same bug like `getLeadingPadding`. These functions are a bit overloaded and there are cases where we actually want to use the flexDirection to get the edge in question. For example, many of the calls to `setLayoutPosition` in `CalculateLayout.cpp` call `leadingEdge()` / `trailingEdge()` to set the proper position for cases like row-reverse where items need to line up in a different direction.

Reviewed By: NickGerleman

Differential Revision: D50140503

fbshipit-source-id: 0375f57bada65773b2f25b32e438b8e55dec6a73
@facebook-github-bot
Copy link
Contributor

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

joevilches added a commit to joevilches/yoga that referenced this pull request Oct 12, 2023
…verse flex direction (facebook#1420)

Summary:
X-link: facebook/litho#962

X-link: facebook/react-native#40804

Pull Request resolved: facebook#1420

This stack is ultimately aiming to solve facebook#1208

**The problem**
Turns out that we do not even check direction when determining which edge is the leading (start) and trailing (end) edges. This is not how web does it as the start/end is based on the writing direction NOT the flex direction: https://developer.mozilla.org/en-US/docs/Web/CSS/CSS_flexible_box_layout/Basic_concepts_of_flexbox#start_and_end_lines. While web does not have marginStart and marginEnd, they do have margin-inline-start/end which relies on the writing mode to determine the "start"/"end": https://developer.mozilla.org/en-US/docs/Web/CSS/margin-inline-start.

This means that if you do something like
```
export default function Playground(props: Props): React.Node {
  return (
    <View style={styles.container}>
      <View style={styles.item} />
    </View>
  );
}

const styles = StyleSheet.create({
  container: {
    marginEnd: 100,
    flexDirection: 'row-reverse',
    backgroundColor: 'red',
    display: 'flex',
    width: 100,
    height: 100,
  },
  item: {
    backgroundColor: 'blue',
    width: 10,
  },
});
```

You get  {F1116264350}
As you can see the margin gets applied to the left edge even thought the direction is ltr and it should be applied to the right edge.

**The solution**
I ended up fixing this by creating a new `leadingLayoutEdge` and `trailingLayoutEdge` function that take the flex direction as well as the direction. Based on the errata, the a few functions will use these new functions to determine which `YGEdge` is the starting/ending.

You might be wondering why I did not put this logic inside of `leadingEdge(flexDirection)` / `trailingEdge(flexDirection)` since other areas could potentially have the same bug like `getLeadingPadding`. These functions are a bit overloaded and there are cases where we actually want to use the flexDirection to get the edge in question. For example, many of the calls to `setLayoutPosition` in `CalculateLayout.cpp` call `leadingEdge()` / `trailingEdge()` to set the proper position for cases like row-reverse where items need to line up in a different direction.

Reviewed By: NickGerleman

Differential Revision: D50140503

fbshipit-source-id: 79c52756f268b64b5c4d00606dd0e92df6c6ab56
joevilches added a commit to joevilches/litho that referenced this pull request Oct 12, 2023
…verse flex direction (facebook#962)

Summary:
Pull Request resolved: facebook#962

X-link: facebook/react-native#40804

X-link: facebook/yoga#1420

This stack is ultimately aiming to solve facebook/yoga#1208

**The problem**
Turns out that we do not even check direction when determining which edge is the leading (start) and trailing (end) edges. This is not how web does it as the start/end is based on the writing direction NOT the flex direction: https://developer.mozilla.org/en-US/docs/Web/CSS/CSS_flexible_box_layout/Basic_concepts_of_flexbox#start_and_end_lines. While web does not have marginStart and marginEnd, they do have margin-inline-start/end which relies on the writing mode to determine the "start"/"end": https://developer.mozilla.org/en-US/docs/Web/CSS/margin-inline-start.

This means that if you do something like
```
export default function Playground(props: Props): React.Node {
  return (
    <View style={styles.container}>
      <View style={styles.item} />
    </View>
  );
}

const styles = StyleSheet.create({
  container: {
    marginEnd: 100,
    flexDirection: 'row-reverse',
    backgroundColor: 'red',
    display: 'flex',
    width: 100,
    height: 100,
  },
  item: {
    backgroundColor: 'blue',
    width: 10,
  },
});
```

You get  {F1116264350}
As you can see the margin gets applied to the left edge even thought the direction is ltr and it should be applied to the right edge.

**The solution**
I ended up fixing this by creating a new `leadingLayoutEdge` and `trailingLayoutEdge` function that take the flex direction as well as the direction. Based on the errata, the a few functions will use these new functions to determine which `YGEdge` is the starting/ending.

You might be wondering why I did not put this logic inside of `leadingEdge(flexDirection)` / `trailingEdge(flexDirection)` since other areas could potentially have the same bug like `getLeadingPadding`. These functions are a bit overloaded and there are cases where we actually want to use the flexDirection to get the edge in question. For example, many of the calls to `setLayoutPosition` in `CalculateLayout.cpp` call `leadingEdge()` / `trailingEdge()` to set the proper position for cases like row-reverse where items need to line up in a different direction.

Reviewed By: NickGerleman

Differential Revision: D50140503

fbshipit-source-id: 3c916dd9ac89167b58983ebba382a5217cf4aace
joevilches added a commit to joevilches/litho that referenced this pull request Oct 12, 2023
…verse flex direction (facebook#962)

Summary:
Pull Request resolved: facebook#962

X-link: facebook/react-native#40804

X-link: facebook/yoga#1420

This stack is ultimately aiming to solve facebook/yoga#1208

**The problem**
Turns out that we do not even check direction when determining which edge is the leading (start) and trailing (end) edges. This is not how web does it as the start/end is based on the writing direction NOT the flex direction: https://developer.mozilla.org/en-US/docs/Web/CSS/CSS_flexible_box_layout/Basic_concepts_of_flexbox#start_and_end_lines. While web does not have marginStart and marginEnd, they do have margin-inline-start/end which relies on the writing mode to determine the "start"/"end": https://developer.mozilla.org/en-US/docs/Web/CSS/margin-inline-start.

This means that if you do something like
```
export default function Playground(props: Props): React.Node {
  return (
    <View style={styles.container}>
      <View style={styles.item} />
    </View>
  );
}

const styles = StyleSheet.create({
  container: {
    marginEnd: 100,
    flexDirection: 'row-reverse',
    backgroundColor: 'red',
    display: 'flex',
    width: 100,
    height: 100,
  },
  item: {
    backgroundColor: 'blue',
    width: 10,
  },
});
```

You get  {F1116264350}
As you can see the margin gets applied to the left edge even thought the direction is ltr and it should be applied to the right edge.

**The solution**
I ended up fixing this by creating a new `leadingLayoutEdge` and `trailingLayoutEdge` function that take the flex direction as well as the direction. Based on the errata, the a few functions will use these new functions to determine which `YGEdge` is the starting/ending.

You might be wondering why I did not put this logic inside of `leadingEdge(flexDirection)` / `trailingEdge(flexDirection)` since other areas could potentially have the same bug like `getLeadingPadding`. These functions are a bit overloaded and there are cases where we actually want to use the flexDirection to get the edge in question. For example, many of the calls to `setLayoutPosition` in `CalculateLayout.cpp` call `leadingEdge()` / `trailingEdge()` to set the proper position for cases like row-reverse where items need to line up in a different direction.

Reviewed By: NickGerleman

Differential Revision: D50140503

fbshipit-source-id: 6d74746c67f37c7600a3d9e22c825634ee2933db
@facebook-github-bot
Copy link
Contributor

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

joevilches added a commit to joevilches/yoga that referenced this pull request Oct 12, 2023
…verse flex direction (facebook#1420)

Summary:
X-link: facebook/litho#962

X-link: facebook/react-native#40804

Pull Request resolved: facebook#1420

This stack is ultimately aiming to solve facebook#1208

**The problem**
Turns out that we do not even check direction when determining which edge is the leading (start) and trailing (end) edges. This is not how web does it as the start/end is based on the writing direction NOT the flex direction: https://developer.mozilla.org/en-US/docs/Web/CSS/CSS_flexible_box_layout/Basic_concepts_of_flexbox#start_and_end_lines. While web does not have marginStart and marginEnd, they do have margin-inline-start/end which relies on the writing mode to determine the "start"/"end": https://developer.mozilla.org/en-US/docs/Web/CSS/margin-inline-start.

This means that if you do something like
```
export default function Playground(props: Props): React.Node {
  return (
    <View style={styles.container}>
      <View style={styles.item} />
    </View>
  );
}

const styles = StyleSheet.create({
  container: {
    marginEnd: 100,
    flexDirection: 'row-reverse',
    backgroundColor: 'red',
    display: 'flex',
    width: 100,
    height: 100,
  },
  item: {
    backgroundColor: 'blue',
    width: 10,
  },
});
```

You get  {F1116264350}
As you can see the margin gets applied to the left edge even thought the direction is ltr and it should be applied to the right edge.

**The solution**
I ended up fixing this by creating a new `leadingLayoutEdge` and `trailingLayoutEdge` function that take the flex direction as well as the direction. Based on the errata, the a few functions will use these new functions to determine which `YGEdge` is the starting/ending.

You might be wondering why I did not put this logic inside of `leadingEdge(flexDirection)` / `trailingEdge(flexDirection)` since other areas could potentially have the same bug like `getLeadingPadding`. These functions are a bit overloaded and there are cases where we actually want to use the flexDirection to get the edge in question. For example, many of the calls to `setLayoutPosition` in `CalculateLayout.cpp` call `leadingEdge()` / `trailingEdge()` to set the proper position for cases like row-reverse where items need to line up in a different direction.

Reviewed By: NickGerleman

Differential Revision: D50140503

fbshipit-source-id: 6c0174821b2ed9628b80cc19d1b1d4f5eb873ba2
joevilches added a commit to joevilches/react-native that referenced this pull request Oct 12, 2023
…verse flex direction (facebook#40804)

Summary:
X-link: facebook/litho#962

Pull Request resolved: facebook#40804

X-link: facebook/yoga#1420

This stack is ultimately aiming to solve facebook/yoga#1208

**The problem**
Turns out that we do not even check direction when determining which edge is the leading (start) and trailing (end) edges. This is not how web does it as the start/end is based on the writing direction NOT the flex direction: https://developer.mozilla.org/en-US/docs/Web/CSS/CSS_flexible_box_layout/Basic_concepts_of_flexbox#start_and_end_lines. While web does not have marginStart and marginEnd, they do have margin-inline-start/end which relies on the writing mode to determine the "start"/"end": https://developer.mozilla.org/en-US/docs/Web/CSS/margin-inline-start.

This means that if you do something like
```
export default function Playground(props: Props): React.Node {
  return (
    <View style={styles.container}>
      <View style={styles.item} />
    </View>
  );
}

const styles = StyleSheet.create({
  container: {
    marginEnd: 100,
    flexDirection: 'row-reverse',
    backgroundColor: 'red',
    display: 'flex',
    width: 100,
    height: 100,
  },
  item: {
    backgroundColor: 'blue',
    width: 10,
  },
});
```

You get  {F1116264350}
As you can see the margin gets applied to the left edge even thought the direction is ltr and it should be applied to the right edge.

**The solution**
I ended up fixing this by creating a new `leadingLayoutEdge` and `trailingLayoutEdge` function that take the flex direction as well as the direction. Based on the errata, the a few functions will use these new functions to determine which `YGEdge` is the starting/ending.

You might be wondering why I did not put this logic inside of `leadingEdge(flexDirection)` / `trailingEdge(flexDirection)` since other areas could potentially have the same bug like `getLeadingPadding`. These functions are a bit overloaded and there are cases where we actually want to use the flexDirection to get the edge in question. For example, many of the calls to `setLayoutPosition` in `CalculateLayout.cpp` call `leadingEdge()` / `trailingEdge()` to set the proper position for cases like row-reverse where items need to line up in a different direction.

Reviewed By: NickGerleman

Differential Revision: D50140503

fbshipit-source-id: 8c0566cc918c3a24633b0e16dfb12b3c3440a877
joevilches added a commit to joevilches/litho that referenced this pull request Oct 12, 2023
…verse flex direction (facebook#962)

Summary:
Pull Request resolved: facebook#962

X-link: facebook/react-native#40804

X-link: facebook/yoga#1420

This stack is ultimately aiming to solve facebook/yoga#1208

**The problem**
Turns out that we do not even check direction when determining which edge is the leading (start) and trailing (end) edges. This is not how web does it as the start/end is based on the writing direction NOT the flex direction: https://developer.mozilla.org/en-US/docs/Web/CSS/CSS_flexible_box_layout/Basic_concepts_of_flexbox#start_and_end_lines. While web does not have marginStart and marginEnd, they do have margin-inline-start/end which relies on the writing mode to determine the "start"/"end": https://developer.mozilla.org/en-US/docs/Web/CSS/margin-inline-start.

This means that if you do something like
```
export default function Playground(props: Props): React.Node {
  return (
    <View style={styles.container}>
      <View style={styles.item} />
    </View>
  );
}

const styles = StyleSheet.create({
  container: {
    marginEnd: 100,
    flexDirection: 'row-reverse',
    backgroundColor: 'red',
    display: 'flex',
    width: 100,
    height: 100,
  },
  item: {
    backgroundColor: 'blue',
    width: 10,
  },
});
```

You get  {F1116264350}
As you can see the margin gets applied to the left edge even thought the direction is ltr and it should be applied to the right edge.

**The solution**
I ended up fixing this by creating a new `leadingLayoutEdge` and `trailingLayoutEdge` function that take the flex direction as well as the direction. Based on the errata, the a few functions will use these new functions to determine which `YGEdge` is the starting/ending.

You might be wondering why I did not put this logic inside of `leadingEdge(flexDirection)` / `trailingEdge(flexDirection)` since other areas could potentially have the same bug like `getLeadingPadding`. These functions are a bit overloaded and there are cases where we actually want to use the flexDirection to get the edge in question. For example, many of the calls to `setLayoutPosition` in `CalculateLayout.cpp` call `leadingEdge()` / `trailingEdge()` to set the proper position for cases like row-reverse where items need to line up in a different direction.

Reviewed By: NickGerleman

Differential Revision: D50140503

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

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

joevilches added a commit to joevilches/react-native that referenced this pull request Oct 12, 2023
…verse flex direction (facebook#40804)

Summary:
X-link: facebook/litho#962

Pull Request resolved: facebook#40804

X-link: facebook/yoga#1420

This stack is ultimately aiming to solve facebook/yoga#1208

**The problem**
Turns out that we do not even check direction when determining which edge is the leading (start) and trailing (end) edges. This is not how web does it as the start/end is based on the writing direction NOT the flex direction: https://developer.mozilla.org/en-US/docs/Web/CSS/CSS_flexible_box_layout/Basic_concepts_of_flexbox#start_and_end_lines. While web does not have marginStart and marginEnd, they do have margin-inline-start/end which relies on the writing mode to determine the "start"/"end": https://developer.mozilla.org/en-US/docs/Web/CSS/margin-inline-start.

This means that if you do something like
```
export default function Playground(props: Props): React.Node {
  return (
    <View style={styles.container}>
      <View style={styles.item} />
    </View>
  );
}

const styles = StyleSheet.create({
  container: {
    marginEnd: 100,
    flexDirection: 'row-reverse',
    backgroundColor: 'red',
    display: 'flex',
    width: 100,
    height: 100,
  },
  item: {
    backgroundColor: 'blue',
    width: 10,
  },
});
```

You get  {F1116264350}
As you can see the margin gets applied to the left edge even thought the direction is ltr and it should be applied to the right edge.

**The solution**
I ended up fixing this by creating a new `leadingLayoutEdge` and `trailingLayoutEdge` function that take the flex direction as well as the direction. Based on the errata, the a few functions will use these new functions to determine which `YGEdge` is the starting/ending.

You might be wondering why I did not put this logic inside of `leadingEdge(flexDirection)` / `trailingEdge(flexDirection)` since other areas could potentially have the same bug like `getLeadingPadding`. These functions are a bit overloaded and there are cases where we actually want to use the flexDirection to get the edge in question. For example, many of the calls to `setLayoutPosition` in `CalculateLayout.cpp` call `leadingEdge()` / `trailingEdge()` to set the proper position for cases like row-reverse where items need to line up in a different direction.

Reviewed By: NickGerleman

Differential Revision: D50140503

fbshipit-source-id: 798df5354610a0347760c325962a85f135e6230d
joevilches added a commit to joevilches/yoga that referenced this pull request Oct 12, 2023
…verse flex direction (facebook#1420)

Summary:
X-link: facebook/litho#962

X-link: facebook/react-native#40804

Pull Request resolved: facebook#1420

This stack is ultimately aiming to solve facebook#1208

**The problem**
Turns out that we do not even check direction when determining which edge is the leading (start) and trailing (end) edges. This is not how web does it as the start/end is based on the writing direction NOT the flex direction: https://developer.mozilla.org/en-US/docs/Web/CSS/CSS_flexible_box_layout/Basic_concepts_of_flexbox#start_and_end_lines. While web does not have marginStart and marginEnd, they do have margin-inline-start/end which relies on the writing mode to determine the "start"/"end": https://developer.mozilla.org/en-US/docs/Web/CSS/margin-inline-start.

This means that if you do something like
```
export default function Playground(props: Props): React.Node {
  return (
    <View style={styles.container}>
      <View style={styles.item} />
    </View>
  );
}

const styles = StyleSheet.create({
  container: {
    marginEnd: 100,
    flexDirection: 'row-reverse',
    backgroundColor: 'red',
    display: 'flex',
    width: 100,
    height: 100,
  },
  item: {
    backgroundColor: 'blue',
    width: 10,
  },
});
```

You get  {F1116264350}
As you can see the margin gets applied to the left edge even thought the direction is ltr and it should be applied to the right edge.

**The solution**
I ended up fixing this by creating a new `leadingLayoutEdge` and `trailingLayoutEdge` function that take the flex direction as well as the direction. Based on the errata, the a few functions will use these new functions to determine which `YGEdge` is the starting/ending.

You might be wondering why I did not put this logic inside of `leadingEdge(flexDirection)` / `trailingEdge(flexDirection)` since other areas could potentially have the same bug like `getLeadingPadding`. These functions are a bit overloaded and there are cases where we actually want to use the flexDirection to get the edge in question. For example, many of the calls to `setLayoutPosition` in `CalculateLayout.cpp` call `leadingEdge()` / `trailingEdge()` to set the proper position for cases like row-reverse where items need to line up in a different direction.

Reviewed By: NickGerleman

Differential Revision: D50140503

fbshipit-source-id: 9ad5b36ed609e21d5b15ec9c97d17071f7b49d30
Joe Vilches and others added 2 commits October 12, 2023 14:30
Summary:
This stack is ultimately aiming to solve facebook#1208

This adds an value to the Errata enum. I will use this to gate this fix as there is potential for users to rely on this bug or have a hack in place to fix it and this would be a breaking change.

Differential Revision: D50145273

fbshipit-source-id: f652422fbea8f2127a38b63f1f54571e48b43935
…verse flex direction (facebook#1420)

Summary:
X-link: facebook/litho#962

X-link: facebook/react-native#40804

Pull Request resolved: facebook#1420

This stack is ultimately aiming to solve facebook#1208

**The problem**
Turns out that we do not even check direction when determining which edge is the leading (start) and trailing (end) edges. This is not how web does it as the start/end is based on the writing direction NOT the flex direction: https://developer.mozilla.org/en-US/docs/Web/CSS/CSS_flexible_box_layout/Basic_concepts_of_flexbox#start_and_end_lines. While web does not have marginStart and marginEnd, they do have margin-inline-start/end which relies on the writing mode to determine the "start"/"end": https://developer.mozilla.org/en-US/docs/Web/CSS/margin-inline-start.

This means that if you do something like
```
export default function Playground(props: Props): React.Node {
  return (
    <View style={styles.container}>
      <View style={styles.item} />
    </View>
  );
}

const styles = StyleSheet.create({
  container: {
    marginEnd: 100,
    flexDirection: 'row-reverse',
    backgroundColor: 'red',
    display: 'flex',
    width: 100,
    height: 100,
  },
  item: {
    backgroundColor: 'blue',
    width: 10,
  },
});
```

You get  {F1116264350}
As you can see the margin gets applied to the left edge even thought the direction is ltr and it should be applied to the right edge.

**The solution**
I ended up fixing this by creating a new `leadingLayoutEdge` and `trailingLayoutEdge` function that take the flex direction as well as the direction. Based on the errata, the a few functions will use these new functions to determine which `YGEdge` is the starting/ending.

You might be wondering why I did not put this logic inside of `leadingEdge(flexDirection)` / `trailingEdge(flexDirection)` since other areas could potentially have the same bug like `getLeadingPadding`. These functions are a bit overloaded and there are cases where we actually want to use the flexDirection to get the edge in question. For example, many of the calls to `setLayoutPosition` in `CalculateLayout.cpp` call `leadingEdge()` / `trailingEdge()` to set the proper position for cases like row-reverse where items need to line up in a different direction.

Reviewed By: NickGerleman

Differential Revision: D50140503

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

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

joevilches added a commit to joevilches/litho that referenced this pull request Oct 12, 2023
…verse flex direction (facebook#962)

Summary:
Pull Request resolved: facebook#962

X-link: facebook/react-native#40804

X-link: facebook/yoga#1420

This stack is ultimately aiming to solve facebook/yoga#1208

**The problem**
Turns out that we do not even check direction when determining which edge is the leading (start) and trailing (end) edges. This is not how web does it as the start/end is based on the writing direction NOT the flex direction: https://developer.mozilla.org/en-US/docs/Web/CSS/CSS_flexible_box_layout/Basic_concepts_of_flexbox#start_and_end_lines. While web does not have marginStart and marginEnd, they do have margin-inline-start/end which relies on the writing mode to determine the "start"/"end": https://developer.mozilla.org/en-US/docs/Web/CSS/margin-inline-start.

This means that if you do something like
```
export default function Playground(props: Props): React.Node {
  return (
    <View style={styles.container}>
      <View style={styles.item} />
    </View>
  );
}

const styles = StyleSheet.create({
  container: {
    marginEnd: 100,
    flexDirection: 'row-reverse',
    backgroundColor: 'red',
    display: 'flex',
    width: 100,
    height: 100,
  },
  item: {
    backgroundColor: 'blue',
    width: 10,
  },
});
```

You get  {F1116264350}
As you can see the margin gets applied to the left edge even thought the direction is ltr and it should be applied to the right edge.

**The solution**
I ended up fixing this by creating a new `leadingLayoutEdge` and `trailingLayoutEdge` function that take the flex direction as well as the direction. Based on the errata, the a few functions will use these new functions to determine which `YGEdge` is the starting/ending.

You might be wondering why I did not put this logic inside of `leadingEdge(flexDirection)` / `trailingEdge(flexDirection)` since other areas could potentially have the same bug like `getLeadingPadding`. These functions are a bit overloaded and there are cases where we actually want to use the flexDirection to get the edge in question. For example, many of the calls to `setLayoutPosition` in `CalculateLayout.cpp` call `leadingEdge()` / `trailingEdge()` to set the proper position for cases like row-reverse where items need to line up in a different direction.

Reviewed By: NickGerleman

Differential Revision: D50140503

fbshipit-source-id: 9cc0a3c9614c2879ad900faf9eaa142803b93781
joevilches added a commit to joevilches/react-native that referenced this pull request Oct 12, 2023
…verse flex direction (facebook#40804)

Summary:
X-link: facebook/litho#962

Pull Request resolved: facebook#40804

X-link: facebook/yoga#1420

This stack is ultimately aiming to solve facebook/yoga#1208

**The problem**
Turns out that we do not even check direction when determining which edge is the leading (start) and trailing (end) edges. This is not how web does it as the start/end is based on the writing direction NOT the flex direction: https://developer.mozilla.org/en-US/docs/Web/CSS/CSS_flexible_box_layout/Basic_concepts_of_flexbox#start_and_end_lines. While web does not have marginStart and marginEnd, they do have margin-inline-start/end which relies on the writing mode to determine the "start"/"end": https://developer.mozilla.org/en-US/docs/Web/CSS/margin-inline-start.

This means that if you do something like
```
export default function Playground(props: Props): React.Node {
  return (
    <View style={styles.container}>
      <View style={styles.item} />
    </View>
  );
}

const styles = StyleSheet.create({
  container: {
    marginEnd: 100,
    flexDirection: 'row-reverse',
    backgroundColor: 'red',
    display: 'flex',
    width: 100,
    height: 100,
  },
  item: {
    backgroundColor: 'blue',
    width: 10,
  },
});
```

You get  {F1116264350}
As you can see the margin gets applied to the left edge even thought the direction is ltr and it should be applied to the right edge.

**The solution**
I ended up fixing this by creating a new `leadingLayoutEdge` and `trailingLayoutEdge` function that take the flex direction as well as the direction. Based on the errata, the a few functions will use these new functions to determine which `YGEdge` is the starting/ending.

You might be wondering why I did not put this logic inside of `leadingEdge(flexDirection)` / `trailingEdge(flexDirection)` since other areas could potentially have the same bug like `getLeadingPadding`. These functions are a bit overloaded and there are cases where we actually want to use the flexDirection to get the edge in question. For example, many of the calls to `setLayoutPosition` in `CalculateLayout.cpp` call `leadingEdge()` / `trailingEdge()` to set the proper position for cases like row-reverse where items need to line up in a different direction.

Reviewed By: NickGerleman

Differential Revision: D50140503

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

This pull request has been merged in f4337fb.

facebook-github-bot pushed a commit to facebook/litho that referenced this pull request Oct 12, 2023
…verse flex direction (#962)

Summary:
Pull Request resolved: #962

X-link: facebook/react-native#40804

X-link: facebook/yoga#1420

This stack is ultimately aiming to solve facebook/yoga#1208

**The problem**
Turns out that we do not even check direction when determining which edge is the leading (start) and trailing (end) edges. This is not how web does it as the start/end is based on the writing direction NOT the flex direction: https://developer.mozilla.org/en-US/docs/Web/CSS/CSS_flexible_box_layout/Basic_concepts_of_flexbox#start_and_end_lines. While web does not have marginStart and marginEnd, they do have margin-inline-start/end which relies on the writing mode to determine the "start"/"end": https://developer.mozilla.org/en-US/docs/Web/CSS/margin-inline-start.

This means that if you do something like
```
export default function Playground(props: Props): React.Node {
  return (
    <View style={styles.container}>
      <View style={styles.item} />
    </View>
  );
}

const styles = StyleSheet.create({
  container: {
    marginEnd: 100,
    flexDirection: 'row-reverse',
    backgroundColor: 'red',
    display: 'flex',
    width: 100,
    height: 100,
  },
  item: {
    backgroundColor: 'blue',
    width: 10,
  },
});
```

You get  {F1116264350}
As you can see the margin gets applied to the left edge even thought the direction is ltr and it should be applied to the right edge.

**The solution**
I ended up fixing this by creating a new `leadingLayoutEdge` and `trailingLayoutEdge` function that take the flex direction as well as the direction. Based on the errata, the a few functions will use these new functions to determine which `YGEdge` is the starting/ending.

You might be wondering why I did not put this logic inside of `leadingEdge(flexDirection)` / `trailingEdge(flexDirection)` since other areas could potentially have the same bug like `getLeadingPadding`. These functions are a bit overloaded and there are cases where we actually want to use the flexDirection to get the edge in question. For example, many of the calls to `setLayoutPosition` in `CalculateLayout.cpp` call `leadingEdge()` / `trailingEdge()` to set the proper position for cases like row-reverse where items need to line up in a different direction.

Reviewed By: NickGerleman

Differential Revision: D50140503

fbshipit-source-id: 5b580c7570f6ae1e2d031971926ac4e8f52dd362
facebook-github-bot pushed a commit to facebook/react-native that referenced this pull request Oct 12, 2023
…verse flex direction (#40804)

Summary:
X-link: facebook/litho#962

Pull Request resolved: #40804

X-link: facebook/yoga#1420

This stack is ultimately aiming to solve facebook/yoga#1208

**The problem**
Turns out that we do not even check direction when determining which edge is the leading (start) and trailing (end) edges. This is not how web does it as the start/end is based on the writing direction NOT the flex direction: https://developer.mozilla.org/en-US/docs/Web/CSS/CSS_flexible_box_layout/Basic_concepts_of_flexbox#start_and_end_lines. While web does not have marginStart and marginEnd, they do have margin-inline-start/end which relies on the writing mode to determine the "start"/"end": https://developer.mozilla.org/en-US/docs/Web/CSS/margin-inline-start.

This means that if you do something like
```
export default function Playground(props: Props): React.Node {
  return (
    <View style={styles.container}>
      <View style={styles.item} />
    </View>
  );
}

const styles = StyleSheet.create({
  container: {
    marginEnd: 100,
    flexDirection: 'row-reverse',
    backgroundColor: 'red',
    display: 'flex',
    width: 100,
    height: 100,
  },
  item: {
    backgroundColor: 'blue',
    width: 10,
  },
});
```

You get  {F1116264350}
As you can see the margin gets applied to the left edge even thought the direction is ltr and it should be applied to the right edge.

**The solution**
I ended up fixing this by creating a new `leadingLayoutEdge` and `trailingLayoutEdge` function that take the flex direction as well as the direction. Based on the errata, the a few functions will use these new functions to determine which `YGEdge` is the starting/ending.

You might be wondering why I did not put this logic inside of `leadingEdge(flexDirection)` / `trailingEdge(flexDirection)` since other areas could potentially have the same bug like `getLeadingPadding`. These functions are a bit overloaded and there are cases where we actually want to use the flexDirection to get the edge in question. For example, many of the calls to `setLayoutPosition` in `CalculateLayout.cpp` call `leadingEdge()` / `trailingEdge()` to set the proper position for cases like row-reverse where items need to line up in a different direction.

Reviewed By: NickGerleman

Differential Revision: D50140503

fbshipit-source-id: 5b580c7570f6ae1e2d031971926ac4e8f52dd362
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