Remove "IsPresented/ShouldShowSplitMode" check

[drasticactions]

Description of Change

FlyoutPage contains a check for "IsPresented," a value used to either open or close the underlying flyout pane. When setting the mode "Split," the pane is opened and, I believe, intended to stay open. So, there was a check inside the flyout page to detect if "IsPresented" was being set, and if you tried to close it via code, it would cause an exception.

While this made sense when the original control was implemented, the logic for when "Split" mode is enabled and for what platforms are more complex. On Windows, depending on the window size, you may end up in "Split" mode automatically. If you implemented your state to set "IsPresented," that will then cause a crash even though the underlying code would work fine if it was passed through.

This commit tries to address this by removing this underlying check and letting the user always set "IsPresented" via code, no matter the intended behavior.

Issues Fixed

Fixes #11785 Fixes #10291

[drasticactions]

https://github.com/dotnet/maui/pull/13497/commits/6efec4ac17c5f0916fe6ed8609184a73541d0121

I removed these two tests, as they would break with my change. As I wrote above, this use case of a user setting that value via Code regardless of the current behavior is okay and would not result in a broken application.

Likewise, For WinUI and Catalyst, they are detected as running under "Tablet" mode.

[jsuarezruiz]

This PR also will fix https://github.com/dotnet/maui/issues/10291

[PureWeen]

This PR also will fix #10291

I don't think these changes are related to this issue. #10291 looks like an NRE inside the LayoutChildren call.

[PureWeen]

@drasticactions on each of the platforms does the following code work as expected?

flyoutPage.FlyoutLayoutBehavior = FlyoutLayoutBehavior.Split;
flyoutPage.IsPresented = false;

The Details page successfully takes up the full width?

I noticed on Shell it does not work :-(

shell.FlyoutBehavior = FlyoutBehavior.Locked;
shell.FlyoutIsPresented = false;

Causes the layouts to be a bit awkward.

[drasticactions]

@PureWeen

I changed FlyoutPage. Shell implements IFlyoutView so I'm not sure how my change affects that. I don't want to change Shell functionality but can you point to where that's happening through my PR?

[drasticactions]

For

flyoutPage.FlyoutLayoutBehavior = FlyoutLayoutBehavior.Split;
flyoutPage.IsPresented = false;

With my PR, for all platforms, it should close the flyout. That's intended for the reasons I gave above. Depending on the platform you're running, the default FlyoutLayoutBehavior can be Split, and code that programmatically calls IsPresented when the default is switched will cause crashes. I think addressing that is a good first step to then discussing the general design goals of this control and what should happen for each FlyoutLayoutBehavior function down the road.

[PureWeen]

@PureWeen

I changed FlyoutPage. Shell implements IFlyoutView so I'm not sure how my change affects that. I don't want to change Shell functionality but can you point to where that's happening through my PR?

I'm not saying you broke shell just that shell currently doesn't work with this combination and FlyoutPage on iOS and Windows are based on the same code as shell.

For

flyoutPage.FlyoutLayoutBehavior = FlyoutLayoutBehavior.Split;
flyoutPage.IsPresented = false;

With my PR, for all platforms, it should close the flyout. That's intended for the reasons I gave above. Depending on the platform you're running, the default FlyoutLayoutBehavior can be Split, and code that programmatically calls IsPresented when the default is switched will cause crashes. I think addressing that is a good first step to then discussing the general design goals of this control and what should happen for each FlyoutLayoutBehavior function down the road.

Based on what I tested on Shell, I'm assuming that the above code won't display correctly (since iOS and Windows FlyoutPages are basically the same as Shell). Which is why I was curious if you tested this combination?

As this PR relates to the referenced issues

https://github.com/dotnet/maui/issues/10291 is an NRE exception so I don't think this issue addresses it.

I don't think this PR will fix https://github.com/dotnet/maui/issues/11785 in a way that we can backport if the combination I've outlined above doesn't display correctly. I think we need to couple this fix with tests that indicate that the details view will layout over the entire window if you set IsPresented to false in Split mode

[drasticactions]

@PureWeen I changed FlyoutPage. Shell implements IFlyoutView so I'm not sure how my change affects that. I don't want to change Shell functionality but can you point to where that's happening through my PR?

I'm not saying you broke shell just that shell currently doesn't work with this combination and FlyoutPage on iOS and Windows are based on the same code as shell.

Okay, I think I understand what you're saying, I'll break it out and tell me if I got it:

This is what happens with Shell right now, when it's locked.

And what happens when it's locked, and you force set FlyoutIsPresented to false, this happens.

This is unrelated to my change, that's what it does. I need to debug deeper but I think we're either setting that specific padding somewhere or it's WinUIs NavigationView default for a docked NavMenu (It supports being fully open, docked, or closed and opened as a flyover).

Having said that, my change will cause FlyoutPage to operate in a similar way, since it's then matching what Shell does. While it's not ideal, IMO, it's better than a crash; A layout issue can be worked around in the PlatformView handler, where this crash can not be.

[drasticactions]

@PureWeen Is this PR okay to merge? Do you have more concerns?

[PureWeen]

@drasticactions There's very little chance this will get backported to .NET7, seeing as how the current crash is basically "as designed". So, I don't think there's any need to rush a fix for the crash without fixing the other scenarios.

AFAICT the crash explains the rules of the FlyoutPage accurately.

If someone wants to work around the crash, then they need to change the FlyoutLayoutBehavior to PopOver and then set IsPresented to false. I tested that on the sample, and it works fine.

While it's not ideal, IMO, it's better than a crash; A layout issue can be worked around in the PlatformView handler, where this crash can not be.

This isn't entirely accurate. If someone wants to work around the current crash they can provide an implementation for IFlyoutPageController.CanChangeIsPresented and have it always return true. That will effectively match what you are doing in the PR here.

[PureWeen]

@drasticactions once we merge https://github.com/dotnet/maui/pull/12909 let's revisit this one

I think once https://github.com/dotnet/maui/pull/12909 is fixed we can remove these checks and then users can do whatever they want.

We'll need to make sure it works alright on Android/WinUI though first

[drasticactions]

@drasticactions once we merge #12909 let's revisit this one

I think once #12909 is fixed we can remove these checks and then users can do whatever they want.

We'll need to make sure it works alright on Android/WinUI though first

Testing on iOS/Catalyst/Android/WinUI, I set up a Flyout Page and ran through the various settings, programmatically closing the flyout and letting the defaults close and open it as needed. I couldn't get a crash. The only weirdness was in WinUI with what we showed above, which already exists and is unrelated to my change.

[drasticactions]

@PureWeen Any thoughts?

[rmarinho]

Can we close this one @drasticactions @PureWeen ?

[PureWeen]

Closing this issue for now. The core of this issue came from bugs in the iOS FlyoutPage implementation. The heart of this issue is still accurate, but we need to fix issues with some of the combinations this creates before enabling this.