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

[Android/iOS] Swipe to close in SwipeView #9664

Merged
merged 24 commits into from
Mar 20, 2020
Merged

[Android/iOS] Swipe to close in SwipeView #9664

merged 24 commits into from
Mar 20, 2020

Conversation

jsuarezruiz
Copy link
Contributor

@jsuarezruiz jsuarezruiz commented Feb 21, 2020

Description of Change

In this PR we fix several issues related to the swipe gesture in SwipeView:

  • Allow swipe to close SwipeItems.
  • Allow to move from one open SwipeItem to another doing swipe.
  • Invalid touch in SwipeItems closing the SwipeView.

Issues Resolved

API Changes

None

Platforms Affected

  • iOS
  • Android

Behavioral/Visual Changes

None

Before/After Screenshots

Before

wrong-behavior

Realize that to close the SwipeView the tap works but not the swipe. In a similar way, trying to move from the left open SwipeItem to the right one was not possible without previously closing the SwipeView.

After

Android
swipe-close-droid
swipe-items-droid

iOS
swipe-close-ios
swipe-items-ios

Testing Procedure

PR Checklist

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

Copy link
Member

@jfversluis jfversluis left a comment

Choose a reason for hiding this comment

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

Build errors :(

##[error]Xamarin.Forms.Controls.Issues\Xamarin.Forms.Controls.Issues.Shared\Issue8781.xaml.cs(42,5): Error CS0103: The name 'resultEntry' does not exist in the current context

I think you need to do some #if APP like suggested

@jfversluis
Copy link
Member

##[error]Xamarin.Forms.Controls.Issues\Xamarin.Forms.Controls.Issues.Shared\Issue8781.xaml.cs(37,14): Error CS1998: This async method lacks 'await' operators and will run synchronously. Consider using the 'await' operator to await non-blocking API calls, or 'await Task.Run(...)' to do CPU-bound work on a background thread.

@samhouts samhouts added the i/high Completely doesn't work, crashes, or is unusably slow, has no obvious workaround; occurs less often label Feb 27, 2020
@jsuarezruiz jsuarezruiz added the blocker Issue blocks next stable release. Prioritize fixing and reviewing this issue. label Mar 2, 2020
@vniehues
Copy link

vniehues commented Mar 4, 2020

In Previous versions the SwipeView was preventing underlying TapGestureRecognizers from working;
e.g. You have a list with swipeViews but you also want to open a detail page using a simple tap. that tap didn't come through.

Is that fixed as well? Or should I open a new issue for that one?

@jsuarezruiz
Copy link
Contributor Author

jsuarezruiz commented Mar 4, 2020

@vniehues Could you take a look to this PR #9638?, do you mean the selection events?. In that PR we added tests using ListView, CollectionView events and also using GestureRecognizer.
swipeview-tapgesture

@vniehues
Copy link

vniehues commented Mar 5, 2020

@jsuarezruiz In the PR and the gif you showed it looks like my issue will be resolved.

Just to clarify, I don't mean the built-in selection event. In my case I added a TapGestureRecognizer to the ItemTemplates first child (StackLayout in this case) but the TapGestureRecognizer did not get the Input. At least it didn't call the specified command (which it did before adding the SwipeView).

Do I have to add the TapGestureRecognizer to the SwipeView or its Content (e.g. StackLayout)?

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.

Nice! would it be possible to add some UI Tests? Especially for the swiping back and forth scenarios?

That swipe to reveal and entry with a math question is awesome!!

Copy link
Member

@jfversluis jfversluis left a comment

Choose a reason for hiding this comment

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

A UI Test is always a good idea! So maybe add that, but this looks good to go so merge at will or add the test and merge then ;)

@jsuarezruiz
Copy link
Contributor Author

Thanks for the feedback! I have added more samples and UITests to verify the SwipView opening and closing by doing swipe.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
a/shell 🐚 a/swipeview blocker Issue blocks next stable release. Prioritize fixing and reviewing this issue. ControlGallery i/high Completely doesn't work, crashes, or is unusably slow, has no obvious workaround; occurs less often p/Android p/iOS 🍎 proposal-accepted t/bug 🐛 t/enhancement ➕
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants