Bug 42548 - MasterDetailPage loses hamburger menu when reusing NavigationPage as Detail
Summary: MasterDetailPage loses hamburger menu when reusing NavigationPage as Detail
Status: RESOLVED ANSWERED
Alias: None
Product: Forms
Classification: Xamarin
Component: Forms (show other bugs)
Version: 2.3.1
Hardware: PC Windows
: Highest normal
Target Milestone: ---
Assignee: Bugzilla
URL:
Depends on:
Blocks:
 
Reported: 2016-07-14 03:46 UTC by Brian Lagunas
Modified: 2017-10-01 14:52 UTC (History)
10 users (show)

See Also:
Tags: ac
Is this bug a regression?: ---
Last known good build:


Attachments
image showing the bug (99.06 KB, image/jpeg)
2016-07-14 03:46 UTC, Brian Lagunas
Details
App to explore iconography/text of non-root detail pages. (293.61 KB, application/x-zip-compressed)
2017-07-18 17:10 UTC, Chris King
Details

Description Brian Lagunas 2016-07-14 03:46:25 UTC
Created attachment 16660 [details]
image showing the bug

Using a MasterDetailPage; when using a NavigationPage as the Detail, and reusing it instead of creating a new instance, the hamburger icon is lost when you set NavigationPage.SetHasBackButton(this, false) on the Pages.  To reproduce the issue run this application:

    public class App : Application
    {
        public App()
        {
            MainPage = new MainMasterDetailPage();
        }
    }

    public class MainMasterDetailPage : MasterDetailPage
    {
        NavigationPage _navPage;

        public MainMasterDetailPage()
        {
            Title = "Main";

            var btnViewA = new Button() { Text = "ViewA" };
            btnViewA.Clicked += Button_Clicked;

            var btnViewB = new Button() { Text = "ViewB" };
            btnViewB.Clicked += Button_Clicked;

            var stack = new StackLayout();
            stack.Children.Add(btnViewA);
            stack.Children.Add(btnViewB);

            var master = new ContentPage() { Title = "Master", Content = stack };

            _navPage = new NavigationPage(new ViewA());

            Master = master;
            Detail = _navPage;
        }

        private async void Button_Clicked(object sender, EventArgs e)
        {
            Page newRoot = null;

            var btn = (Button)sender;
            if (btn.Text == "ViewA")
                newRoot = new ViewA();
            else
                newRoot = new ViewB();

            await _navPage.Navigation.PushAsync(newRoot);

            IsPresented = false;
        }
    }

    public class ViewA : ContentPage
    {
        public ViewA()
        {
            //hide the back button, and only show the hamburger
            //no hambuger is shown
            NavigationPage.SetHasBackButton(this, false);

            Title = "ViewA";
            Content = new Label() { Text = "View A" };
        }
    }

    public class ViewB : ContentPage
    {
        public ViewB()
        {
            //hide the back button, and only show the hamburger
            //no hambuger is shown
            NavigationPage.SetHasBackButton(this, false);

            Title = "ViewB";
            Content = new Label() { Text = "View B" };
        }
    }
Comment 1 Paul DiPietro [MSFT] 2016-07-15 17:24:26 UTC
This should be clarified to only be occurring with AppCompat.
Comment 2 Lee McPherson 2016-10-10 00:04:53 UTC
The only temporary workaround I have found is to remove the previous page just before adding a new one so the NavigationStack count stays at one.  (Check for Device.OS == TargetPlatform.Android first.)
Comment 3 adrianknight89 2016-10-16 19:29:39 UTC
Brian,

Are you trying to change navigation root with each click? Inside Button_Clicked, you have a variable named "newRoot", but instead of setting it as the root page, you're pushing it on the navigation stack.

If you did:

_navPage.Navigation.InsertPageBefore(newRoot,_navPage.Navigation.NavigationStack.First());
await _navPage.Navigation.PopToRootAsync();

hamburger icon stays.

Hiding the back button works. Hamburger icon is gone in your example because the navigation stack isn't reset with each push.
Comment 4 adrianknight89 2016-10-16 19:32:07 UTC
That said, I think Lee was getting at the same thing. I just want to understand the end goal.
Comment 5 Brian Lagunas 2016-10-17 19:07:49 UTC
No, this is not about resetting the NavigationPage.NavigationStack to always having one Page.  This is about not showing the back button, only show the hamburger button, when pushing more than one page onto the NavigationPage.NavigationStack.
Comment 6 Brian Lagunas 2017-05-18 04:46:22 UTC
Is there any update on a fix?
Comment 7 Chris King 2017-07-18 17:06:42 UTC
For a navigation page acting as the detail of an MDP what iconography/text could/should/is displayed? 

Currently, by default, for the root page, all platforms provide an icon/text that causes the master page to become visible: On Android/UWP its a hamburger icon while on iOS its the name of the detail page. However, if a page is pushed onto the navigation stack then the behavior differs: On Android/iOS iconography/text for showing the master page is replaced with iconography/text for popping the stack while on UWP the hamburger continues to be presented (bug?).

Next, if the navigation stack has more than the root page and NavigationPage.SetHasBackButton is used to attempt to suppress the pop iconography/text what happens? On Android/iOS platforms the iconography/text it suppressed (UWP currently doesn't present any pop iconography/text so the operation is a no-op) however it is not replaced with iconography/text that causes the master page to be presented similar to the root page (this is the reported issue but only for Android, see comment 2).

All this said, it's not clear (to me at least, at this moment) what the correct behavior is (hence this dump). If the navigation stack has more then the root then the title bar could display iconogrpahy/text to pop the stack _or_ show the master page _or_ nothing at all. 

If I had to guess, XF never expected to support iconography/text for displaying the master page on non-root detail pages; XF Android/iOS behavior is expected per the design goals at the time (and UWP should match). That said, choosing to display iconography/text for popping if after calling SetHasBackButton to false would be a breaking change. Instead, we'd need to add something like SetHasDisplayMasterButton.

I'll circle back with Jason and discuss.
Comment 8 Chris King 2017-07-18 17:10:35 UTC
Created attachment 23634 [details]
App to explore iconography/text of non-root detail pages.
Comment 9 Chris King 2017-07-18 21:45:56 UTC
Talked with Jason and as it stands now the XF Android/iOS behavior is as designed and UWP should be changed to match; so we consider this a feature (to extend our API to enable showing iconography/text to expand the master page in addition to a back button) and have added it to our backlog.
Comment 10 NMackay 2017-07-19 08:15:15 UTC
The whole hamburger behavior is inconsistent and has caused us to seriously restrict our app design as navigating within a detail page can allow the user the change the root detail page and you can't reliably clear the pages opened.

When you have a detail page wrapped in in a navigation that pushes another page the hamburger should be replaced with a back button and swipe right disabled (in Android & iOS anyway), as it stands, there will be many apps out there with memory leaks.

To say it's resolved and is on the backlog means it'll never be looked at.
Comment 11 Nicholas Bauer 2017-10-01 14:52:20 UTC
What's the intended navigation model of the MasterDetailPage, then, and how does it differ from NavigationPage alone?

Is it's purpose just to have a format that looks better on tablets, or is it intended to allow the user to choose any Master item from any place in the application?

The latter is a natural use of MasterDetailPage and works on tablets and PCs. Should the phone implementation really be no better than a NavigationPage where the Master page is the root?

Note You need to log in before you can comment on or make changes to this bug.