Skip to content
This repository has been archived by the owner on May 1, 2024. It is now read-only.

UWP IconImageSource from netstandard 2.0 #6534

Closed
jonkas2211 opened this issue Jun 14, 2019 · 15 comments
Closed

UWP IconImageSource from netstandard 2.0 #6534

jonkas2211 opened this issue Jun 14, 2019 · 15 comments
Labels
e/3 🕒 3 help wanted We welcome community contributions to any issue, but these might be a good place to start! inactive Issue is older than 6 months and needs to be retested p/UWP t/bug 🐛 up-for-grabs We welcome community contributions to any issue, but these might be a good place to start!

Comments

@jonkas2211
Copy link
Contributor

Description

If you want to add your images in to the netstandard project and trie to add them in as a IconImageSource for a ToolbarItem then it does not appear on UWP.
On Android it will appear as shown in the screenshot.
If you add the image as an Image view inside your layout it will just work fine, so the ImageSource is not the problem.

Steps to Reproduce

  1. Add a image to the shared netstandard project.
  2. Create an ImageSource of that image e.g. var imageSource = ImageSource.FromResource("UWPToolbarItemsImageBug.Images.test.png", typeof(App).Assembly);
  3. Create a ToolbarItem and set the IconImageSource property
  4. Add the ToolbarItem to the ToolbarItems

Expected Behavior

Image should be shown in the toolbar.

Actual Behavior

Image does not appear in the toolbar on UWP

Basic Information

  • Version with issue: lates (4.1.0.496342-pre2)
  • Last known good version: -
  • IDE: VS2019 & VS2017
  • Platform Target Frameworks:
    • iOS: not tested
    • Android: 9.0
    • UWP: 16299, 17763
  • Android Support Library Version: 28.0.0.1
  • Nuget Packages: -
  • Affected Devices: My Computer

Screenshots

grafik

Reproduction Link

UWPToolbarItemsImageBug.zip

@jonkas2211 jonkas2211 added s/unverified New report that has yet to be verified t/bug 🐛 labels Jun 14, 2019
@jfversluis jfversluis added p/UWP and removed s/unverified New report that has yet to be verified labels Jun 14, 2019
@jonkas2211
Copy link
Contributor Author

Hallo @jfversluis,
can you give me a crudely estimation, when this will be fixed?

At the moment the alternative is, to copy all the images to the UWP project, which will produce redundancy of image files.

@jonkas2211
Copy link
Contributor Author

After investigation, I found out the error can be caused, because the Xamarin.Forms.Platform.UWP.StreamImageSourceHandler is not an IIconElementHandler.

@jfversluis
Copy link
Member

Hi @jonkas2211 unfortunately I do not have any estimation for you, sorry about that.

Since you have been investigating we are of course more than happy to see a pull request that fixes this issue. Have you actually verified that this is the problem?

@jonkas2211
Copy link
Contributor Author

Hello @jfversluis,

The Method Xamarin.Forms.Platform.UWP.ImageExtensions.ToWindowsIconElementAsync(..)

public static async Task<IconElement> ToWindowsIconElementAsync(this ImageSource source, CancellationToken cancellationToken = default(CancellationToken))
{
	if (source == null || source.IsEmpty)
		return null;

	var handler = Registrar.Registered.GetHandlerForObject<IIconElementHandler>(source);
	if (handler == null)
		return null;

	try
	{
		return await handler.LoadIconElementAsync(source, cancellationToken);
	}
	catch (OperationCanceledException)
	{
		// no-op
	}

	return null;
}

Is looking for the handler to implement IIconElementHandler. The StreamImageSourceHandler does not implement this, but the FileImageSourceHandler for example does.

I would love to create a pull request, but unfortunately I am no UWP expert and I dont know how to conver a Stream to a IconElement. I surely could save the Stream as a kind of image to the disk and relead it from an uri, but this is not how it supposed be?

@jonkas2211
Copy link
Contributor Author

It might be a limitation of UWP

@jfversluis
Copy link
Member

From what I can see this is simply not implemented in UWP indeed. The original implementation is from @mattleibow maybe he can shine his light on this?

I have been looking into it a little bit and the only possibility I see right now is to save the stream to a (temp) location and then load the images from there, but that doesn’t seem ideal as well. So then yes, it is a limitation on UWP.

@mattleibow
Copy link
Contributor

Just spitballing here, but we might be able to do something with a bitmap that is a brush and use the brush. I see this was also added in a later version of UWP: https://docs.microsoft.com/en-us/uwp/api/windows.ui.xaml.controls.iconsourceelement

We might be able to do some things if we get a little bit creative.

@jonkas2211
Copy link
Contributor Author

That seems to be a plan, but how would we handle targeting Windows 10, version 1809 (introduced v10.0.17763.0)?

@mattleibow
Copy link
Contributor

It might be possible to use multitargeting to generate separate dlls so we can still support a desired target framework. But, the thing to do would be to bump the target version to the highest, but keep the minimum at the lowest. Then, use the ApiInformation type to see if the type or members are available (https://docs.microsoft.com/en-us/uwp/api/Windows.Foundation.Metadata.ApiInformation). This way the app can light up on newer platforms, and not crash on older ones.

@jonkas2211
Copy link
Contributor Author

For reference look at this Issue on the Microsoft microsoft-ui-xaml repository: microsoft/microsoft-ui-xaml#601

@jonkas2211
Copy link
Contributor Author

So I tried to create a brush an set the Foreground from the IconElement, but it seems to end in a silent exception:

public async Task<IconElement> LoadIconElementAsync(ImageSource imagesource, CancellationToken cancellationToken = new CancellationToken())
{
    IconElement image = null;
    if (imagesource is StreamImageSource streamSource)
    {
        image = new BitmapIcon();
        var brush = new ImageBrush();
        using (Stream stream = await((IStreamImageSource)streamSource).GetStreamAsync(cancellationToken))
        {
            if (stream == null)
                return null;

            var source = new BitmapImage();
            await source.SetSourceAsync(stream.AsRandomAccessStream());
            brush.ImageSource = source;
            image.SetForeground(brush); // throws silent Argument Exception seen in output
        }
    }
    return image;
}

@mattleibow
Copy link
Contributor

If you wrap the code in a try catch, is it possible to see what it is? Maybe the async is causing the silence.

@jonkas2211
Copy link
Contributor Author

Thanks @mattleibow if things could always be easy as that.
So the exception is saying, that the ImageBrush is not a Foreground Type: type is not a Foregroundable type

@samhouts samhouts added the e/3 🕒 3 label Jul 30, 2019
@samhouts samhouts added inactive Issue is older than 6 months and needs to be retested help wanted We welcome community contributions to any issue, but these might be a good place to start! up-for-grabs We welcome community contributions to any issue, but these might be a good place to start! labels Jan 18, 2020
@jonkas2211
Copy link
Contributor Author

Hallo @samhouts,
this issue seems to be fixed with pull request #8147.

@jfversluis
Copy link
Member

Thanks for letting us know! Closing this one then

@jfversluis jfversluis mentioned this issue Apr 3, 2020
2 tasks
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
e/3 🕒 3 help wanted We welcome community contributions to any issue, but these might be a good place to start! inactive Issue is older than 6 months and needs to be retested p/UWP t/bug 🐛 up-for-grabs We welcome community contributions to any issue, but these might be a good place to start!
Projects
None yet
Development

No branches or pull requests

4 participants