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

TabbedPage.ToolbarItems IconImageSource with Uri stop working after installing glidex.forms nuget package! #69

Closed
msangtarash opened this issue Jul 31, 2020 · 5 comments · Fixed by #77

Comments

@msangtarash
Copy link

I've followings:

<TabbedPage>
    <TabbedPage.ToolbarItems>
        <ToolbarItem IconImageSource="{app:UriImageSource Uri='https://www.google.com/images/branding/googlelogo/1x/googlelogo_color_272x92dp.png'}" />
        <ToolbarItem IconImageSource="google.png" />
    </TabbedPage.ToolbarItems>
    <NavigationPage Title="Tab1">
        <x:Arguments>
            <ContentPage>
                <StackLayout HorizontalOptions="Center" VerticalOptions="Center">
                    <Label Text="1" />
                    <Image
                        HeightRequest="150"
                        Source="{app:UriImageSource Uri='https://www.google.com/images/branding/googlelogo/1x/googlelogo_color_272x92dp.png'}"
                        WidthRequest="150" />
                    <Label Text="2" />
                    <Image
                        HeightRequest="150"
                        Source="google.png"
                        WidthRequest="150" />
                </StackLayout>
            </ContentPage>
        </x:Arguments>
    </NavigationPage>
</TabbedPage>

We've 2 toolbar items with icon and 2 image. 1 toolbar item has local image and another one has remote image. We've the same conditions with 2 images in content page.

After I install glidex.forms (and I don't have to even configure it in MainActivity.cs!), the toolbar item which has remote image source stops working! Another ToolbarItem & the two images in content page are fine!

Additional note:

I've tested this in another project which has glideX configured with custom IGlideHandler provided. The handler isn't being invoked for the toolbar item with remote source!

I know this sounds crazy, but I've attached a simple repo. Just install glideX.forms to make it broken! /-:

XFTabbedPageToolbarIconIssue.zip

@ysmoradi
Copy link
Contributor

ysmoradi commented Aug 2, 2020

Glide's ImageViewHandler is not implementing IImageSourceHandler.
It's implementing IImageViewHandler only!
That's an issue )-:

@ysmoradi
Copy link
Contributor

ysmoradi commented Aug 2, 2020

I was able to make it work by followings:

public async Task<Bitmap> LoadImageAsync(ImageSource source, Context context, CancellationToken cancelationToken = default)
{
    try
    {
        if (source is UriImageSource uriImageSource)
        {
            var url = uriImageSource.Uri.OriginalString;
            Forms.Debug("Loading `{0}` as a web URL", url);
            RequestManager request = With(context);
            var builder = request.AsBitmap().Load(url);
            var result = await builder.Submit().GetAsync();
            return (Bitmap)result;
        }
        // This code needs other image source types handling + another hook should gets added into IGlideHandler, so we can customize requests before submitting (for example by adding access token)
        return null;
    }
    catch (Exception exc)
    {
        //Since developers can't catch this themselves, I think we should log it and silently fail
        Forms.Warn("Unexpected exception in glidex: {0}", exc);
        return null;
    }
}

It seems GlideX.Forms is loading images & image buttons by Glidex and other images such as page icons and toolbar items and pages background images using xf default image handler which is not performant )-:

@jonathanpeppers @jamesmontemagno

@ysmoradi
Copy link
Contributor

ysmoradi commented Aug 3, 2020

Every image we use in pages backgrounds (which are typically large) and toolbar images etc is loading without Glide!
And if we use uri source images, it won't load at all )-:
Could you please let us know you've seen this issue by sending initial response to us?

@jonathanpeppers
Copy link
Owner

It doesn't seem like Xamarin.Forms ToolbarItem.IconImageSource calls into IImageViewHandler:

There appears to be no way for glidex to hook into creating these icons.

I think an issue needs to be filed for Xamarin.Forms, because the fix will be there?

A workaround for now, would be to create the tabs with regular views, here is a blog post about doing this: https://www.sharpnado.com/pure-xamarin-forms-tabs/

If you could use a regular <Image/>, I think everything would work?

@ysmoradi
Copy link
Contributor

ysmoradi commented Aug 4, 2020

I think xamarin forms is fine (at least for UriImageSource), it calls IImageHandler for image & image view and it calls IImageSourceHandler for icons & backgrounds (for UriImageSources at the moment)
So, we need to implement IImageSourceHandler.
Note that you're exporting your class using ExportImageSourceHandler, but you're implementing IImageViewHandler only!

Update => I've also submitted PR.

Update 2 => The only wrong thing with XF is why it's not calling our handler for Stream & File Images Sources? This needs another issue.

jonathanpeppers added a commit that referenced this issue Sep 25, 2020
Fixes: #69
Context: xamarin/Xamarin.Forms#11676

There are certain Android APIs where `IImageViewHandler` is not
possible for Xamarin.Forms to be able to call. For example, if you
have an `IMenuItem`:

    menuItem.SetIcon (drawable);

`IImageViewHandler` requires an `ImageView` -- so Xamarin.Forms must
fall back to `IImageSourceHandler` for this to work at *all*.

Adding `IImageSourceHandler` required various refactoring, but things
seem to work as before. `IImageViewHandler` appears to be called by
Xamarin.Forms when it *should* be, and `IImageSourceHandler` is called
as a fallback.

Perhaps in MAUI, `IImageViewHandler` should have an additional method
for `IMenuItem`?

Co-authored-by: Yaser Moradi <ysmoradi@outlook.com>
jonathanpeppers added a commit that referenced this issue Sep 25, 2020
Fixes: #69
Context: xamarin/Xamarin.Forms#11676

There are certain Android APIs where `IImageViewHandler` is not
possible for Xamarin.Forms to be able to call. For example, if you
have an `IMenuItem`:

    menuItem.SetIcon (drawable);

`IImageViewHandler` requires an `ImageView` -- so Xamarin.Forms must
fall back to `IImageSourceHandler` for this to work at *all*.

Adding `IImageSourceHandler` required various refactoring, but things
seem to work as before. `IImageViewHandler` appears to be called by
Xamarin.Forms when it *should* be, and `IImageSourceHandler` is called
as a fallback.

Perhaps in MAUI, `IImageViewHandler` should have an additional method
for `IMenuItem`?

Co-authored-by: Yaser Moradi <ysmoradi@outlook.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants