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

[iOS] Shell/NavigationPage TitleView #20959

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

kubaflo
Copy link
Contributor

@kubaflo kubaflo commented Mar 2, 2024

Fixes #9333
Fixes #19231
Fixes #5063

Current behavior

Layout options have no effect on the title view. The content inside the titleView behaves as it has both HorizontalOptions and VerticalOptions set to Fill. Also, notice that there's a horizontal margin that cannot be removed even with setting the negative values to margin eg. Margin="-20,0" (#19231)

Currently

Screenshot 2024-03-02 at 14 58 41

Proposed behavior #5063

Utilizes our current LayoutOptions to indicate how you want a TitleView to layout on the screen and removes the default horizontal margin so that we can customize it.

<NavigationPage.TitleView>
    <HorizontalStackLayout BackgroundColor="red">
        <Label VerticalOptions="Center" Text="Navigation page's title"/>
        <Image Source="dotnet_bot.png" Scale="0.8"/>
    </HorizontalStackLayout>
</NavigationPage.TitleView>

Default - (horizontal and vertical options set to fill implicitly)

Screenshot 2024-03-02 at 15 00 41

HorizontalOptions="Start"

Screenshot 2024-03-02 at 15 02 14

HorizontalOptions="Center"

Screenshot 2024-03-02 at 15 04 44

HorizontalOptions="End"

Screenshot 2024-03-02 at 15 05 47

VerticalOptions="Start"

Screenshot 2024-03-02 at 15 09 49

VerticalOptions="Center"

Screenshot 2024-03-02 at 15 07 58

VerticalOptions="End"

Screenshot 2024-03-02 at 15 06 56

VerticalOptions="Start" HorizontalOptions="Start"

Screenshot 2024-03-02 at 15 15 30

VerticalOptions="Center" HorizontalOptions="Center"

Screenshot 2024-03-02 at 15 14 42

VerticalOptions="End" HorizontalOptions="End"

Screenshot 2024-03-02 at 15 15 07

Remarks

I didn't implement anything that changes the behavior when additional toolbar items are added. For example, if there's a back button the title view places itself to the right of this button. It is because I'm not sure what is the desired behavior. On one hand, it looks weird that when we want to center the title it is moved to the right, but on the other, it is the default iOS's behavior and I believe a future discussion should be made xamarin/Xamarin.Forms#4848
Screenshot 2024-03-02 at 16 10 55

@kubaflo kubaflo requested a review from a team as a code owner March 2, 2024 15:11
@ghost ghost added the community ✨ Community Contribution label Mar 2, 2024
@ghost
Copy link

ghost commented Mar 2, 2024

Hey there @kubaflo! Thank you so much for your PR! Someone from the team will get assigned to your PR shortly and we'll get it reviewed.

@kubaflo kubaflo changed the title [iOS] Shell/NavigationPage TitleView (#5063) [iOS] Shell/NavigationPage TitleView Mar 3, 2024
@jsuarezruiz jsuarezruiz added area-controls-shell Shell Navigation, Routes, Tabs, Flyout area-controls-titleview TitleView labels Mar 4, 2024
@jsuarezruiz
Copy link
Contributor

/azp run MAUI-UITests-public

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@Pastajello
Copy link

Pastajello commented Mar 29, 2024

Oh my, never knew I'd see a fix from the issue I suffered from 4 years ago in X.F. days! Are there any plans to add it to .net 8 SR 4? (hopefully)

@@ -1765,6 +1766,7 @@ class Container : UIView
IPlatformViewHandler _child;
UIImageView _icon;
bool _disposed;
const int SystemMargin = 16;
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you include a comment with a link to the Apple docs or similar?

else if (screenWidth >= 414)
{
// 5.5 inch
spacer.Width -= 4;
Copy link
Contributor

Choose a reason for hiding this comment

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

Have you done tests with 6.1" and 6.7" simulators or devices?

@jsuarezruiz
Copy link
Contributor

/azp run

Copy link

Azure Pipelines successfully started running 3 pipeline(s).

Copy link
Member

@PureWeen PureWeen left a comment

Choose a reason for hiding this comment

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

can you retarget this for net9?

@kubaflo kubaflo changed the base branch from main to net9.0 April 30, 2024 18:04
@kubaflo
Copy link
Contributor Author

kubaflo commented Apr 30, 2024

can you retarget this for net9?

Yep, I think it is a good idea

@TheXenocide
Copy link

can you retarget this for net9?

Yep, I think it is a good idea

Is there no chance we can get this fix in a patch release for .NET 8? Preferably sooner than later? I'm wrestling with this issue at the moment and would really appreciate a fix but we only use LTS versions for production apps.

@TheXenocide
Copy link

I also can't use this code directly in our app because too many pieces are internal for me to drop this in as a replacement renderer.

@nickl-martin
Copy link

Will this be fixed sometime soon? We can't change the color of our navigation bar to anything but white without having unsightly gaps on either end.

@Felix-Dev
Copy link

@kubaflo Any update on this PR? What is blocking progress here?

@kubaflo
Copy link
Contributor Author

kubaflo commented Oct 11, 2024

Hi, @Felix-Dev it is not up to me what will happen with this pr

@rmarinho rmarinho changed the base branch from net9.0 to main October 25, 2024 21:05
@rmarinho
Copy link
Member

/rebase

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-controls-shell Shell Navigation, Routes, Tabs, Flyout area-controls-titleview TitleView community ✨ Community Contribution
Projects
None yet
9 participants