search

AndreiMisiukevich / TouchEffect

UI-responsive touch effects for Xamarin.Forms
MIT License
195 stars 33 forks source link

TouchEffect inside CollectionView DataTemplate not letting select item on Android #75

Closed Estevete closed 4 years ago

Estevete commented 4 years ago

Placing TouchEffect inside the DataTemplate of a CollectionView (I have tried Frame & Grid) does not work on Android.

I have tried by calling SelectionChangedCommandand doesn't work either. I have tested on iOS and UWP and it works.

Repro link: https://github.com/MADSENSE/Madsense.XamarinForms.Sample/tree/collection-view-frame-android-issue

AndreiMisiukevich commented 4 years ago

does it work with a content view?

Estevete commented 4 years ago

I have tried and no. It doesn't work either with a ContentView

YZahringer commented 4 years ago

I think it's not specific to CollectionView, TapGestureRecognizer does not work on any element with TouchEffect on Android

AndreiMisiukevich commented 4 years ago

So, probably this is the limitation of this package. You should use TouchEff.Command for handling taps in CollectionView instead of default CollectionView's command.

YZahringer commented 4 years ago

@AndreiMisiukevich I think the problem is on e.Handled = true. Any reason to handle the event? This one is not handled on iOS and UWP. https://github.com/AndreiMisiukevich/TouchEffect/blob/f16d527aecaa6845fcc137110c1c4fcae3e6f328/TouchEffect.Droid/PlatformTouchEff.cs#L119-L127

AndreiMisiukevich commented 4 years ago

@AndreiMisiukevich I think the problem is on e.Handled = true. Any reason to handle the event? This one is not handled on iOS and UWP. https://github.com/AndreiMisiukevich/TouchEffect/blob/f16d527aecaa6845fcc137110c1c4fcae3e6f328/TouchEffect.Droid/PlatformTouchEff.cs#L119-L127

Hm, I thought it makes sense to mark the event as "handled" if it's handled indeed :) Do you think we can safely remove this line?

YZahringer commented 4 years ago

I've done some tests and I don't see any issue without handling it. The e.Handled should be set to False, the default value is True.

I think the behavior should be the same on different platforms, the event should be handled on all or none. Currently is handled on Android but not on iOS or UWP.

I often define TouchEffect in a global Style, that's why I use GestureRecognizer in the element. It works well, TouchEffect provides visual states and the gestures execute the commands.

AndreiMisiukevich commented 4 years ago

I've done some tests and I don't see any issue without handling it. The e.Handled should be set to False, the default value is True.

I think the behavior should be the same on different platforms, the event should be handled on all or none. Currently is handled on Android but not on iOS or UWP.

I often define TouchEffect in a global Style, that's why I use GestureRecognizer in the element. It works well, TouchEffect provides visual states and the gestures execute the commands.

Well, if it works well even in the ScrollView sample, then I will appreciate submitting a PR with the fix :) Would you like to do it?