-
Notifications
You must be signed in to change notification settings - Fork 1.9k
[iOS] Fix for Shell custom FlyoutIcon display problem #26016
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
Changes from all commits
671108a
9ac848b
c28691b
b79b7f2
1b7fc76
6d22c0d
615caef
48e3a15
5986d99
b011b91
bed5a23
abec60b
c5d4607
281213c
552e724
ca4d4bb
55f62bd
ec81088
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -273,4 +273,4 @@ Always put that at the top, without the block quotes. Without it, the users will | |
|
|
||
| --- | ||
|
|
||
| **Note for Future Updates:** This document should be expanded as new development patterns, tools, or workflows are discovered. Add sections for specific scenarios, debugging techniques, or tooling as they become relevant to the development process. | ||
| **Note for Future Updates:** This document should be expanded as new development patterns, tools, or workflows are discovered. Add sections for specific scenarios, debugging techniques, or tooling as they become relevant to the development process. | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why is it changing this file? |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,17 @@ | ||
| <?xml version="1.0" encoding="utf-8" ?> | ||
| <Shell | ||
| xmlns="http://schemas.microsoft.com/dotnet/2021/maui" | ||
| xmlns:x="http://schemas.microsoft.com/winfx/2009/xaml" | ||
| Shell.FlyoutBehavior="Flyout" | ||
| FlyoutIcon="groceries.png" | ||
| Shell.ForegroundColor="red" | ||
| x:Class="Maui.Controls.Sample.Issues.Issue25920"> | ||
| <ShellContent Title="Home"> | ||
| <ContentPage> | ||
| <StackLayout VerticalOptions="Center" HorizontalOptions="Center"> | ||
| <Label AutomationId="Label" Text="This is Label"/> | ||
| </StackLayout> | ||
| </ContentPage> | ||
| </ShellContent> | ||
|
|
||
| </Shell> |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,14 @@ | ||
| using System; | ||
| using Microsoft.Maui.Controls.Internals; | ||
|
|
||
| namespace Maui.Controls.Sample.Issues | ||
| { | ||
| [Issue(IssueTracker.Github, 25920, ".NET MAUI set AppShell custom FlyoutIcon display problem", PlatformAffected.iOS | PlatformAffected.Android)] | ||
| public partial class Issue25920: Shell | ||
| { | ||
| public Issue25920() | ||
| { | ||
| InitializeComponent(); | ||
| } | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,20 @@ | ||
| namespace Maui.Controls.Sample.Issues; | ||
|
|
||
| [Issue(IssueTracker.Github, "29092Flyout", "Flyout - Auto Resize chrome icons on iOS to make it more consistent with other platforms - hamburger icon", PlatformAffected.iOS)] | ||
| public partial class Issue29092Flyout : FlyoutPage | ||
| { | ||
| public Issue29092Flyout() | ||
| { | ||
| Flyout = new ContentPage | ||
| { | ||
| IconImageSource = "groceries.png", | ||
| Title = "Flyout" | ||
| }; | ||
|
|
||
| Detail = new NavigationPage(new ContentPage | ||
| { | ||
| Title = "Title", | ||
| Content = new Label() { Text = "Hello, World!", AutomationId = "HelloWorldLabel", }, | ||
| }); | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,28 @@ | ||
| #if TEST_FAILS_ON_WINDOWS && TEST_FAILS_ON_ANDROID | ||
| // https://github.com/dotnet/maui/issues/26148 | ||
| // In Windows, the foreground color is not applied to the custom icon, so as of now, the test is not applicable for Windows. | ||
| // https://github.com/dotnet/maui/pull/27502 | ||
| // For Android, we have separate PR to fix the issue, so the test is not applicable for Android. | ||
| using NUnit.Framework; | ||
| using UITest.Appium; | ||
| using UITest.Core; | ||
|
|
||
| namespace Microsoft.Maui.TestCases.Tests.Issues | ||
| { | ||
| public class Issue25920 : _IssuesUITest | ||
| { | ||
| public Issue25920(TestDevice device) : base(device) { } | ||
|
|
||
| public override string Issue => ".NET MAUI set AppShell custom FlyoutIcon display problem"; | ||
|
|
||
| [Test] | ||
| [Category(UITestCategories.Shell)] | ||
| public void ForegroundColorShouldbeSetandCustomIconAlignedProperly() | ||
| { | ||
| App.WaitForElement("Label"); | ||
|
|
||
| VerifyScreenshot(); | ||
| } | ||
| } | ||
| } | ||
| #endif |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,27 @@ | ||
| #if TEST_FAILS_ON_ANDROID && TEST_FAILS_ON_WINDOWS | ||
| //IconImageSource is not properly updated in Android and Windows, so the test is not applicable for these platforms. | ||
| //https://github.com/dotnet/maui/issues/15211 | ||
| //https://github.com/dotnet/maui/issues/15211#issuecomment-1557569889 | ||
| using NUnit.Framework; | ||
| using UITest.Appium; | ||
| using UITest.Core; | ||
|
|
||
| namespace Microsoft.Maui.TestCases.Tests.Issues; | ||
|
|
||
| public class Issue29092Flyout : _IssuesUITest | ||
| { | ||
| public Issue29092Flyout(TestDevice testDevice) : base(testDevice) | ||
| { | ||
| } | ||
|
|
||
| public override string Issue => "Flyout - Auto Resize chrome icons on iOS to make it more consistent with other platforms - hamburger icon"; | ||
|
|
||
| [Test] | ||
| [Category(UITestCategories.FlyoutPage)] | ||
| public void FlyoutIconShouldAutoscale() | ||
| { | ||
| App.WaitForElement("HelloWorldLabel"); | ||
| VerifyScreenshot(); | ||
| } | ||
| } | ||
| #endif |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,4 @@ | ||
| using System; | ||
| using CoreGraphics; | ||
| using UIKit; | ||
|
|
||
|
|
@@ -25,6 +26,26 @@ public static UIImage ScaleImage(this UIImage target, float maxWidth, float maxH | |
| return ScaleImage(target, new CGSize(targetWidth, targetHeight), disposeOriginal); | ||
| } | ||
|
|
||
| internal static UIImage ResizeImageSource(this UIImage sourceImage, nfloat maxWidth, nfloat maxHeight, CGSize originalImageSize, bool shouldScaleUp = false) | ||
| { | ||
| if (sourceImage?.CGImage is null) | ||
| { | ||
| return null; | ||
| } | ||
|
|
||
| maxWidth = (nfloat)Math.Min(maxWidth, originalImageSize.Width); | ||
| maxHeight = (nfloat)Math.Min(maxHeight, originalImageSize.Height); | ||
|
|
||
| var sourceSize = sourceImage.Size; | ||
|
|
||
| float maxResizeFactor = (float)Math.Min(maxWidth / sourceSize.Width, maxHeight / sourceSize.Height); | ||
|
|
||
| if (maxResizeFactor > 1 && !shouldScaleUp) | ||
| return sourceImage; | ||
|
|
||
| return UIImage.FromImage(sourceImage.CGImage, sourceImage.CurrentScale / maxResizeFactor, sourceImage.Orientation); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
This approach uses the image context to resize the image, should be more efficient than creating a new instance.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @jsuarezruiz, previously, I used a similar drawing code without scaling in this PR. As per @mattleibow Do I need to modify the existing code as you suggested? please check the previous discussion and share the details:
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yeah for now we can just keep this as is, but maybe create a issue to in another PR try refactor the code to what @jsuarezruiz says |
||
| } | ||
|
|
||
| public static UIImage ScaleImage(this UIImage target, CGSize size, bool disposeOriginal = false) | ||
| { | ||
| UIGraphics.BeginImageContext(size); | ||
|
|
||
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.
why is it changing this file?
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.
While rebasing this branch, it appears that this file was changed, but I didn’t make any modifications to it.
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.
its adding spaces instead of tabs