dotnet / maui

.NET MAUI is the .NET Multi-platform App UI, a framework for building native device applications spanning mobile, tablet, and desktop.
https://dot.net/maui
MIT License
21.63k stars 1.62k forks source link

[Android] Flyout Page support for IconImageSource #22099

Open kubaflo opened 2 weeks ago

kubaflo commented 2 weeks ago

Description of Change

When the IconImageSource property was set on the Flyout page in the mobile application, there was an inconsistency between the Android and iOS platforms. While iOS correctly displays the specified icon, Android shows a default hamburger icon instead.

Fixes https://github.com/dotnet/maui/issues/15211

<FlyoutPage
    xmlns="http://schemas.microsoft.com/dotnet/2021/maui"
    xmlns:x="http://schemas.microsoft.com/winfx/2009/xaml"
    x:Class="Maui.Controls.Sample.MainPage"
    x:Name="Self"
    xmlns:local="clr-namespace:Maui.Controls.Sample">
    <FlyoutPage.Flyout>
        <ContentPage Title="Flyout" IconImageSource="groceries.png"/>
    </FlyoutPage.Flyout>
    <FlyoutPage.Detail>
        <NavigationPage>
            <x:Arguments>
                <local:DetailPage />
            </x:Arguments>
        </NavigationPage>
    </FlyoutPage.Detail>
</FlyoutPage>
Before After
dotnet-policy-service[bot] commented 2 weeks ago

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.

jsuarezruiz commented 1 week ago

/azp run

azure-pipelines[bot] commented 1 week ago
Azure Pipelines successfully started running 3 pipeline(s).
rmarinho commented 1 week ago

/azp run

azure-pipelines[bot] commented 1 week ago
Azure Pipelines successfully started running 3 pipeline(s).
kubaflo commented 1 week ago

Thanks for the PR. This is a new API and will need a bit more checks before we can merge. I also am going to dump some questions that popped into my head for anyone to answer.

However, how did this work with iOS?

I am not sure if I recall correctly, but this was done before via the page icon. This new API makes more sense though.

I assume sine this works on iOS, we are using that flow to get the icon? Should we use the same here? Or, should we use this new thing and make iOS also use this?

What about Windows?

Hi @mattleibow,

It worked on iOS before, so I didn't change anything... Maybe I should make changes for iOS to match the logic with Android. The biggest problem is that iOS operates on the flyout page in Controls/compatibility while other platforms in core/handlers

iOS content of FlyoutViewHandler.iOS.cs

        public partial class FlyoutViewHandler : ViewHandler<IFlyoutView, UIKit.UIView>
    {
        protected override UIKit.UIView CreatePlatformView()
        {
            throw new System.NotImplementedException();
        }
    }

Maybe the iOS flyout implementation should be migrated to core/handlers as well?

I believe changes made to Android in this PR should be propagated to Windows as well, but I cannot do it because of lack of a Windows device 😅