Skip to content
This repository has been archived by the owner on May 1, 2024. It is now read-only.

[Bug] MasterDetailPage MasterBehavior override does not work. #6498

Open
DeerSteak opened this issue Jun 11, 2019 · 13 comments
Open

[Bug] MasterDetailPage MasterBehavior override does not work. #6498

DeerSteak opened this issue Jun 11, 2019 · 13 comments

Comments

@DeerSteak
Copy link

DeerSteak commented Jun 11, 2019

Description

MasterDetailPage.MasterBehavior = MasterBehavior.Popover does not work on iOS. On Android phones, the Detail page is dimmed and the menu covers the page. On iOS, regardless of how MasterBehavior is set, iOS always uses a slide-over animation that's messy and gross because it's hard to tell where the Master ends and the Detail begins. None of the overrides seem to work correctly on iOS, for that matter.

Steps to Reproduce

  1. Download the attached sample project. It's just the Xamarin.Forms sample MasterDetail template with one minor change.
  2. Note that MasterBehavior is set to Popover in the MainPage.xaml file
  3. Run it on iOS
  4. Open the hamburger menu
  5. The Detail slides over instead of hte menu popping over.

Expected Behavior

That the Detail would dim and the Master would slide out (pop over it)

Actual Behavior

The Detail slides over and the menu doesn't dim. The Menu page slides out and it's hard to tell where the Menu ends and the Detail begins.

Basic Information

  • Version with issue: 4.0.latest
  • Last known good version: N/A
  • IDE: Visual Studio 2017 15.9.12 (also happens on VS 2019 Mac)
  • Platform Target Frameworks:
    • iOS: Default (12, I think)
    • Android:
    • UWP:
  • Android Support Library Version:
  • Nuget Packages: Only Xamarin.Forms and Xamarin.iOS (and dependencies)
  • Affected Devices: iPhone

Screenshots

iOS Simulator
visual studio

Reproduction Link

MasterDetailExample.zip

@DeerSteak DeerSteak added s/unverified New report that has yet to be verified t/bug 🐛 labels Jun 11, 2019
@pauldipietro pauldipietro added this to New in Triage Jun 11, 2019
@jfversluis
Copy link
Member

jfversluis commented Jun 13, 2019

As far as I know this is the default behavior for iOS and it is a restriction by Apple's native components that on an iPhone the MasterBehavior does not have any influence.

You can even see in our source that there are two types of renderers, one for phone and one for tablet. In the tablet one you see something happening with the behavior, in the phone one it is not taken into account since it will not have any effect any way.

There might be an exception for iPhone Plus variants, but I think only in landscape mode.

@DeerSteak
Copy link
Author

DeerSteak commented Jun 13, 2019

@jfversluis MasterDetailPage implements a UISplitViewController on tablets but not phones. Apple's documentation makes no distinction between phone and tablet.

But this is great because your answer gave me an idea to implement. Now that you've pointed it out, I realized can most likely create a "custom" renderer by creating my own class from the TabletMasterDetailRenderer source.

So that's a potential workaround.

@SebastianKruseWago
Copy link

SebastianKruseWago commented Jun 13, 2019

@jfversluis The iPad variant is also implemented a wrong way because it breaks the SplitView which is available since iOS 9. See: #3085 and #2017. Beside that: No, Apple has no restrictions on that. For example the "9GAG"-App, "Amazon"-App (and the "Sparkasse"-App (only available in Germany I think) as well) is using popover instead slide as well on iPhones.

@DeerSteak I don't would recommand copying from the current TabletMasterDetailRenderer due to the bug mentioned above (except your app requires the full screen anyway).

@DeerSteak
Copy link
Author

DeerSteak commented Jun 13, 2019

Whatever Shell does (which it does have a popover for iOS), MasterDetailPage should be able to do. I started to look at the difference between the ShellRenderer and the PhoneMasterDetailRenderer but that's a huge rabbit hole I don't have time for. :-/

This failed spectacularly as @SebastianKruseWago predicted. (I realized I could just subclass the TabletMasterDetailRenderer)

[assembly: ExportRenderer(typeof(MasterDetailPage), typeof(CustomPhoneMasterDetailRenderer))]
namespace MasterDetailExample.iOS.Renderers
{
    public class CustomPhoneMasterDetailRenderer : TabletMasterDetailRenderer
    {        
        
    }
}

@jfversluis
Copy link
Member

Point taken, apparently I was sorely mistaken about how iOS handles this. I must have mixed it up with some other functionality. Looks like we have to have a look at the whole MasterBehavior for both phones and tablets.

@SebastianKruseWago
Copy link

@jfversluis If it helps: The last known "good" version for SplitView was 2.3.4-sr2. This might be related to this issue because I think PreferredDisplayMode and MasterBehavior should not be different.

@DeerSteak
Copy link
Author

@jfversluis thanks so much for looking into it. :)

@samhouts samhouts added e/7 🕖 7 and removed s/unverified New report that has yet to be verified labels Jun 29, 2019
@samhouts samhouts moved this from New to Ready For Work in Triage Jun 29, 2019
@samhouts samhouts added this to To do in iOS Ready For Work Jun 29, 2019
@samhouts samhouts removed this from Ready For Work in Triage Jun 29, 2019
@FIELDPOINT
Copy link

ANy updates?

@Gakk
Copy link

Gakk commented Nov 25, 2019

Any expectation of when/if this issue will be fixed?

@SebastianKruseWago
Copy link

@FIELDPOINT @Gakk I don't think we will get any answer soon. But you might want to compare it with the SplitView bug on iPad's with Xamarin.Forms and MasterDetail. This is broken and reported since 2015 and still not fixed (it was reported in Bugzilla the first time).

@ChaseFlorell
Copy link

@jfversluis is this issue priority enough that you'd accept a PR if one was submitted? I ask because the PR process into Forms can be laborious and long-running. If one were to put effort into fixing this, I'd hope to see it reviewed and merged with some sense of urgency.

I'm sure someone on this thread would be willing to log the time to get it done, but our time is valuable and so I'd hope the effort wouldn't be wasted with code being hung up in PR review.

@RLittlesII
Copy link

Did someone say something about hired hands? @ChaseFlorell 😄

@jfversluis
Copy link
Member

@ChaseFlorell I understand where you're coming from. I hope you know not a single PR is wasted and we appreciate all and any time that anyone spends on this project.

Having that said; I'm currently no longer involved with the Forms team so I cant really make promises in this regard. Sorry.

I'm happy to have a look when there is a PR and provide a review and try to help things move along as fast as possible.

@samhouts samhouts added this to the 5.0.0 milestone Aug 13, 2020
@samhouts samhouts added this to To do in vNext+1 (5.0.0) Aug 14, 2020
@samhouts samhouts removed this from the 5.0.0 milestone Nov 2, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
Development

No branches or pull requests

8 participants