react-native-windows icon indicating copy to clipboard operation
react-native-windows copied to clipboard

Adds optional API for window sizing root

Open rozele opened this issue 3 years ago • 2 comments

Description

Type of Change

Erase all that don't apply.

  • Bug fix (non-breaking change which fixes an issue)

Why

It may not always be the case that the XamlRoot is the same size as the HWND. For example, in our app, to work around https://github.com/microsoft/microsoft-ui-xaml/issues/2101, we resize the XamlRoot content to be larger than the window by the DPI scale factor and then resizes the ReactRootView to be the same size as the HWND. However, this breaks the LogBoxModule and the AlertModule, which make the assumption that the XamlRoot is the same size as the window.

Resolves #10211

What

To workaround this assumption, this change adds an API to XamlUIService that allows you to specify which view under XamlRoot is equivalent to the window size for the purpose of sizing things like the ContentDialog smoke screen and the LogBox popup.

Testing

As a demonstration example, I changed the Playground-win32 app to set the ReactRootView as the window sizing root and forced the ReactRootView to be smaller than the XamlRoot grid. You'll notice that the LogBox popup and ContentDialog smoke screen still attach to the window at the origin (they are not offset to the position of the window sizing root), but this is not something I plan to change as the intention of the window sizing root is that it actually matches the size of the window (so no offsets should be required):

https://user-images.githubusercontent.com/1106239/176501861-d53483d8-d6c9-467e-bd76-d5177c523b5b.mp4

Here's the same working correctly when the ReactRootView actually matches the size of the HWND:

https://user-images.githubusercontent.com/1106239/176502260-705d743c-ee0b-4afb-b63b-f33234742e24.mp4

Optional: Describe the tests that you ran locally to verify your changes.

Microsoft Reviewers: Open in CodeFlow

rozele avatar Jun 29 '22 17:06 rozele

This pull request has been automatically marked as stale because it has been marked as requiring author feedback but has not had any activity for 7 days. It will be closed if no further activity occurs within 7 days of this comment.

ghost avatar Jul 08 '22 00:07 ghost

@asklar How about something like this:

winrt::Size XamlUIService::GetWindowSize() void XamlUIService::OnWindowSizeChanged(std::function<void()> callback) void XamlUIService::SetWindowSize(winrt::Size size)

Eventually we need to improve XamlUIService multi-window support, so these APIs may eventually take a int64_t rootTag parameter as well, or create a per root XamlUIService instance, but we can address that later.

rozele avatar Jul 19 '22 14:07 rozele

@rozele @asklar Did that offline conversation end up happening? How can unstick this PR?

chrisglein avatar Dec 14 '22 19:12 chrisglein

It did not. Presumably some API will be needed for Fabric that doesn't depend on XAML, so we'll migrate to that once it's available. This can be closed if there's no near term interest for Paper.

rozele avatar Dec 20 '22 04:12 rozele

Discussed in PR Triage. No near term interest for this change on Paper.

chiaramooney avatar Apr 12 '23 23:04 chiaramooney