Skip to content

Add method to invoke nested Yoga layout#10237

Closed
rozele wants to merge 4 commits intomicrosoft:mainfrom
rozele:issue7601
Closed

Add method to invoke nested Yoga layout#10237
rozele wants to merge 4 commits intomicrosoft:mainfrom
rozele:issue7601

Conversation

@rozele
Copy link
Contributor

@rozele rozele commented Jul 5, 2022

Description

Type of Change

Erase all that don't apply.

  • New feature (non-breaking change which adds functionality)

Why

Yoga does not recurse layout beyond a node that provides a self-measurement function. If you want to build a custom control that allows for XAML layout with descendants from React that still use Yoga layout, you need to kick off a new layout calculation from each "inner root".

Resolves #7601

What

This change adds a hook to self-measuring shadow nodes to conditionally measure any nested children that may require Yoga layout, as well as an ABIViewManager interface to participate in these events.

Microsoft Reviewers: Open in CodeFlow

@rozele rozele requested review from a team as code owners July 5, 2022 15:39
@ghost ghost added the Area: Layout label Jul 5, 2022
@asklar
Copy link
Member

asklar commented Jul 5, 2022

I think in order to use this API, your native VM must implement a panel so that you can override layout, is that right?
can you provide a sample of how this API is to be used? Ideally if you just want to have a native VM for e.g. a XAML Grid, you shouldn't need to declare a subclass of Grid just so that you can override measure/arrange

@rozele
Copy link
Contributor Author

rozele commented Jul 5, 2022

@asklar - indeed - there's not really any alternative for this. I'll work up a sample at some point.

@rozele rozele marked this pull request as draft August 15, 2022 14:54
@rozele
Copy link
Contributor Author

rozele commented Aug 15, 2022

We're considering other options for handling re-entrant / nested layout. Converting to draft for now.

@rozele
Copy link
Contributor Author

rozele commented Aug 22, 2022

Note: dependent on #10422, leaving in draft state until #10422 is merged

rozele added a commit to rozele/react-native-windows that referenced this pull request Aug 22, 2022
Core VMs can override `SetLayoutProps` to get a callback just before
native layout values are applied from Yoga. There is no option to
observe these changes from Yoga in ABI VMs.

This change introduces a new interface to receive a callback when
computed Yoga layout values are being applied to a native view.

We have a use case for this interface that is also dependent on microsoft#10422 (ensuring parent SetLayoutProps occur
after children), microsoft#10237 (adding an API to invoke nested Yoga layout for
a given React tag), and microsoft#10393 (custom measure func). Specifically, the following sequence occurs:
1. Prop updates cause Yoga layout to be applied on a root node
2. SetLayoutProps is invoked on children of custom VM,
Top/Left/Width/Height get set
3. SetLayoutProps/OnLayout is called on the custom VM
4. Custom VM runs logic to re-caclulate dimensions of each child in
OnLayout
5. Custom VM invokes XamlUIService::ApplyYogaLayout with computed
dimensions on each child.
6. Custom VM applies Top/Left values to children.

The custom measure function is used to prevent children from having Yoga
layout computed twice.
@rozele rozele marked this pull request as ready for review August 22, 2022 15:17
@rozele
Copy link
Contributor Author

rozele commented Aug 22, 2022

Please note, this change is also going to be useful should we ever support nested Views in Text with RichTextBlock, as we'll need an option to invoke Yoga layout on a specific sub-tree from TextViewManager.

@asklar - this is the best example I can think of without adding a custom VM to playground. We don't have any such examples yet, so not planning on setting that up for this PR. The sequence of events would look something like the following to implement nested Views in Text.

  1. View added as descendant of Text in React JS
  2. TextViewManager or VirtualTextViewManager detects child in not Inline but some type of UIElement
  3. Node where child is added notifies TextViewManager that it needs to replace the native view with RichTextBlock
  4. View is added to RichTextBlock via InlineUIContainer
  5. On subsequent SetLayoutProps call, state in TextViewManager shadow node knows that we need to recursively invoke Yoga for the nested Views
  6. Nodes in Text inline tree are visited, when we hit an InlineUIContainer, we invoke ApplyYogaLayout on NativeUIManager.

Another good thing to note is that this type of thing is already supported on the public surface for Android and iOS view managers / shadow nodes, so strictly speaking it's a bug / gap that we don't support this in RNW.

rozele added a commit to rozele/react-native-windows that referenced this pull request Aug 22, 2022
Core VMs can override `SetLayoutProps` to get a callback just before
native layout values are applied from Yoga. There is no option to
observe these changes from Yoga in ABI VMs.

This change introduces a new interface to receive a callback when
computed Yoga layout values are being applied to a native view.

We have a use case for this interface that is also dependent on microsoft#10422 (ensuring parent SetLayoutProps occur
after children), microsoft#10237 (adding an API to invoke nested Yoga layout for
a given React tag), and microsoft#10393 (custom measure func). Specifically, the following sequence occurs:
1. Prop updates cause Yoga layout to be applied on a root node
2. SetLayoutProps is invoked on children of custom VM,
Top/Left/Width/Height get set
3. SetLayoutProps/OnLayout is called on the custom VM
4. Custom VM runs logic to re-caclulate dimensions of each child in
OnLayout
5. Custom VM invokes XamlUIService::ApplyYogaLayout with computed
dimensions on each child.
6. Custom VM applies Top/Left values to children.

The custom measure function is used in this example to prevent children from having Yoga
layout computed twice.
rozele added a commit to rozele/react-native-windows that referenced this pull request Aug 22, 2022
Core VMs can override `SetLayoutProps` to get a callback just before
native layout values are applied from Yoga. There is no option to
observe these changes from Yoga in ABI VMs.

This change introduces a new interface to receive a callback when
computed Yoga layout values are being applied to a native view.

We have a use case for this interface that is also dependent on microsoft#10422 (ensuring parent SetLayoutProps occur
after children), microsoft#10237 (adding an API to invoke nested Yoga layout for
a given React tag), and microsoft#10393 (custom measure func). Specifically, the following sequence occurs:
1. Prop updates cause Yoga layout to be applied on a root node
2. SetLayoutProps is invoked on children of custom VM,
Top/Left/Width/Height get set
3. SetLayoutProps/OnLayout is called on the custom VM
4. Custom VM runs logic to re-caclulate dimensions of each child in
OnLayout
5. Custom VM invokes XamlUIService::ApplyYogaLayout with computed
dimensions on each child.
6. Custom VM applies Top/Left values to children.

The custom measure function is used in this example to prevent children from having Yoga
layout computed twice.
rozele added a commit to rozele/react-native-windows that referenced this pull request Aug 22, 2022
Core VMs can override `SetLayoutProps` to get a callback just before
native layout values are applied from Yoga. There is no option to
observe these changes from Yoga in ABI VMs.

This change introduces a new interface to receive a callback when
computed Yoga layout values are being applied to a native view.

We have a use case for this interface that is also dependent on microsoft#10422 (ensuring parent SetLayoutProps occur
after children), microsoft#10237 (adding an API to invoke nested Yoga layout for
a given React tag), and microsoft#10393 (custom measure func). Specifically, the following sequence occurs:
1. Prop updates cause Yoga layout to be applied on a root node
2. SetLayoutProps is invoked on children of custom VM,
Top/Left/Width/Height get set
3. SetLayoutProps/OnLayout is called on the custom VM
4. Custom VM runs logic to re-caclulate dimensions of each child in
OnLayout
5. Custom VM invokes XamlUIService::ApplyYogaLayout with computed
dimensions on each child.
6. Custom VM applies Top/Left values to children.

The custom measure function is used in this example to prevent children from having Yoga
layout computed twice.
@rozele rozele force-pushed the issue7601 branch 2 times, most recently from 21aae46 to eb4f768 Compare August 22, 2022 20:43
"Invokes Yoga layout for a specific React tag. "
"This method is useful for views that use @IViewManagerRequiresNativeLayout "
"but have descendant views that require Yoga layout.")
void ApplyYogaLayout(Int64 tag, Single width, Single height);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why is this in XamlUIService though? Seems more appropriate for the react context ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair enough. Wasn't sure where to put it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps a better approach would be to create a new IDL for UIManager module and expose that from ReactContext. We could then add things like GetReactRootView, ApplyYogaLayout, a callback for batch completion for native modules, etc.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another good question - why is DispatchEvent in XamlUIService? cc @acoates-ms. It looks to me like XamlUIService has become a dumping ground for APIs that we don't know where else to put :). I'll file a new issue to clean up XamlUIService (deprecate the API and move everything to APIs that are a bit more uniform in usage scenario).

@rozele
Copy link
Contributor Author

rozele commented Aug 24, 2022

Based on conversation with @asklar - moving to draft to confirm this does not break rn-xaml.

@rozele rozele marked this pull request as draft August 24, 2022 19:44
acoates-ms pushed a commit that referenced this pull request Sep 12, 2022
* Adds IViewManagerWithOnLayout interface for ABI VMs

Core VMs can override `SetLayoutProps` to get a callback just before
native layout values are applied from Yoga. There is no option to
observe these changes from Yoga in ABI VMs.

This change introduces a new interface to receive a callback when
computed Yoga layout values are being applied to a native view.

We have a use case for this interface that is also dependent on #10422 (ensuring parent SetLayoutProps occur
after children), #10237 (adding an API to invoke nested Yoga layout for
a given React tag), and #10393 (custom measure func). Specifically, the following sequence occurs:
1. Prop updates cause Yoga layout to be applied on a root node
2. SetLayoutProps is invoked on children of custom VM,
Top/Left/Width/Height get set
3. SetLayoutProps/OnLayout is called on the custom VM
4. Custom VM runs logic to re-caclulate dimensions of each child in
OnLayout
5. Custom VM invokes XamlUIService::ApplyYogaLayout with computed
dimensions on each child.
6. Custom VM applies Top/Left values to children.

The custom measure function is used in this example to prevent children from having Yoga
layout computed twice.

* Change files

* Ensure we set the IViewManagerWithOnLayout instance to ABIViewManager

* yarn format
@rozele rozele force-pushed the issue7601 branch 2 times, most recently from 1548210 to b2c24d1 Compare October 4, 2022 15:36
@chiaramooney chiaramooney requested a review from asklar November 2, 2022 23:40
@rozele rozele force-pushed the issue7601 branch 2 times, most recently from cb264e0 to 37fc6c5 Compare November 11, 2022 21:20
@chiaramooney
Copy link
Contributor

ping @chrisglein

@chiaramooney chiaramooney added the Needs: Attention 👋 An issue that had been tagged "Needs: Author Feedback" has received activity (label applied by bot) label Mar 29, 2023
Comment on lines +13 to +23
const [hover1, setHover1] = useState(false);
const [hover2, setHover2] = useState(false);
const [hover3, setHover3] = useState(false);
const [hover4, setHover4] = useState(false);

const getHoverProps = (setter: (value: boolean) => void) => {
return {
onMouseEnter: () => setter(true),
onMouseLeave: () => setter(false),
};
};
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Took a moment to realize why you were doing this hover text changing. The reason I take it is that you have a XAML item (GridViewItem) with a Yoga-driven child that's changing layout size (due to the font change).

Adding a comment to describe the why of this would help with the maintainability of the code.

m_viewManagerWithChildren{viewManager.try_as<IViewManagerWithChildren>()},
m_viewManagerWithPointerEvents{viewManager.try_as<IViewManagerWithPointerEvents>()},
m_viewManagerWithDropViewInstance{viewManager.try_as<IViewManagerWithDropViewInstance>()},
m_viewManagerWithOnLayout{viewManager.try_as<IViewManagerWithOnLayout>()} {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't see m_viewManagerWithOnLayout being removed, but it's no longer being set here. Is that a problem?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only use is in this same file:

if (m_viewManagerWithOnLayout) {
    m_viewManagerWithOnLayout.OnLayout(viewToUpdate.as<xaml::FrameworkElement>(), left, top, width, height);

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ouch, good catch.

@chrisglein
Copy link
Member

Change makes sense to me in that your handing off the responsibilities to the ViewManager to deal with this whole business. The one code example you have here just has one child and calls ApplyLayout manually on them. What has this looked like with your more complicated ViewManagers? (I'm assuming you've applied this code to Messenger) What would it look like to implement in codegen like react-native-xaml?

I'm prone to accept this change, but want to set up the right guidance for folks to write CalculateLayoutOnChildren correctly. What would the documentation for that look like? With that documented, this change looks good.

@chrisglein chrisglein added Needs: Author Feedback The issue/PR needs activity from its author (label drives bot activity) and removed Needs: Attention 👋 An issue that had been tagged "Needs: Author Feedback" has received activity (label applied by bot) labels Jul 12, 2023
@rozele
Copy link
Contributor Author

rozele commented Jul 17, 2023

What has this looked like with your more complicated ViewManagers? (I'm assuming you've applied this code to Messenger)

In the Messenger example, it's a bit simpler. It doesn't implicate XAML layout at all. There's a set of self-measuring leaf nodes in our call window example, where the grid layout for the call window is some cross-platform C++ algorithm. Each of these self-measuring grids has nested React content in it, so we use this to allow re-entrancy for Yoga layout.

What would it look like to implement in codegen like react-native-xaml?

It's a good question. I would think that in react-native-xaml, we might do something like the following:

  1. Add a custom DependencyProperty to every component generated by react-native-xaml
  2. For every "parent" component in react-native-xaml, we codegen the CalculateLayoutOnChildren interface / method
  3. Inside that method, we check each child for the custom DependencyProperty
  4. If the custom DependencyProperty is present, we take no action, we let XAML "do it's thing"
  5. If the custom DependencyProperty is not present, we perform this re-entry layout by waiting for something like the SizeChanging event and calling ApplyLayout. This of course is not ideal, because SizeChanging fires out of band from the layout pass, so the children would get updated in a subsequent layout pass, causing possible tearing. I guess the alternative is that every react-native-xaml component gets some kind of derived XAML component type, and we invoke ApplyLayout in ArrangeOverride / MeasureOverride to keep it in a single layout pass.

This breaks down pretty quickly if anyone wanted to add a module that interoperates with react-native-xaml though. So it might be better for this custom DependencyProperty to be part of React Native, and get automatically assigned to any IViewManagerRequiresNativeLayout ABIViewManager component.

Alternatively, we could do this on behalf of all possible XAML layout interoperable modules (via SizeChanged + ApplyLayout) so react-native-xaml gets this "for free" at the expense of tearing.

The reason we went with the more flexible approach of asking the user to invoke ApplyLayout directly was so that we could decide the layout constraints passed to Yoga, rather than assuming that all available width/height gets passed to Yoga (which we would need to do if we wanted to create a baseline functionality for this). It's not a deal breaker, but one option is certainly more "flexible" than the other (at the expense of more code).

@microsoft-github-policy-service microsoft-github-policy-service bot added Needs: Attention 👋 An issue that had been tagged "Needs: Author Feedback" has received activity (label applied by bot) and removed Needs: Author Feedback The issue/PR needs activity from its author (label drives bot activity) labels Jul 17, 2023
@rozele
Copy link
Contributor Author

rozele commented Jul 17, 2023

What would the documentation for that look like?

It's a good question. I don't think any of the IViewManager interfaces are very well documented today. Our internal documentation is basically "go look at some other module that is already using the interface" :(

This is the closest thing you have today: https://microsoft.github.io/react-native-windows/docs/view-managers This needs to be broken down by interface, giving an example of how it's implemented and what it's useful for.

Also, IMO, there's no better way to document something than to give an example of it's use, but discoverability is certainly an issue.

@rozele
Copy link
Contributor Author

rozele commented Jul 17, 2023

@chrisglein The more important consideration here though is how this will work with Fabric. React Native Windows already has a nice abstraction for this (i.e., LayoutService) where we can route calls to either Paper or Fabric depending on which UIManager is managing the node. So, LayoutService::ApplyLayout could still work with Fabric, but we'll need to eventually update LayoutService to trigger Yoga layout on Fabric node sub-trees.

@jonthysell
Copy link
Contributor

@chrisglein can you follow up on @rozele 's answers and approve if this is ready?

Copy link
Member

@chrisglein chrisglein left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

High level questions for this did get answered in the comment thread. But there are some (small) edits suggested in the code itself that should still be addressed before merging.

@microsoft-github-policy-service microsoft-github-policy-service bot added the Needs: Author Feedback The issue/PR needs activity from its author (label drives bot activity) label Jan 17, 2024
@microsoft-github-policy-service microsoft-github-policy-service bot added the no-recent-activity Issue/PR has gone stale and may be closed (label applied by bot) label Jan 25, 2024
@jonthysell jonthysell removed the Needs: Attention 👋 An issue that had been tagged "Needs: Author Feedback" has received activity (label applied by bot) label Aug 1, 2024
@microsoft-github-policy-service microsoft-github-policy-service bot removed the no-recent-activity Issue/PR has gone stale and may be closed (label applied by bot) label Aug 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area: Layout Needs: Author Feedback The issue/PR needs activity from its author (label drives bot activity)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Can't nest layouts

5 participants