Skip to content

Rework modal to be implemented entirely using public APIs #14256

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.

Sign up for GitHub

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

Merged
acoates-ms merged 15 commits into from
Jan 10, 2025
Merged

Rework modal to be implemented entirely using public APIs #14256

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
c4c6699
Rework modal implementation to use public APIs
acoates-ms Jan 8, 2025
7b630b6
shutdown fix
acoates-ms Jan 8, 2025
ab100c2
Change files
acoates-ms Jan 8, 2025
4d60990
format
acoates-ms Jan 8, 2025
fa2f6ed
remove dead code
acoates-ms Jan 8, 2025
2beb8c4
format
acoates-ms Jan 8, 2025
ea55f07
fixes
acoates-ms Jan 8, 2025
12c9808
fix
acoates-ms Jan 8, 2025
e0e1a40
UIA tree for root component should be kept distinct from main UIA tree
acoates-ms Jan 8, 2025
6950636
input offset fix for sub rootviews
acoates-ms Jan 8, 2025
2273c7a
Split modal into two componentview's so that we dont have multiple ro…
acoates-ms Jan 10, 2025
1f7c367
format
acoates-ms Jan 10, 2025
0a939ab
Ensure rootview removes itself from the island, before a new one adds…
acoates-ms Jan 10, 2025
bf6683d
defork some modal test pages
acoates-ms Jan 10, 2025
092bcb2
remove override
acoates-ms Jan 10, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 11 additions & 27 deletions vnext/Microsoft.ReactNative/Fabric/Composition/CompositionEventHandler.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,9 +142,8 @@ struct CompositionInputKeyboardSource : winrt::implements<

CompositionEventHandler::CompositionEventHandler(
const winrt::Microsoft::ReactNative::ReactContext &context,
const winrt::Microsoft::ReactNative::ReactNativeIsland &reactNativeIsland,
const int fragmentTag)
: m_fragmentTag(fragmentTag), m_context(context), m_wkRootView(reactNativeIsland) {}
const winrt::Microsoft::ReactNative::ReactNativeIsland &reactNativeIsland)
: m_context(context), m_wkRootView(reactNativeIsland) {}

void CompositionEventHandler::Initialize() noexcept {
#ifdef USE_WINUI3
Expand Down Expand Up @@ -365,6 +364,9 @@ void CompositionEventHandler::onPointerWheelChanged(
facebook::react::Point ptLocal;
facebook::react::Point ptScaled = {static_cast<float>(position.X), static_cast<float>(position.Y)};

// In the case of a sub rootview, we may have a non-zero origin. hitTest takes a pt in the parent coords, so we
// need to apply the current origin
ptScaled += RootComponentView().layoutMetrics().frame.origin;
auto tag = RootComponentView().hitTest(ptScaled, ptLocal);

if (tag == -1)
Expand Down Expand Up @@ -977,6 +979,11 @@ void CompositionEventHandler::getTargetPointerArgs(
tag = -1;

ptScaled = {position.X, position.Y};

// In the case of a sub rootview, we may have a non-zero origin. hitTest takes a pt in the parent coords, so we need
// to apply the current origin
ptScaled += RootComponentView().layoutMetrics().frame.origin;
Copy link
Contributor

@PPatBoyd PPatBoyd Jan 9, 2025

Choose a reason for hiding this comment

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

comment says the hitTest pt is taken in parent coordinates and we're incorporating the origin in the subrootview (separate window) for it hit test... for the other lines I was thinking "should this origin be incorporated inside hitTest instead of called before it, if it's the RootComponentView()'s layoutMetrics?" ... then saw the if(capturedPointers) { ... } that's more curious. When do we have m_capturedPointers vs not?

Is the underlying type of facebook::react::Point a float? Considering the implications of different DPI on different displays which can do curious things in some contexts


if (std::find(m_capturedPointers.begin(), m_capturedPointers.end(), pointerId) != m_capturedPointers.end()) {
assert(m_pointerCapturingComponentTag != -1);
tag = m_pointerCapturingComponentTag;
Expand All @@ -989,30 +996,7 @@ void CompositionEventHandler::getTargetPointerArgs(
ptLocal.y = ptScaled.y - (clientRect.top / strongRootView.ScaleFactor());
}
} else {
if (m_fragmentTag == -1) {
tag = RootComponentView().hitTest(ptScaled, ptLocal);
return;
}

// check if the fragment tag exists
if (!fabricuiManager->GetViewRegistry().findComponentViewWithTag(m_fragmentTag)) {
return;
}

auto fagmentView = fabricuiManager->GetViewRegistry().componentViewDescriptorWithTag(m_fragmentTag).view;
auto fagmentchildren = fagmentView.Children();

// call the hitTest with the fargment as the RootComponent
for (auto index = fagmentchildren.Size(); index > 0; index--) {
auto childView = fagmentchildren.GetAt(index - 1);
auto targetTag =
winrt::get_self<winrt::Microsoft::ReactNative::implementation::ComponentView>(childView)->hitTest(
ptScaled, ptLocal);
if (targetTag != -1) {
tag = targetTag;
break;
}
}
tag = RootComponentView().hitTest(ptScaled, ptLocal);
}
}

Expand Down
4 changes: 1 addition & 3 deletions vnext/Microsoft.ReactNative/Fabric/Composition/CompositionEventHandler.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,7 @@ class CompositionEventHandler : public std::enable_shared_from_this<CompositionE
public:
CompositionEventHandler(
const winrt::Microsoft::ReactNative::ReactContext &context,
const winrt::Microsoft::ReactNative::ReactNativeIsland &ReactNativeIsland,
const int fragmentTag);
const winrt::Microsoft::ReactNative::ReactNativeIsland &ReactNativeIsland);
virtual ~CompositionEventHandler();

void Initialize() noexcept;
Expand Down Expand Up @@ -152,7 +151,6 @@ class CompositionEventHandler : public std::enable_shared_from_this<CompositionE

std::map<PointerId, ActiveTouch> m_activeTouches; // iOS is map of touch event args to ActiveTouch..?
PointerId m_touchId = 0;
int m_fragmentTag = -1;

std::map<PointerId, std::vector<ReactTaggedView>> m_currentlyHoveredViewsPerPointer;
winrt::weak_ref<winrt::Microsoft::ReactNative::ReactNativeIsland> m_wkRootView;
Expand Down
5 changes: 2 additions & 3 deletions vnext/Microsoft.ReactNative/Fabric/Composition/ReactNativeIsland.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -455,7 +455,7 @@ void ReactNativeIsland::InitRootView(

m_context = winrt::Microsoft::ReactNative::ReactContext(std::move(context));
m_reactViewOptions = std::move(viewOptions);
m_CompositionEventHandler = std::make_shared<::Microsoft::ReactNative::CompositionEventHandler>(m_context, *this, -1);
m_CompositionEventHandler = std::make_shared<::Microsoft::ReactNative::CompositionEventHandler>(m_context, *this);
m_CompositionEventHandler->Initialize();

UpdateRootViewInternal();
Expand All @@ -477,8 +477,7 @@ void ReactNativeIsland::AddFragmentCompositionEventHandler(
if (!m_CompositionEventHandler) {
// Create CompositionEventHandler if not already created
m_context = winrt::Microsoft::ReactNative::ReactContext(context);
m_CompositionEventHandler =
std::make_shared<::Microsoft::ReactNative::CompositionEventHandler>(m_context, *this, componentView.Tag());
m_CompositionEventHandler = std::make_shared<::Microsoft::ReactNative::CompositionEventHandler>(m_context, *this);
m_CompositionEventHandler->Initialize();
m_isInitialized = true;
}
Expand Down
4 changes: 4 additions & 0 deletions vnext/Microsoft.ReactNative/Fabric/Composition/RootComponentView.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -266,6 +266,10 @@ winrt::IInspectable RootComponentView::UiaProviderFromPoint(const POINT &ptPixel
static_cast<facebook::react::Float>(ptPixels.y) / m_layoutMetrics.pointScaleFactor};

facebook::react::Point localPt;

// In the case of a sub rootview, we may have a non-zero origin. hitTest takes a pt in the parent coords, so we need
// to apply the current origin
ptDips += m_layoutMetrics.frame.origin;
auto tag = hitTest(ptDips, localPt, true);

auto uiManager = ::Microsoft::ReactNative::FabricUIManager::FromProperties(m_reactContext.Properties());
Expand Down
Loading