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

[PM-7134] Replace FFImageLoading with Net MAUI Image Control to avoid background issues #3123

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion src/App/App.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,6 @@
<PackageReference Include="Plugin.Fingerprint" Version="3.0.0-beta.1" />
<PackageReference Include="SkiaSharp.Views.Maui.Controls" Version="2.88.4-preview.84" />
<PackageReference Include="SkiaSharp.Views.Maui.Controls.Compatibility" Version="2.88.4-preview.84" />
<PackageReference Include="FFImageLoadingCompat.Maui" Version="0.1.1" />
<PackageReference Include="AsyncAwaitBestPractices.MVVM" Version="6.0.6" />
<PackageReference Include="CommunityToolkit.Mvvm" Version="8.2.1" />
<PackageReference Include="PCLCrypto" Version="2.1.40-alpha" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,17 +18,16 @@
<u:InverseBoolConverter x:Key="inverseBool" />
</controls:BaseCipherViewCell.Resources>

<controls:CachedImage
<Image
x:Name="_iconImage"
Grid.Column="0"
Grid.RowSpan="2"
BitmapOptimizations="True"
IsOpaque="True"
HorizontalOptions="Center"
VerticalOptions="Center"
WidthRequest="22"
HeightRequest="22"
Success="Icon_Success"
Error="Icon_Error"
Loaded="Image_OnLoaded"
AutomationProperties.IsInAccessibleTree="False" />

<controls:IconLabel
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
namespace Bit.App.Controls
using Bit.Core.Services;

namespace Bit.App.Controls
{
public partial class AuthenticatorViewCell : BaseCipherViewCell
{
Expand All @@ -7,8 +9,36 @@ public AuthenticatorViewCell()
InitializeComponent();
}

protected override CachedImage Icon => _iconImage;
protected override Image Icon => _iconImage;

protected override IconLabel IconPlaceholder => _iconPlaceholderImage;

private async void Image_OnLoaded(object sender, EventArgs e)
{
if (Handler?.MauiContext == null) { return; }
if (_iconImage?.Source == null) { return; }

try
{
var result = await _iconImage.Source.GetPlatformImageAsync(Handler.MauiContext);
if (result == null)
{
Icon_Error(sender, e);
}
else
{
Icon_Success(sender, e);
}
}
catch (InvalidOperationException) //Can occur with incorrect/malformed uris
{
Icon_Error(sender, e);
}
catch (Exception ex)
{
LoggerHelper.LogEvenIfCantBeResolved(ex);
Icon_Error(sender, e);
}
}
}
}
42 changes: 0 additions & 42 deletions src/Core/Controls/CachedImage.cs

This file was deleted.

42 changes: 4 additions & 38 deletions src/Core/Controls/CipherViewCell/BaseCipherViewCell.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ namespace Bit.App.Controls
{
public abstract class BaseCipherViewCell : ExtendedGrid
{
protected virtual CachedImage Icon { get; }
protected virtual Image Icon { get; }

protected virtual IconLabel IconPlaceholder { get; }

Expand All @@ -15,7 +15,7 @@ public abstract class BaseCipherViewCell : ExtendedGrid
// the app would crash because there can't be any lock files by the app when it gets suspended.
// So, the approach has changed to reuse the IconLabel default icon to use it for these placeholders
// as well. In order to do that both icon controls change their visibility dynamically here reacting to
// CachedImage events and binding context changes.
// Image OnLoaded event and binding context changes.

protected override void OnBindingContextChanged()
{
Expand Down Expand Up @@ -47,8 +47,7 @@ private void UpdateIconImages(bool showIcon)
});
}

#if !UT
public void Icon_Success(object sender, FFImageLoading.Maui.CachedImageEvents.SuccessEventArgs e)
public void Icon_Success(object sender, EventArgs e)
{
if (BindingContext is CipherItemViewModel cipherItemVM)
{
Expand All @@ -62,7 +61,7 @@ public void Icon_Success(object sender, FFImageLoading.Maui.CachedImageEvents.Su
}
}

public void Icon_Error(object sender, FFImageLoading.Maui.CachedImageEvents.ErrorEventArgs e)
public void Icon_Error(object sender, EventArgs e)
{
if (BindingContext is CipherItemViewModel cipherItemVM)
{
Expand All @@ -74,38 +73,5 @@ public void Icon_Error(object sender, FFImageLoading.Maui.CachedImageEvents.Erro
IconPlaceholder.IsVisible = true;
});
}
#else
private void Icon_Success(object sender, EventArgs e) {}
private void Icon_Error(object sender, EventArgs e) {}
#endif
}

public class StubBaseCipherViewCellSoLinkerDoesntRemoveMethods : BaseCipherViewCell
{
protected override CachedImage Icon => new CachedImage();
protected override IconLabel IconPlaceholder => new IconLabel();

public static void CallThisSoLinkerDoesntRemoveMethods()
{
#if !UT
var stub = new StubBaseCipherViewCellSoLinkerDoesntRemoveMethods();

try
{
stub.Icon_Success(stub, new FFImageLoading.Maui.CachedImageEvents.SuccessEventArgs(new FFImageLoading.Work.ImageInformation(), FFImageLoading.Work.LoadingResult.Disk));
}
catch (Exception)
{
}

try
{
stub.Icon_Error(stub, new FFImageLoading.Maui.CachedImageEvents.ErrorEventArgs(new InvalidOperationException("stub")));
}
catch (Exception)
{
}
#endif
}
}
}
7 changes: 3 additions & 4 deletions src/Core/Controls/CipherViewCell/CipherViewCell.xaml
Original file line number Diff line number Diff line change
Expand Up @@ -29,18 +29,17 @@
<ColumnDefinition Width="60" />
</Grid.ColumnDefinitions>

<controls:CachedImage
<Image
x:Name="_iconImage"
Grid.Column="0"
BitmapOptimizations="True"
IsOpaque="True"
HorizontalOptions="CenterAndExpand"
VerticalOptions="CenterAndExpand"
Margin="9"
WidthRequest="22"
HeightRequest="22"
Aspect="AspectFit"
Success="Icon_Success"
Error="Icon_Error"
Loaded="Image_OnLoaded"
AutomationProperties.IsInAccessibleTree="False"
AutomationId="CipherWebsiteIcon" />

Expand Down
31 changes: 30 additions & 1 deletion src/Core/Controls/CipherViewCell/CipherViewCell.xaml.cs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
using System.Windows.Input;
using Bit.App.Abstractions;
using Bit.App.Pages;
using Bit.Core.Services;
using Bit.Core.Utilities;

namespace Bit.App.Controls
Expand All @@ -23,7 +24,7 @@ public CipherViewCell()
_iconImage.HeightRequest = ICON_IMAGE_DEFAULT_WIDTH * fontScale;
}

protected override CachedImage Icon => _iconImage;
protected override Image Icon => _iconImage;

protected override IconLabel IconPlaceholder => _iconPlaceholderImage;

Expand All @@ -40,5 +41,33 @@ private void MoreButton_Clicked(object sender, EventArgs e)
ButtonCommand?.Execute(cipherItem.Cipher);
}
}

private async void Image_OnLoaded(object sender, EventArgs e)
{
if (Handler?.MauiContext == null) { return; }
if (_iconImage?.Source == null) { return; }

try
{
var result = await _iconImage.Source.GetPlatformImageAsync(Handler.MauiContext);
if (result == null)
{
Icon_Error(sender, e);
}
else
{
Icon_Success(sender, e);
}
}
catch (InvalidOperationException) //Can occur with incorrect/malformed uris
{
Icon_Error(sender, e);
}
catch (Exception ex)
{
LoggerHelper.LogEvenIfCantBeResolved(ex);
Icon_Error(sender, e);
}
}
}
}
1 change: 0 additions & 1 deletion src/Core/Core.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@
<PackageReference Include="CommunityToolkit.Mvvm" Version="8.2.1" />
<PackageReference Include="Portable.BouncyCastle" Version="1.9.0" />
<!-- HACK: When running Unit Tests we cannot load FFImageLoading because it doesn't support "raw" net8.0 -->
<PackageReference Condition="!$(CustomConstants.Contains(UT))" Include="FFImageLoadingCompat.Maui" Version="0.1.1" />
</ItemGroup>
<ItemGroup Condition="$([MSBuild]::GetTargetPlatformIdentifier('$(TargetFramework)')) == 'android'">
<PackageReference Include="Xamarin.AndroidX.AutoFill" Version="1.1.0.18" />
Expand Down
16 changes: 1 addition & 15 deletions src/Core/MauiProgram.cs
Original file line number Diff line number Diff line change
@@ -1,12 +1,8 @@
using Bit.App.Controls;

using Camera.MAUI;
using CommunityToolkit.Maui;
#if !UT
using FFImageLoading.Maui;
#endif
using Microsoft.Extensions.Logging;
using Microsoft.Maui.Controls.Compatibility.Hosting;
using Microsoft.Maui.Handlers;
using SkiaSharp.Views.Maui.Controls.Hosting;
using AppEffects = Bit.App.Effects;

Expand All @@ -26,9 +22,6 @@ public static MauiAppBuilder ConfigureMauiAppBuilder(Action<IEffectsBuilder> cus
.UseMauiCompatibility()
.UseMauiCameraView()
.UseSkiaSharp()
#if !UT
.UseFFImageLoading()
#endif
.ConfigureEffects(effects =>
{
#if ANDROID
Expand Down Expand Up @@ -63,13 +56,6 @@ public static MauiAppBuilder ConfigureMauiAppBuilder(Action<IEffectsBuilder> cus
builder.Logging.AddDebug();
#endif

ExplicitlyPreventThingsGetRemovedBecauseOfLinker();

return builder;
}

private static void ExplicitlyPreventThingsGetRemovedBecauseOfLinker()
{
StubBaseCipherViewCellSoLinkerDoesntRemoveMethods.CallThisSoLinkerDoesntRemoveMethods();
}
}
17 changes: 13 additions & 4 deletions src/Core/Pages/Vault/CipherItemViewModel.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ namespace Bit.App.Pages
public class CipherItemViewModel : ExtendedViewModel, IGroupingsPageListItem
{
private readonly bool _websiteIconsEnabled;
private string _iconImageSource = string.Empty;
private UriImageSource _iconImageSource = null;

public CipherItemViewModel(CipherView cipherView, bool websiteIconsEnabled, bool fuzzyAutofill = false)
{
Expand All @@ -27,14 +27,23 @@ public bool ShowIconImage
&& IconImageSource != null;
}

public string IconImageSource
public UriImageSource IconImageSource
{
get
{
if (_iconImageSource == string.Empty) // default value since icon source can return null
if (_iconImageSource == null) // default value since icon source can return null
{
_iconImageSource = IconImageHelper.GetIconImage(Cipher);
var iconImageStr = IconImageHelper.GetIconImage(Cipher);
if (string.IsNullOrWhiteSpace(iconImageStr)) { return null; }

_iconImageSource = new UriImageSource
{
Uri = new Uri(iconImageStr),
CacheValidity = TimeSpan.FromDays(90),
CachingEnabled = true
};
}

return _iconImageSource;
}
}
Expand Down