-
Notifications
You must be signed in to change notification settings - Fork 473
Fix issue with tint not applying to loaded images #2077
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
Changes from all commits
1ef6a7d
586f28d
5857a4e
25b934d
70dc76d
934880c
b25318f
45570cf
868faab
12fa1ae
43b326b
519c024
0807479
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
|
|
@@ -27,21 +27,8 @@ protected override void OnAttachedTo(View bindable, FrameworkElement platformVie | |||||||
|
|
||||||||
| ApplyTintColor(platformView, bindable, TintColor); | ||||||||
|
|
||||||||
| PropertyChanged += OnIconTintColorBehaviorPropertyChanged; | ||||||||
| bindable.PropertyChanged += OnElementPropertyChanged; | ||||||||
| this.PropertyChanged += (s, e) => | ||||||||
| { | ||||||||
| if (e.PropertyName == TintColorProperty.PropertyName) | ||||||||
| { | ||||||||
| if (currentColorBrush is not null && TintColor is not null) | ||||||||
| { | ||||||||
| currentColorBrush.Color = TintColor.ToWindowsColor(); | ||||||||
| } | ||||||||
| else | ||||||||
| { | ||||||||
| ApplyTintColor(platformView, bindable, TintColor); | ||||||||
| } | ||||||||
| } | ||||||||
| }; | ||||||||
| } | ||||||||
|
|
||||||||
| /// <inheritdoc/> | ||||||||
|
|
@@ -50,6 +37,8 @@ protected override void OnDetachedFrom(View bindable, FrameworkElement platformV | |||||||
| base.OnDetachedFrom(bindable, platformView); | ||||||||
|
|
||||||||
| bindable.PropertyChanged -= OnElementPropertyChanged; | ||||||||
| PropertyChanged -= OnIconTintColorBehaviorPropertyChanged; | ||||||||
|
|
||||||||
| RemoveTintColor(platformView); | ||||||||
| } | ||||||||
|
|
||||||||
|
|
@@ -83,10 +72,34 @@ static bool TryGetSourceImageUri(WImage? imageControl, IImageElement? imageEleme | |||||||
| return false; | ||||||||
| } | ||||||||
|
|
||||||||
| void OnIconTintColorBehaviorPropertyChanged(object? sender, PropertyChangedEventArgs e) | ||||||||
| { | ||||||||
| ArgumentNullException.ThrowIfNull(sender); | ||||||||
| var iconTintColorBehavior = (IconTintColorBehavior)sender; | ||||||||
|
|
||||||||
| if (iconTintColorBehavior.View is not IView bindable | ||||||||
| || bindable.Handler?.PlatformView is not FrameworkElement platformView) | ||||||||
| { | ||||||||
| return; | ||||||||
| } | ||||||||
|
|
||||||||
| if (e.PropertyName == TintColorProperty.PropertyName) | ||||||||
| { | ||||||||
| if (currentColorBrush is not null && TintColor is not null) | ||||||||
| { | ||||||||
| currentColorBrush.Color = TintColor.ToWindowsColor(); | ||||||||
| } | ||||||||
| else | ||||||||
| { | ||||||||
| ApplyTintColor(platformView, bindable, TintColor); | ||||||||
| } | ||||||||
| } | ||||||||
| } | ||||||||
|
|
||||||||
| void OnElementPropertyChanged(object? sender, PropertyChangedEventArgs e) | ||||||||
| { | ||||||||
| if (e.PropertyName is not string propertyName | ||||||||
| || sender is not View bindable | ||||||||
| || sender is not IView bindable | ||||||||
| || bindable.Handler?.PlatformView is not FrameworkElement platformView) | ||||||||
| { | ||||||||
| return; | ||||||||
|
|
@@ -99,7 +112,7 @@ void OnElementPropertyChanged(object? sender, PropertyChangedEventArgs e) | |||||||
| } | ||||||||
| } | ||||||||
|
|
||||||||
| void ApplyTintColor(FrameworkElement platformView, View element, Color? color) | ||||||||
| void ApplyTintColor(FrameworkElement platformView, IView element, Color? color) | ||||||||
| { | ||||||||
| RemoveTintColor(platformView); | ||||||||
|
|
||||||||
|
|
@@ -128,7 +141,7 @@ void ApplyTintColor(FrameworkElement platformView, View element, Color? color) | |||||||
| } | ||||||||
| } | ||||||||
|
|
||||||||
| void LoadAndApplyImageTintColor(View element, WImage image, Color color) | ||||||||
| void LoadAndApplyImageTintColor(IView element, WImage image, Color color) | ||||||||
| { | ||||||||
| if (element is IImageElement { Source: UriImageSource uriImageSource }) | ||||||||
| { | ||||||||
|
|
@@ -140,6 +153,21 @@ void LoadAndApplyImageTintColor(View element, WImage image, Color color) | |||||||
|
|
||||||||
| ApplyTintColor(); | ||||||||
| } | ||||||||
| else if (image.IsLoaded) | ||||||||
| { | ||||||||
| // Sometimes WImage source doesn't match View source so the image is not ready to be tinted | ||||||||
| // We must wait for next ImageOpened event | ||||||||
| 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) | ||||||||
| { | ||||||||
|
||||||||
| { | |
| { | |
| image.ImageOpened -= OnImageOpened; // Unsubscribe first to prevent duplicate subscriptions |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] The logic inside this handler duplicates the tint-application flow; consider extracting it into a shared helper method to reduce code duplication and improve readability.