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

Conversation

mike-rowley
Copy link
Contributor

This is the PR for Bug #934 IconTintColorEffect does not work when the image is in Shared project as EmbeddeResource
#934

I have implemented the fix @TheDude1604 suggested in the bug report and it resolves the issue on IOS. I have tested it on a physical device as well as a simulator.

If there are any issues please let me know and I will try to resolve them. Thanks for considering this PR.

@net-foundation-cla
Copy link

net-foundation-cla bot commented Mar 25, 2022

CLA assistant check
All CLA requirements met.

@ghost
Copy link

ghost commented Mar 25, 2022

CLA assistant check
Thank you for your submission, we really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.

❌ mike-rowley sign now
You have signed the CLA already but the status is still pending? Let us recheck it.

@pictos
Copy link
Contributor

pictos commented Mar 25, 2022

@mike-rowley can you assign the CLA?

@mike-rowley
Copy link
Contributor Author

I did both CLAs, now when I click the "sign now" link it says "You have signed the CLA for multiple repositories or organizations" and all options are disabled with my name etc in the fields. Seems like some sort of glitch in the CLA process unless there is something I am missing.

@pictos
Copy link
Contributor

pictos commented Mar 25, 2022

@brminnick do you have the permission to check if everything it's fine?

@TheCodeTraveler
Copy link
Contributor

Done ✅

Copy link
Contributor

@pictos pictos left a comment

Choose a reason for hiding this comment

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

@mike-rowley thanks for sharing the solution with us❣️
So, this retry logic doesn't look very solid to me, because there's a chance to the image loads after all retries attempts, would be good if we can use something more solid. So my suggestion is to use the IsLoading property inside the XF.Image control, here's a snippet of code that I tried on my end and worked. I put this code inside the void SetUIImageViewTintColor(UIImageView imageView, Color color) method.

if (imageView.Image == null)
{
    Element.PropertyChanged += (_, e) =>
    {
        if (e.PropertyName == Image.IsLoadingProperty.PropertyName)
        {
            var b = Element as Xamarin.Forms.Image ?? throw new NullReferenceException();
            if (!b.IsLoading)
            {
                SetUIImageViewTintColor(imageView, color);
            }
        }
    };
}

I'm not sure if we have something like that for the ImageButton control, can you take a look on that? If you need any help, just let me know (:

@mike-rowley
Copy link
Contributor Author

Thanks @pictos I took your code and tested it on my side and it works great, a much more elegant solution! I also converted that code to the Button and it works there as well so I pushed another PR with the updated code.

@pictos
Copy link
Contributor

pictos commented Mar 26, 2022

@mike-rowley glad that you liked the solution! I have just a feel comments and after that we should be good to merge it

mike-rowley and others added 3 commits March 26, 2022 15:48
…Color/IconTintColorEffectRouter.ios.cs

Co-authored-by: Pedro Jesus <pedrojesus.cefet@gmail.com>
…Color/IconTintColorEffectRouter.ios.cs

Co-authored-by: Pedro Jesus <pedrojesus.cefet@gmail.com>
@mike-rowley
Copy link
Contributor Author

@pictos Good call on unsubscribing, I think this version should do it.

Somehow I accidentally referenced a bug from 2017 in the summary when I pushed it and I am not sure how to change that "#2" in the summary.

Copy link
Contributor

@pictos pictos left a comment

Choose a reason for hiding this comment

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

@mike-rowley thanks for this!

@pictos pictos merged commit 6b6a02e into xamarin:main Mar 27, 2022
bijington pushed a commit that referenced this pull request May 18, 2022
…to PR #1836 (#1857)

* Fix for Bug #934 (IconTintColorEffect does not work when the image is in Shared project as EmbeddeResource)

* Revised Fix for #934

* Update src/CommunityToolkit/Xamarin.CommunityToolkit/Effects/IconTintColor/IconTintColorEffectRouter.ios.cs

Co-authored-by: Pedro Jesus <pedrojesus.cefet@gmail.com>

* Update src/CommunityToolkit/Xamarin.CommunityToolkit/Effects/IconTintColor/IconTintColorEffectRouter.ios.cs

Co-authored-by: Pedro Jesus <pedrojesus.cefet@gmail.com>

* Revised Fix #2 for Bug #934

* updated the sample

* improved variable name

* changed the null check object

* Update IconTintColorEffectRouter.ios.cs To Fix ClearTintColor Crash

I have added a null check on button.ImageView in ClearTintColor to prevent a crash when OnDetach is called.  This NullReferenceException occured when changing Application.MainPage.

* Update IconTintColorEffectRouter.ios.cs To Fix ClearTintColor Crash

Co-authored-by: Pedro Jesus <pedrojesus.cefet@gmail.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants