Skip to content
This repository has been archived by the owner on May 1, 2024. It is now read-only.

[iOS] Fix issue using TapGesture #11419

Merged
merged 10 commits into from
Dec 4, 2020
Merged

[iOS] Fix issue using TapGesture #11419

merged 10 commits into from
Dec 4, 2020

Conversation

jsuarezruiz
Copy link
Contributor

@jsuarezruiz jsuarezruiz commented Jul 13, 2020 •
edited
Loading

Description of Change

Fix issue using TapGesture on iOS.

Issues Resolved

API Changes

None

Platforms Affected

  • iOS

Behavioral/Visual Changes

None

Before/After Screenshots

fix10623

Testing Procedure

Launch Core Gallery and navigate to the issue 10623. Tap the rating control. If you can modify the number of selected stars, the test has passed.

PR Checklist

  • Targets the correct branch
  • Tests are passing (or failures are unrelated)

@jsuarezruiz jsuarezruiz added t/bug 🐛 p/iOS 🍎 i/high Completely doesn't work, crashes, or is unusably slow, has no obvious workaround; occurs less often a/gestures 🖖 a/shapes labels Jul 13, 2020
@samhouts samhouts added the Core label Jul 13, 2020
@samhouts samhouts requested review from hartez and PureWeen July 13, 2020 16:37
@samhouts samhouts changed the base branch from 4.7.0 to 4.8.0 July 28, 2020 16:03
@samhouts
Copy link
Member

@jsuarezruiz The test needs to be updated to use brushes.

@PureWeen PureWeen added the DO-NOT-MERGE-!!! 🛑 This is in progress and needs to be updated before it can be merged. label Jul 30, 2020
@samhouts samhouts changed the base branch from 4.8.0 to 5.0.0 August 4, 2020 22:17
@samhouts samhouts added this to the 5.0.0 milestone Aug 4, 2020
@jsuarezruiz jsuarezruiz removed the DO-NOT-MERGE-!!! 🛑 This is in progress and needs to be updated before it can be merged. label Aug 17, 2020
@jsuarezruiz
Copy link
Contributor Author

Updated the test to use brushes.

PureWeen
PureWeen suggested changes Aug 17, 2020
View reviewed changes
Copy link
Contributor

@PureWeen PureWeen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

  D:\agent\1\s\Xamarin.Forms.Controls.Issues\Xamarin.Forms.Controls.Issues.Shared\Issue11050.xaml.cs(15,29): error CS0117: 'UITestCategories' does not contain a definition for 'Shapes' [D:\agent\1\s\Xamarin.Forms.Core.macOS.UITests\Xamarin.Forms.Core.macOS.UITests.csproj]

@samhouts samhouts requested a review from PureWeen August 24, 2020 21:34
@dhindrik dhindrik mentioned this pull request Sep 7, 2020
Open
@jsuarezruiz jsuarezruiz mentioned this pull request Oct 23, 2020
Closed
@PureWeen PureWeen added the blocker Issue blocks next stable release. Prioritize fixing and reviewing this issue. label Nov 2, 2020
@bholmes
Copy link

bholmes commented Nov 3, 2020 •
edited
Loading

Wow you are kidding me? I do not commit to the repo for years, and one day I decide to pick off something that seems easy. I start to type the commit message and find that you have created this PR.

Of course I am not really upset, just facepalming at the irony.

The solution I had was not as good, I disabled user interaction on all the native views in the shape renderer class allowing events to pass through. Also my focus was only on 12685

@bholmes
Copy link

bholmes commented Nov 3, 2020

Here is a copy of my test for this. Verified that the patch passes this test.

https://gist.github.com/bholmes/f7b68c31641e99019c6b11ee6286b575

@jsuarezruiz
Copy link
Contributor Author

jsuarezruiz commented Nov 4, 2020 •
edited
Loading

:( Sorry. Actually this PR was related to 10623 until recently I tried and added 12685 (which is exactly the same).
But, I have added your tests for 12685!. Thanks.

@PureWeen
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

No pipelines are associated with this pull request.

@rmarinho
Copy link
Member

Failing test on iOS not related

@jsuarezruiz jsuarezruiz mentioned this pull request Dec 2, 2020
Closed
rmarinho
rmarinho reviewed Dec 3, 2020
View reviewed changes
Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Issue10623.cs
{
public Issue10623()
{
Device.SetFlags(new List<string>(Device.Flags ?? new List<string>()) { "Shapes_Experimental" });
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

think we don't need this right?

@rmarinho
Copy link
Member

rmarinho commented Dec 3, 2020

We should run all uitests as this test is not just on iOS

@rmarinho rmarinho merged commit fdb0dac into 5.0.0 Dec 4, 2020
@rmarinho rmarinho deleted the fix-10623 branch December 4, 2020 12:11
pictos pushed a commit to pictos/Xamarin.Forms that referenced this pull request Dec 30, 2020
@jsuarezruiz @samhouts
@hartez @rmarinho @pictos
[iOS] Fix issue using TapGesture (xamarin#11419) fixes xamarin#10623 f…
c9dff77 
…ixes xamarin#12685 fixes xamarin#13020

* Added repro sample

* Fixed the issue

* Update Issue11050.xaml.cs

* Updated issue 10626. Replaced Color by Brush.

* Fixed build error

* Fixed build error

* Added Issue12685 test

* Changes in text format

* Fix rebase error

Co-authored-by: Samantha Houts <[email protected]>
Co-authored-by: E.Z. Hart <[email protected]>
Co-authored-by: Rui Marinho <[email protected]>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@rmarinho rmarinho rmarinho left review comments

@PureWeen PureWeen PureWeen approved these changes

@hartez hartez hartez approved these changes

Assignees

@hartez hartez

@PureWeen PureWeen

Labels
a/gestures 🖖 a/shapes blocker Issue blocks next stable release. Prioritize fixing and reviewing this issue. Core i/high Completely doesn't work, crashes, or is unusably slow, has no obvious workaround; occurs less often p/iOS 🍎 t/bug 🐛
Projects
None yet
Milestone
5.0.0
6 participants
@jsuarezruiz @samhouts @bholmes @PureWeen @rmarinho @hartez