xamarin / XamarinCommunityToolkit

The Xamarin Community Toolkit is a collection of Animations, Behaviors, Converters, and Effects for mobile development with Xamarin.Forms. It simplifies and demonstrates common developer tasks building iOS, Android, and UWP apps with Xamarin.Forms.
MIT License
1.58k stars 471 forks source link

[Bug] [Android] TouchEffect ripple is drawn, although CanExecute is false #1567

Closed Andreas0xffff closed 3 years ago

Andreas0xffff commented 3 years ago

Description

When using TouchEffect not in combination with a ViewGroup/Layout, but with a direct VisualElement (e.g. Label), the CanExecute state is ignored for the native animation/ripple.

Steps to Reproduce

  1. Add a TouchEffect with native animation in XCT Sample App main label: https://github.com/xamarin/XamarinCommunityToolkit/blob/a09a14fb58b3e119594dfe9222506e16266ac8b2/samples/XCT.Sample/Pages/WelcomePage.xaml#L26
    ....
    <Label
    Margin="20,32,20,0"
    Style="{StaticResource label_welcome_header}"
    Text="Introducing the Xamarin Community Toolkit"
    xct:TouchEffect.NativeAnimation="True"
    xct:TouchEffect.Command="{Binding HeaderLabelCommand}"
    />
    ...
  2. Add a command with alternating CanExecute value: https://github.com/xamarin/XamarinCommunityToolkit/blob/a09a14fb58b3e119594dfe9222506e16266ac8b2/samples/XCT.Sample/ViewModels/WelcomeViewModel.cs#L13

    public class WelcomeViewModel : BaseGalleryViewModel
    {
    public ICommand HeaderLabelCommand { get; }
    
    public WelcomeViewModel()
    {
        HeaderLabelCommand = CommandFactory.Create((t) => { }, (x) => ((System.DateTime.Now.Second / 10) % 2) == 0);
    }
    ...
  3. The ripple is drawn all the time. Thats wrong.

When using the TouchEffect with native animation on an StackLayout, the ripple is correctly drawn only when CanExecute is true.

Basic Information

Workaround

Use TouchEffect not on an direct VisualElement, use an intermediate Layout/ViewGroup.

Reproduction imagery

Animation2

Andreas0xffff commented 3 years ago

One possible solution might be a change in the StartRipple function in PlatformTouchEffect.android.cs: https://github.com/xamarin/XamarinCommunityToolkit/blob/d154bd7bcf87347f3a9c1a6aab976be2bdd4e2c5/src/CommunityToolkit/Xamarin.CommunityToolkit/Effects/Touch/PlatformTouchEffect.android.cs#L310

void StartRipple(float x, float y)
{
    if (effect?.IsDisabled ?? true)
        return;

    if (effect.CanExecute && effect.NativeAnimation)
    {
        UpdateRipple();
        if (rippleView != null)
        {
            rippleView.Enabled = true;
            rippleView.BringToFront();
            ripple?.SetHotspot(x, y);
            rippleView.Pressed = true;
        }
        else if (IsForegroundRippleWithTapGestureRecognizer)
        {
            ripple?.SetHotspot(x, y);
            View.Pressed = true;
        }
    }
    else
    {
        // Change RippleColor to transparent => only way to prevent ripple drawing if CanExecute = false
        if (rippleView == null && ripple != null)
        {
            if (rippleColor != Forms.Color.Transparent)
            {
                rippleColor = Forms.Color.Transparent;
                ripple.SetColor(new ColorStateList( new[] { new int[] { } },
                                             new[] { (int)rippleColor.ToAndroid() }));
            }
        }
    }
}
AndreiMisiukevich commented 3 years ago

@Andreas0xffff I think it's not bad idea :)

maybe just (and it should be fine)

else 
{
    ripple?.SetColor(new ColorStateList(new[] { new int[] { } }, new[] { (int)Forms.Color.Transparent.ToAndroid() }));
}