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 start/end would not be respected in flex edge getters #1479

Closed
wants to merge 6 commits into from

Commits on Dec 4, 2023

  1. Insets no longer apply to statically positioned nodes (facebook#1454)

    Summary:
    
    X-link: facebook/react-native#41369
    
    One of the most basic aspects of statically positioned nodes is that [insets do not apply to them](https://developer.mozilla.org/en-US/docs/Web/CSS/position#static). So I put a guard inside `Node::relativePosition` where we take that into account when setting the position.
    
    Reviewed By: NickGerleman
    
    Differential Revision: D50507808
    Joe Vilches authored and facebook-github-bot committed Dec 4, 2023
    Configuration menu
    Copy the full SHA
    45fff62 View commit details
    Browse the repository at this point in the history
  2. Let containing blocks layout their absolute descendants, not parents (f…

    …acebook#1470)
    
    Summary:
    X-link: facebook/react-native#41488
    
    
    The way we plan on implementing `position: static` is by changing how we lay out absolutely positioned nodes. Instead of letting their direct parent lay them out we are going to let their containing block handle it. This is useful because by the time the containing block gets to this step it will already know its size, which is needed to ensure that absolute nodes can get the right value with percentage units. Additionally, it means that we can "translate" the position of the absolute nodes to be relative to their parent fairly easily, instead of some second pass that would not be possible with a different design.
    
    This change just gets the core pieces of this process going. It makes it so that containing blocks will layout out absolute descendants that they contain. We also pass in the containing block size to the owner size args for `layoutAbsoluteChild`. This new path will only happen if we have the errata turned off. If there is no positioned ancestor for a given node we just assume the root is. This is not exactly how it works on the web - there is a notion of an initial containing block - but we are not implementing that as of right now.
    
    Reviewed By: NickGerleman
    
    Differential Revision: D51182593
    Joe Vilches authored and facebook-github-bot committed Dec 4, 2023
    Configuration menu
    Copy the full SHA
    c435db5 View commit details
    Browse the repository at this point in the history
  3. Allow the containing block to set trailing position of absolute desce…

    …ndants (facebook#1471)
    
    Summary:
    X-link: facebook/react-native#41489
    
    
    If we are going to allow the containing block to layout its absolute descendants and NOT the direct parent then we need to change step 11 which is concerned with setting the trailing position in the case we are row or column reverse. This is the very last step in the function and is positioned that way because it operates on the assumption that all children have their position set by this time. That is no longer a valid assumption if CBs layout their absolute children. In that case the CB also needs to take care of setting the position here.
    
    Because of this problem I moved some things around. It now works like:
    
    * If errata is set, the direct parent will set trailing position for all non absolute children in step 11
    * If errata is set the CB will set trailing position of absolute descendants after they are laid out inside of layoutAbsoluteDescendants
    
    Reviewed By: NickGerleman
    
    Differential Revision: D51217291
    Joe Vilches authored and facebook-github-bot committed Dec 4, 2023
    Configuration menu
    Copy the full SHA
    ec599a9 View commit details
    Browse the repository at this point in the history
  4. Use containing block to adjust absolute child position (facebook#1472)

    Summary:
    X-link: facebook/react-native#41490
    
    
    This change has most of the logic needed for supporting `position: static`. We do two things here that fix a lot of the broken static test:
    
    1) We pass in the containing node to `layoutAbsoluteChild` and use it to properly position the child in the case that insets are defined.
    2) We rewrite the absolute child's position to be relative to it's parent in the event that insets are defined for that child (and thus it is positioned relative to its CB). Yoga's layout position has always be relative to parent, so I feel it is easier to just adjust the coordinates of a node to adhere to that design rather than change the consumers of yoga.
    
    The "hard" part of this algorithm is determining how to iterate the offset from the containing block needed to do this translation described above. That is handled in `layoutAbsoluteDescendants`.
    
    Reviewed By: NickGerleman
    
    Differential Revision: D51224327
    Joe Vilches authored and facebook-github-bot committed Dec 4, 2023
    Configuration menu
    Copy the full SHA
    b355e9a View commit details
    Browse the repository at this point in the history
  5. Introduce positionAbsoluteChild (facebook#1473)

    Summary:
    
    X-link: facebook/react-native#41491
    
    To simplify the logic a bit I introduce a new function called `positionAbsoluteChild`. This function will, eventually, be the **sole function that matters** when determining the layout position of an absolute node. Because [absolute nodes do not participate in flex layout](https://drafts.csswg.org/css-flexbox/#abspos-items), we can determine the position of said node independently of its siblings. The only information we need are the node itself, its parent, and its containing block - which we have all of in `layoutAbsoluteChild`.
    
    Right now, however, this is purely a BE change with no functionality different. There was a big set of if statements at the end of `layoutAbsoluteChild` that would position the node on the main and cross axis for certain cases. The old code had it so that the main and cross axis had basically the same logic but the code was repeated. This puts that logic, as is, in `positionAbsoluteChild` and calls that from `layoutAbsoluteChild`.
    
    I will soon edit this function to actually do what it is envisioned to do (i.e. be the sole place that position is set for absolute nodes).
    
    Reviewed By: NickGerleman
    
    Differential Revision: D51272855
    Joe Vilches authored and facebook-github-bot committed Dec 4, 2023
    Configuration menu
    Copy the full SHA
    a674dc8 View commit details
    Browse the repository at this point in the history
  6. Fix issue where start/end would not be respected in flex edge getters (

    …facebook#1479)
    
    Summary:
    
    X-link: facebook/react-native#41682
    
    There are two ways to get the value of a style for a specific edge right now:
    
    1) From the inline start/end edge which is determined via the writing direction (ltr or rtl), assuming you do not have errata on
    2) From the flex start/end edge which is determined via the flex direction (row, row-reverse, column, column-reverse)
    
    There is a weird curiosity in the second case: you can define a style to be on the "start" or "end" edge when writing the stylex/css. The physical edge that this refers to is dependent on the writing direction. So `start` would be `left` in `ltr` and `right` in `rtl`, with `end` the opposite. It is **never** determined via the flex direction. Additionally, `start`/`end` takes precedence over the physical edge it corresponds to in the case both are defined.
    
    So, all of this means that to actually get the value of a style from the flex start/end edges, we need to account for the case that one of these relative edges was defined and would overwrite any physical edge. Since this mapping is solely determined by the writing direction, we need to pass that in to all the flex start/end getters and do that logic. This is done in  `flexStartRelativeEdge`/`flexEndRelativeEdge` which was added earlier but for some reason only being used on border.
    
    Reviewed By: NickGerleman
    
    Differential Revision: D51293315
    joevilches authored and facebook-github-bot committed Dec 4, 2023
    Configuration menu
    Copy the full SHA
    d17f697 View commit details
    Browse the repository at this point in the history