search

microsoft / react-native-windows

A framework for building native Windows apps with React.
https://microsoft.github.io/react-native-windows/
Other
16.24k stars 1.14k forks source link

Flyout full placement mode is busted #2969

Closed kmelmon closed 4 years ago

kmelmon commented 5 years ago

Flyout has a 'full' placement mode that centers the Flyout. This isn't working - it is being positioned at [0,0].

Repro Steps: Launch RNTester Go to Flyout test page Choose 'full' from placement dropdown Open the Flyout

RESULT The Flyout is positioned at (0,0) in the control instead of centered.

The issue is that we're overriding MaxWidth/MaxHeight to 5000x5000 and the XAML logic for positioning the flyout is getting confused in full placement mode.

kmelmon commented 5 years ago

I've been working on a fix for this.

kmelmon commented 5 years ago

@khetanashita is going to try building a custom flyout based on Luke Longley's investigation.

KAnder425 commented 4 years ago

Additonal debugging.... The full placement mode of the RNW Flyout correctly centers most uses of the Flyout.  The only scenario in which it fails to center properly is if the Flyout contains a Picker.    When there is no Picker involved, SetLayoutProps (with the correct width/height) is called on the flyout during the opening layout of the flyout, and then, when layout is complete, FlyoutBase::PerformPlacement is triggered.  PerformPlacement does some calculations using the flyout presenters maxSize settings and the available core window size to figure out where to offset the Flyout in full placement mode. Since the max width/height were set to match the layout of the Flyout during the call to SetLayoutProps prior to the call to PerfomPlacement, everything centers nicely.   However, if a Picker is embedded in the Flyout, the code flow changes in an important way.  The reason is due to the workaround fix that we did to handle the Yoga/XAML layout problems with the Pickers;   https://github.com/microsoft/react-native-windows/pull/2859 and https://github.com/microsoft/react-native-windows/pull/2584   In that workaround fix, if a Picker is involved, we force a call to UpdateLayout prior to running the layout pass. UpdateLayout has  an interesting side-effect in that it triggers a size change event on the flyout which, in turn, triggers a premature PerformPlacement when in full placement mode. Instead of PerformPlacement being called after the call to SetLayoutProps, it's called before layout is complete. As a result, the maxSize settings for the flyout presenter are still at the maximum settings (50000/50000) and the flyout is incorrectly positioned at the top left corner of the core window.

The upshot of this is that if we fix the underlying Yoga/XAML layout problems as referenced in https://github.com/microsoft/react-native-windows/issues/2643, this will also fix the Flyout's full placement centering problem.

khetanashita commented 4 years ago

Ken worked on this issue and checked in a fix. Closing it.

kmelmon commented 4 years ago

@khetanashita @KAnder425 was this something additional to the temporary workaround done in #3640? I didn't see any checkins to the RNW repo... If there's a more permanent solution, we should get that checked into master.