forked from facebook/litho
-
Notifications
You must be signed in to change notification settings - Fork 0
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Fix issue where marginStart and marginEnd were not working with rowRe…
…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
- Loading branch information
1 parent
c3db860
commit 12f2423
Showing
4 changed files
with
125 additions
and
38 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters