Simplify Flyout and Popup coordinate spaces (#11822)

rozele · web-flow · commit d60370603779 · 2023-10-25T15:34:33.000-07:00
* Simplify Flyout and Popup coordinate spaces

We are seeing some issues with pointer event handling in XAML Islands
apps. It appears that part of the issue has to do with the coordinate
spaces not exactly lining up. It is too difficult to reason about the
coordinate spaces when there are minor differences in measurement
behavior. The coordinate spaces for TouchEventHandler and
UIManager.measure need to line up exactly for Pressable to work as
expected (as Pressable calls UIManager.measure to ensure the pointer is
still in the hit box after a pointerMove event occurs).

TouchEventHandler transforms the RoutedPointerEventArgs
pointer position to the view passed to the TouchEventHandler:

UIManager.measure measures against either the ReactRootView or the view
corresponding to the last ancestor before a ShadowNode with IsWindowed
set to true.

This latter logic is the part in UIManager.measure is the part that
complicates the reasoning about coordinate spaces. Even though it's
designed to measure against the same native component, it's an extra
(and arguably unnecessary) step to figuring out if the coordinate spaces
match. It's much simpler to assume that all coordinate spaces are
relative to the ReactRootView.

This change should probably be marked as breaking, even though it could
conceivably be considered a bug fix. Breaking in part because the X/Y
positions returned from UIManager.measure will no longer be relative to
the Flyout or Popup root. Also breaking because `pointerEvents` values
for nodes above the Flyout will now impact the Flyout.

For example:
```
&lt;View pointerEvents="none"&gt;
  &lt;Flyout isOpen={true}&gt;
    &lt;Button title="Click Me" onPress={() =&gt; alert("Pressed")} /&gt;
  &lt;/Flyout&gt;
&lt;/View&gt;
```

Previously, the Flyout content would not respect the ancestors
`pointerEvents` value. With this change, the pointer event will
propagate through the entire tree, and respect any
`"box-none"` or `"none"` props for ancestors in the main window.

* Change files
diff --git a/change/react-native-windows-4a09e136-8bbd-4b99-95ab-fdaed43e9292.json b/change/react-native-windows-4a09e136-8bbd-4b99-95ab-fdaed43e9292.json
@@ -0,0 +1,7 @@
+{
+  "type": "prerelease",
+  "comment": "Simplify Flyout and Popup coordinate spaces",
+  "packageName": "react-native-windows",
+  "email": "email not defined",
+  "dependentChangeType": "patch"
+}
diff --git a/vnext/Microsoft.ReactNative/Modules/NativeUIManager.cpp b/vnext/Microsoft.ReactNative/Modules/NativeUIManager.cpp
@@ -999,34 +999,11 @@ void NativeUIManager::measure(
     return;
   }
 
-  // Traverse up the react node tree to find any windowed popups.
-  // If there are none, then we use the top-level root provided by our caller.
-  xaml::FrameworkElement feRootView = nullptr;
-  int64_t rootTag = shadowNode.m_tag;
-  int64_t childTag = rootTag;
-  while (true) {
-    auto &currNode = m_host->GetShadowNodeForTag(rootTag);
-    if (currNode.m_parent == InvalidTag)
-      break;
-    ShadowNodeBase &rootNode = static_cast<ShadowNodeBase &>(currNode);
-    if (rootNode.IsWindowed()) {
-      ShadowNodeBase &childNode = static_cast<ShadowNodeBase &>(m_host->GetShadowNodeForTag(childTag));
-      feRootView = childNode.GetView().try_as<xaml::FrameworkElement>();
-      break;
-    }
-    childTag = currNode.m_tag;
-    rootTag = currNode.m_parent;
-  }
-
+  // Retrieve the XAML element for the root view containing this view
+  auto feRootView = static_cast<ShadowNodeBase &>(shadowRoot).GetView().try_as<xaml::FrameworkElement>();
   if (feRootView == nullptr) {
-    // Retrieve the XAML element for the root view containing this view
-    if (auto xamlRootView = static_cast<ShadowNodeBase &>(shadowRoot).GetView()) {
-      feRootView = xamlRootView.as<xaml::FrameworkElement>();
-    }
-    if (feRootView == nullptr) {
-      m_context.JSDispatcher().Post([callback = std::move(callback)]() { callback(0, 0, 0, 0, 0, 0); });
-      return;
-    }
+    m_context.JSDispatcher().Post([callback = std::move(callback)]() { callback(0, 0, 0, 0, 0, 0); });
+    return;
   }
 
   winrt::Rect rectInParentCoords = GetRectOfElementInParentCoords(feView, feRootView);
diff --git a/vnext/Microsoft.ReactNative/Views/FlyoutViewManager.cpp b/vnext/Microsoft.ReactNative/Views/FlyoutViewManager.cpp
@@ -114,10 +114,6 @@ class FlyoutShadowNode : public ShadowNodeBase {
   winrt::Flyout GetFlyout();
   void AdjustDefaultFlyoutStyle(float maxWidth, float maxHeight);
 
-  bool IsWindowed() override {
-    return true;
-  }
-
  private:
   void SetTargetFrameworkElement();
   winrt::Popup GetFlyoutParentPopup() const;
@@ -152,7 +148,7 @@ FlyoutShadowNode::~FlyoutShadowNode() {
 
 void FlyoutShadowNode::AddView(ShadowNode &child, int64_t /*index*/) {
   auto childView = static_cast<ShadowNodeBase &>(child).GetView();
-  m_touchEventHandler->AddTouchHandlers(childView);
+  m_touchEventHandler->AddTouchHandlers(childView, GetRootView());
   m_previewKeyboardEventHandlerOnRoot->hook(childView);
 
   if (m_flyout != nullptr) {
diff --git a/vnext/Microsoft.ReactNative/Views/PopupViewManager.cpp b/vnext/Microsoft.ReactNative/Views/PopupViewManager.cpp
@@ -44,10 +44,6 @@ class PopupShadowNode : public ShadowNodeBase {
   void UpdateTabStops();
   void UpdateLayout();
 
-  bool IsWindowed() override {
-    return true;
-  }
-
  private:
   bool m_autoFocus{true};
   double m_verticalOffset{0};
@@ -132,7 +128,7 @@ void PopupShadowNode::AddView(ShadowNode &child, int64_t index) {
 
   control.Content(childView);
 
-  m_touchEventHandler->AddTouchHandlers(childView);
+  m_touchEventHandler->AddTouchHandlers(childView, GetRootView());
   m_previewKeyboardEventHandlerOnRoot->hook(childView);
 }
 
diff --git a/vnext/Microsoft.ReactNative/Views/ShadowNodeBase.cpp b/vnext/Microsoft.ReactNative/Views/ShadowNodeBase.cpp
@@ -143,6 +143,17 @@ void ShadowNodeBase::UpdateTransformPS() {
   }
 }
 
+XamlView ShadowNodeBase::GetRootView() {
+  if (auto uiManager = GetNativeUIManager(GetViewManager()->GetReactContext()).lock()) {
+    auto shadowNode = uiManager->getHost()->FindShadowNodeForTag(m_rootTag);
+    if (!shadowNode)
+      return nullptr;
+
+    return static_cast<::Microsoft::ReactNative::ShadowNodeBase *>(shadowNode)->GetView();
+  }
+  return nullptr;
+}
+
 void ShadowNodeBase::UpdateHandledKeyboardEvents(
     std::string const &propertyName,
     winrt::Microsoft::ReactNative::JSValue const &value) {
diff --git a/vnext/Microsoft.ReactNative/Views/ShadowNodeBase.h b/vnext/Microsoft.ReactNative/Views/ShadowNodeBase.h
@@ -98,9 +98,6 @@ struct REACTWINDOWS_EXPORT ShadowNodeBase : public ShadowNode {
   int64_t GetParent() const {
     return m_parent;
   }
-  virtual bool IsWindowed() {
-    return false;
-  }
 
   void ReplaceView(XamlView view);
 
@@ -136,6 +133,8 @@ struct REACTWINDOWS_EXPORT ShadowNodeBase : public ShadowNode {
   bool m_isDisabled = false;
   comp::CompositionPropertySet m_transformPS{nullptr};
 
+  XamlView GetRootView();
+
  public:
   double m_padding[(int)ShadowEdges::CountEdges] = INIT_UNDEFINED_EDGES;
   double m_border[(int)ShadowEdges::CountEdges] = INIT_UNDEFINED_EDGES;
diff --git a/vnext/Microsoft.ReactNative/Views/TextViewManager.cpp b/vnext/Microsoft.ReactNative/Views/TextViewManager.cpp
@@ -180,17 +180,6 @@ class TextShadowNode final : public ShadowNodeBase {
     }
   }
 
-  XamlView GetRootView() {
-    if (auto uiManager = GetNativeUIManager(GetViewManager()->GetReactContext()).lock()) {
-      auto shadowNode = uiManager->getHost()->FindShadowNodeForTag(m_rootTag);
-      if (!shadowNode)
-        return nullptr;
-
-      return static_cast<::Microsoft::ReactNative::ShadowNodeBase *>(shadowNode)->GetView();
-    }
-    return nullptr;
-  }
-
   TextTransform textTransform{TextTransform::Undefined};
   std::shared_ptr<bool> selectionChanged = std::make_shared<bool>(false);
 };
