[iOS] Tapping on ListView with two fingers should not crash #379

Merged
merged 4 commits into from Oct 11, 2016

Conversation

Projects
None yet
8 participants
@adrianknight89
Contributor

adrianknight89 commented Sep 25, 2016

Description of Change

Tapping on ListView with two/three fingers crashed iOS app when context action cells are used. This change ensures such taps are ignored. See the changes made in static void Tapped(UIGestureRecognizer recognizer)

Also made the following refactorings:

- static readonly field name change
- shorter getter
- object initialization
- convert to foreach loops
- ternary operator

Bugs Fixed

@dnfclas

This comment has been minimized.

Show comment
Hide comment
@dnfclas

dnfclas Sep 25, 2016

Hi @adrianknight89, I'm your friendly neighborhood .NET Foundation Pull Request Bot (You can call me DNFBOT). Thanks for your contribution!
You've already signed the contribution license agreement. Thanks!

The agreement was validated by .NET Foundation and real humans are currently evaluating your PR.

TTYL, DNFBOT;

dnfclas commented Sep 25, 2016

Hi @adrianknight89, I'm your friendly neighborhood .NET Foundation Pull Request Bot (You can call me DNFBOT). Thanks for your contribution!
You've already signed the contribution license agreement. Thanks!

The agreement was validated by .NET Foundation and real humans are currently evaluating your PR.

TTYL, DNFBOT;

@StephaneDelcroix

Hey Adrian,

Please, really please, stop trying to refactor the code when it's not necessary.

I'd like to discuss the core of your Pos, but I just can't

@@ -195,9 +191,8 @@ public void Update(UITableView tableView, Cell cell, UITableViewCell nativeCell)
_scroller.Frame = new RectangleF(0, 0, width, height);
isOpen = ScrollDelegate.IsOpen;
- for (var i = 0; i < _buttons.Count; i++)
+ foreach (var b in _buttons)

This comment has been minimized.

@StephaneDelcroix

StephaneDelcroix Sep 26, 2016

Member

we do for loops on purpose. It avoids creating enumerators.

@StephaneDelcroix

StephaneDelcroix Sep 26, 2016

Member

we do for loops on purpose. It avoids creating enumerators.

This comment has been minimized.

@adrianknight89

adrianknight89 Sep 26, 2016

Contributor

i reverted the refactorings. see the changes now.

@adrianknight89

adrianknight89 Sep 26, 2016

Contributor

i reverted the refactorings. see the changes now.

@cleardemon

This comment has been minimized.

Show comment
Hide comment
@cleardemon

cleardemon Sep 28, 2016

Just wanted to add that context actions, as mentioned in the description, are not necessarily related to this; the wider problem seems to be simply tapping on something else as well as the ListView, at the same time. This fix was the same conclusion that I came to internally (but never got the chance to do the PR :).

Just wanted to add that context actions, as mentioned in the description, are not necessarily related to this; the wider problem seems to be simply tapping on something else as well as the ListView, at the same time. This fix was the same conclusion that I came to internally (but never got the chance to do the PR :).

var table = (UITableView)recognizer.View;
+ if (selector._lastPath == null)

This comment has been minimized.

@jamesmontemagno

jamesmontemagno Sep 30, 2016

Should we move this above the UITableView cast? Seems like we could avoid doing that since it wont go any further.

@jamesmontemagno

jamesmontemagno Sep 30, 2016

Should we move this above the UITableView cast? Seems like we could avoid doing that since it wont go any further.

This comment has been minimized.

@adrianknight89

adrianknight89 Sep 30, 2016

Contributor

Good catch. Done.

@adrianknight89

adrianknight89 Sep 30, 2016

Contributor

Good catch. Done.

@fabmax94

This comment has been minimized.

Show comment
Hide comment
@fabmax94

fabmax94 Oct 6, 2016

Hello, I'm building an application for IOS using Xamarin.Forms and I got an exception (https://bugzilla.xamarin.com/show_bug.cgi?id=44786
) that your commit seems to be fix it. Then I try to download your project from this commit as I mentioned and generate the dll, but when I replace the old Xamarin.Form to the new one I got an Unhandle Exception after the splash screen.

fabmax94 commented Oct 6, 2016

Hello, I'm building an application for IOS using Xamarin.Forms and I got an exception (https://bugzilla.xamarin.com/show_bug.cgi?id=44786
) that your commit seems to be fix it. Then I try to download your project from this commit as I mentioned and generate the dll, but when I replace the old Xamarin.Form to the new one I got an Unhandle Exception after the splash screen.

@samhouts samhouts merged commit 9a5dab9 into xamarin:master Oct 11, 2016

@samhouts samhouts added this to the 2.3.4 milestone Jun 27, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment