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

Fix issue with tint not applying to loaded images #2077

Open
wants to merge 11 commits into
base: main
Choose a base branch
from

Conversation

myix765
Copy link
Contributor

@myix765 myix765 commented Jul 31, 2024

Description of Change

On Windows, if an image was already loaded (ie. after loading the page then navigating away and back to the page) the tint color is no longer applied because the ImageOpened event is not triggered. Adding a check for if the image is already loaded fixes this, but then causes an issue with the ToggleImageSource button. This was fixed by adding another check to see if the WImage source and the View source was the same, if not that means the toggle hasn't happened yet so we wait for the ImageOpened event to apply the tint.

This change also fixes the issue with the change tint color button not working in the sample app.

Linked Issues

PR Checklist

  • Has a linked Issue, and the Issue has been approved(bug) or Championed (feature/proposal)
  • Has tests (if omitted, state reason in description)
  • Has samples (if omitted, state reason in description)
  • Rebased on top of main at time of PR
  • Changes adhere to coding standard
  • Documentation created or updated: https://github.com/MicrosoftDocs/CommunityToolkit/pulls

Additional information

OS: Windows 10

Testing the behavior is fixed: go to the IconTintColorBehavior page, click on a different page in the FlyoutMenu, then click back on Behaviors (which should still show the IconTintColorBehavior). Before these changes, when navigating back some tint colors weren't applied.

@myix765 myix765 changed the title Tint navigate bug Fix issue with tint not applying to loaded images Jul 31, 2024
@dotnet-policy-service dotnet-policy-service bot added stale The author has not responded in over 30 days help wanted This proposal has been approved and is ready to be implemented labels Sep 1, 2024
@akhilesh-hexagon
Copy link

Hi @brminnick
We are working on maui winui and icons are having issue while navigate back.
Do you have any timeline to merge this PR?
We are waiting for this PR.

Copy link
Collaborator

@brminnick brminnick left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @myix765! Could you please update the IconTintColorBehaviorPage in the sample app to provide an example that hits every new branch in void LoadAndApplyImageTintColor(IView, WImage, Color)?

In my review, I've left a comment on the two branches in void LoadAndApplyImageTintColor(IView, WImage, Color) that I am unable to reach using the sample app.

We are working on maui winui and icons are having issue while navigate back.
Do you have any timeline to merge this PR?
We are waiting for this PR.

@akhilesh-hexagon If you're also able to help provide a sample that covers these two new branches in void LoadAndApplyImageTintColor(IView, WImage, Color), I can merge this PR as soon as you provide it!

&& image.Source is BitmapImage bitmapImage
&& Uri.Compare(new Uri($"{bitmapImage.UriSource.Scheme}:///{fileImageSource.File}"), bitmapImage.UriSource, UriComponents.Path, UriFormat.Unescaped, StringComparison.OrdinalIgnoreCase) is not 0)
{
image.ImageOpened += OnImageOpened;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please update the IconTintColorBehaviorPage in the sample app to include a use-case that hits this specific line of code where this specific if statement resolves to true?

if (element is IImageElement { Source: FileImageSource fileImageSource }
 				&& image.Source is BitmapImage bitmapImage
 				&& Uri.Compare(new Uri($"{bitmapImage.UriSource.Scheme}:///{fileImageSource.File}"), bitmapImage.UriSource, UriComponents.Path, UriFormat.Unescaped, StringComparison.OrdinalIgnoreCase) is not 0)

I cannot reach this branch using the Sample App and am unable to test it and verify it works.

}
else
{
ApplyTintColor();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please update the IconTintColorBehaviorPage in the sample app to include a use-case that hits this specific line of code where this specific else block resolves?

else
{
    ApplyTimeColor();
}

I cannot reach this branch using the Sample App and am unable to test it and verify it works.

@brminnick brminnick added waiting for feedback Waiting for a response from the author or the core team member and removed stale The author has not responded in over 30 days labels Sep 23, 2024
@dotnet-policy-service dotnet-policy-service bot added the stale The author has not responded in over 30 days label Oct 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted This proposal has been approved and is ready to be implemented stale The author has not responded in over 30 days waiting for feedback Waiting for a response from the author or the core team member
Projects
None yet
4 participants