Xamarin.forms: [Android] Attached Touch Listener events do not dispatch to immediate parent Grid Renderer View on Android when Child fakes handled

[pauldipietro]

Migrated from https://bugzilla.xamarin.com/show_bug.cgi?id=60823

Description

Touch listeners attached by a user do not dispatch to the parent view when the child element has "fake" handled an Event

Grid
-- Label

Label fake handles OnTouchEvent which causes the parent view during the DispatchTouchEvents phase to not call OnTouch on Listeners attach via SetTouchListener . OnTouchEvent is successfully called which is why Xamarin Forms Gestures continue to function properly but if a user has a custom listener they have assigned that will not be called

Steps to Reproduce

1. Tap on the red text "Tap me (inner Grid)!"

Expected Behavior

It should become "Was tapped: Inner Grid (red label)"

Actual Behavior

The above does not occur and the text refers to the green label.

Basic Information

• Version with issue: Releases after 2.3.4.270
• Last known good version: 2.3.4.270
• Platform Target Frameworks:
  
  
  
  • Android: 8.0
  
  
• Android Support Library Version: 25.4.0.2

Screenshots

N/A

Reproduction Link

Original Bugzilla attachment working on 2.3.4.270

[V0ffka71]

@hartez Hi! Any updates? This issue has a big impact on our GridControl.

[deczaloth]

Hi! My team's project is also being affected by this issue. Hope to see some progress on it!

[raducaia]

Also one of my projects wait for an update on this issue. I hope to be resolved soon.

[MichaelRumpler]

@V0ffka71 I played a bit with your repro project and it started working, when I replaced the inner Xamarin.Forms.Grid with an InnerGrid and used your InnerGridRenderer (which was included in the zip, but not in the solution).
If this alone does not work for you you could also override DispatchTouchEvent in your InnerGridRenderer. This method is always called. But it is called for every element on a position, so for MainGrid and InnerGrid. So in the MainGridRenderer you'd need to ignore it if actually the InnerGrid was touched.
I only checked with XF 2.4.0.269-pre2 as there were several other (unrelated) errors when I tried to update to XF 2.5.1.

[davidortinau]

Thanks @MichaelRumpler . I loaded up the sample project "as-is" and it works for me.

2.3.4.270
Support 23.3.0
Android 8.1 on a Pixel

This does NOT work for me:
3.0.0-pre3
Support 27.0.2
Android 8.1 on a Pixel

TapTest-3.0.0-pre3.zip

@MichaelRumpler perhaps you could test the solution you mention with this new project?

[PureWeen]

Quick update

Still looking for the best way to fix this but the core issue comes from the label in the grid
https://github.com/xamarin/Xamarin.Forms/blob/master/Xamarin.Forms.Platform.Android/Renderers/LabelRenderer.cs#L223

it seems like this call here
https://github.com/xamarin/Xamarin.Forms/blob/8c23ad6c893b8e5b7f3fd2573c4ebde0a0441950/Xamarin.Forms.Platform.Android/Platform.cs#L1210

Doesn't "make up" for the true returned from the Label. That event is called by the innerGrid but that doesn't cause a propagation out to anyone watching

There's a dance that happens with propagating events in Android and having the Label in the Grid causes the upstream event to not dispatch to the Inner Grid correctly

For example If you just remove the label from the inner grid and click the grid then the event dispatches to the inner grid

As a workaround you can set the label in the grid to InputTransparent=true

Also Xamarin Forms level GestureRecognizers work fine if you add them on the different grid elements

            tapGrid.GestureRecognizers.Add(new TapGestureRecognizer()
            {
                Command = new Command(() =>
                {

                })
            });
            RootGrid.GestureRecognizers.Add(new TapGestureRecognizer()
            {
                Command = new Command(() =>
                {

                })
            });

The sample is pulling the renderers Android side and adding its own Android level gestures

CustomGestureListener innerListener = new CustomGestureListener("Inner Grid (red label)", mainPage.TextLabel);
            innerListener.BindToElement(mainPage.TapGrid, rootGridRenderer);

            CustomGestureListener rootListener = new CustomGestureListener("Root Grid (green label)", mainPage.TextLabel);
            rootListener.BindToElement(mainPage.RootGrid, rootGridRenderer);

[PureWeen]

@V0ffka71 @deczaloth @raducaia

Can you talk about your use case as it relates to this issue a bit? I'm asking because there was a lot of work done to make the Xamarin Forms Gesture Recognizers work properly and consistently between platforms which is actually what caused this to "Break".

The Gesture Recognizers that are part of Xamarin Forms seem to take care of what's being coded here in Android

The issue here is attaching an Android Gesture in the middle of the view renderer tree and I'm not sure if that's something that will play well with how we need the Gestures to work but perhaps if I understood your use cases a bit more we could figure out the best way to address your issue .

[PureWeen]

I checked in a fix for this but this still feels very isolated so I'm curious how so many people ran into it and if the issue people are having is actually something else.

Another really simple way to get around this issue is to just implement a renderer that doesn't do anything for the parent element. So in the attached project if the parent grid was just changed to

        <local:MainGrid Grid.Row="1" x:Name="tapGrid" HeightRequest="20" >
            <Label Text="Tap me (inner Grid)!" BackgroundColor="Red" />
        </local:MainGrid >

Is it pretty common for Xamarin Forms users to iterate the entire Android visual tree and attach touch listeners themselves at every level?

[MichaelRumpler]

I can't speak for the others, but I am only a user of the DevExpress.Mobile.Grid. They said, they cannot update their XF dependency because of this bug. As I also know a thing or two about gestures, I decided to have a look at it. But my own code did not fail because of this.

In MR.Gestures I override DispatchTouchEvent and use my own GestureListeners there. I don't iterate over the visual tree, but rather do this in every renderer of a MR.Gestures element. I did it this way to ensure that my listeners are called even though the user or Xamarin may set their own TouchListener.

[deczaloth]

Hi @PureWeen, I also came to know about this issue because of the Grid control of DevExpress. I am developing an app using this control and it restricts the version of Xamarin Forms to 2.3.4.247 due to this bug.

@V0ffka71 I think you should be the most appropriate to give details about the exact use case of DevExpress.

[V0ffka71]

@MichaelRumpler Michael, thank you for putting me on the right track. I have played with our source code and found a way to apply this workaround.

@PureWeen We faced this issue during an update of our GridControl (https://www.nuget.org/packages/DevExpress.Mobile.Grid/) to the latest Xamarin.Forms. I've attached a sample project illustrating our use case (https://drive.google.com/file/d/17zKsM8J96Yvpj4RSxAeim4HZGePtOZlg/view?usp=sharing). Because of this issue, taps in "Sort Header" are not passed to the header container. We use two gesture listeners: for the header container (with the default renderer) and for its parent (a Grid descendant with a custom renderer). I've found that if I replace the default renderer of the header container, all works correctly. But I suppose that this is not a correct solution because there is some gesture logic in the default renderer, which will be lost in this case. Also, we can't derive the default renderer to preserve its logic because it is internal. So, I can see 2 available solutions: make the default renderer public or fix the touch logic in the default renderer. :)

[PureWeen]

I've found that if I replace the default renderer of the header container, all works correctly. But I suppose that this is not a correct solution because there is some gesture logic in the default renderer,

I had this thought but was thinking since you were wanting to rely on your own Gesture Listeners that maybe it wouldn't matter since Gesture Recognizers just weren't cutting it. But having that logic "lost" from implementing a custom renderer probably isn't the best result so I'm going to poke that one a little bit.

In the end I do agree with @MichaelRumpler about the overriding as that seems like the most reliable way to not be infringed upon by the user

[PureWeen]

But I suppose that this is not a correct solution because there is some gesture logic in the default renderer,

The point of that renderer (and why it's internal) is for us to be able to know that the renderer being used is our own and thus we can just do whatever to it in order to minimize breaking other peoples renderers.

So for example the Gesture code inside Default Renderer is only really triggered if the parent renderer of an element is a default renderer
https://github.com/xamarin/Xamarin.Forms/blob/7e4628e0dd645aae03358b4a74639a2feed64f0d/Xamarin.Forms.Platform.Android/Renderers/MotionEventHelper.cs#L17

Currently the point of that logic is to stop touch events from bubbling through the Visual Tree

-Grid (DefaultRenderer)
--Label (if you touch me I will return true for OnTouchEvent)
---Label (No touch events will reach me)

-Grid (CustomRenderer)
--Label (if you touch me I will return false for OnTouchEvent)
---Label (My touch events will now be called like they normally do get called on android)

The short circuiting was put in place to make the Android eventing fall more in line with other platforms so that all the GestureRecognizers would fire in a consistent way cross platform. Having an internal DefaultRenderer lets us say ok the user isn't using a custom renderer so let's do our own thing or if the user is using a custom renderer then whatever they've setup should now not break.

I ran a few tests and the Gesture Recognizers still works for the inner grid if it uses a custom renderer

[ozzioma]

Guys any conclusive fix for this?
I'm trying to get use the DevExpress Grid Control for Xamarin and its bombing out on this issue.
Not to mention the insistence on a -Version 2.3.4.247 for the grid control to work.
Been struggling with this all day.

Any one fixed this yet or found a work around?

Thanks

[deczaloth]

@ozzioma The issue is already fixed and merged to the master branch as stated in this link. The question i think would be rather WHEN is this fix to acctually be part of a stable release. My company is also relaying in DevExpress GridColumn and this issue is stopping us from updating Xamarin.Forms which is a pity in view of all the good thinks that have been done in the last year. So, @samhouts @PureWeen @rmarinho @davidortinau when are we to expect this fix to appear in a stable release of XF?

Thanks

[MichaelRumpler]

But @V0ffka71 said above that he found and applied a workaround. Can't you release the DevExpress Grid with that workaround?
In my experience waiting for XF to release the fix will take about half a year (a PR of mine was merged into master in November and released last month). This is too long and I'll have to substitute the DX Grid with something else.

[V0ffka71]

@MichaelRumpler @deczaloth @ozzioma Hi, we are working on an update for the GridControl to support the latest stable Xamarin.Forms (3.0.0.550146). This update will be available soon, please stay tuned.

[PureWeen]

That's great to hear @V0ffka71 !!

[ozzioma]

@V0ffka71 thanks for the update!
However, is it possible to do an alpha or RC release first?
Maybe we can help test and check for bugs before the final release?
I think the broad user base can help with testing on different platforms and cover many scenarios your team might not think of.
Just my 2 cents.

Regards