-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
base: main
Are you sure you want to change the base?
Conversation
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. |
/azp run MAUI-UITests-public |
Azure Pipelines successfully started running 1 pipeline(s). |
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; |
There was a problem hiding this comment.
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; |
There was a problem hiding this comment.
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?
/azp run |
Azure Pipelines successfully started running 3 pipeline(s). |
There was a problem hiding this 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?
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. |
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. |
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. |
@kubaflo Any update on this PR? What is blocking progress here? |
Hi, @Felix-Dev it is not up to me what will happen with this pr |
/rebase |
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 bothHorizontalOptions
andVerticalOptions
set toFill
. 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
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.
Default
- (horizontal and vertical options set to fill implicitly)HorizontalOptions="Start"
HorizontalOptions="Center"
HorizontalOptions="End"
VerticalOptions="Start"
VerticalOptions="Center"
VerticalOptions="End"
VerticalOptions="Start"
HorizontalOptions="Start"
VerticalOptions="Center"
HorizontalOptions="Center"
VerticalOptions="End"
HorizontalOptions="End"
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