Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed NavigationPage TitleBar visibility issue when set to the parent page #25000

Closed
wants to merge 6 commits into from

Conversation

Vignesh-SF3580
Copy link
Contributor

@Vignesh-SF3580 Vignesh-SF3580 commented Sep 30, 2024

Root Cause

The NavigationBar stayed visible because only the current page's HasNavigationBar property was checked, ignoring the parent
NavigationPage's setting when it was false.

Description of Change

The fix now checks both the parent NavigationPage and current page’s HasNavigationBar property. This ensures the NavigationBar is hidden when either the parent or current page has HasNavigationBar set to false.

Issues Fixed

Fixes #13807
Fixes #16816

Tested the behaviour in the following platforms

  • Android
  • Windows
  • iOS
  • Mac

Screenshots

Before
image

After
image

@dotnet-policy-service dotnet-policy-service bot added the community ✨ Community Contribution label Sep 30, 2024
@Vignesh-SF3580 Vignesh-SF3580 marked this pull request as ready for review October 3, 2024 10:46
@Vignesh-SF3580 Vignesh-SF3580 requested a review from a team as a code owner October 3, 2024 10:46
@jfversluis
Copy link
Member

/azp run

Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@PureWeen
Copy link
Member

PureWeen commented Oct 3, 2024

/azp run

Copy link

Azure Pipelines successfully started running 3 pipeline(s).

public void NavigationBarVisibility()
{
App.WaitForElement("label");
VerifyScreenshot();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Triggered the build, it should generate the reference snapshot for this test.

@jsuarezruiz
Copy link
Contributor

/azp run

Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@jsuarezruiz
Copy link
Contributor

/azp run

Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@jsuarezruiz
Copy link
Contributor

/rebase

@PureWeen
Copy link
Member

PureWeen commented Oct 9, 2024

/azp run

Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@PureWeen
Copy link
Member

PureWeen commented Oct 9, 2024

I think this one is going to require some additional design decisions. For example, if you nest the contentpage inside another layer

<TabbedPage
    xmlns="http://schemas.microsoft.com/dotnet/2021/maui"
    xmlns:x="http://schemas.microsoft.com/winfx/2009/xaml"
    x:Class="Maui.Controls.Sample.MainPage"
    xmlns:local="clr-namespace:Maui.Controls.Sample"
    NavigationPage.HasNavigationBar="False"
    >

    <TabbedPage>
        <ContentPage Title="Title 1"></ContentPage>
    </TabbedPage>
    <TabbedPage>
        <ContentPage Title="Title 2"></ContentPage>
    </TabbedPage>
</TabbedPage>

This fix doesn't really work.

Generally, with any extension properties it has to be applied to the ContentPage being displayed. If we want this to work hierarchically then we'd need to apply this logic to all of the attached properties for consistency

image

Plus we'd want to apply this logic to TabbedPage extension properties as well.

One way users can get around this is by setting a resource on the TabbedPage that will set this on all ContentPages

 <TabbedPage.Resources>
        <Style TargetType="ContentPage">
            <Setter Property="NavigationPage.HasNavigationBar" Value="False" />
        </Style>
    </TabbedPage.Resources>

I'm going to close this one for now. We can look at making all of these attached properties inheritable at a later point in time

@PureWeen PureWeen closed this Oct 9, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Nov 9, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
4 participants