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

[Android/iOS] Fix programmatically open right SwipeItems #10575

Merged
merged 16 commits into from
Dec 18, 2020
Merged

[Android/iOS] Fix programmatically open right SwipeItems #10575

merged 16 commits into from
Dec 18, 2020

Conversation

jsuarezruiz
Copy link
Contributor

@jsuarezruiz jsuarezruiz commented May 5, 2020 •
edited
Loading

Description of Change

Changes:

  • Fix programmatically open right SwipeItems.
  • Added animated parameter to open and close the SwipeView.

Issues Resolved

API Changes

Added Animated parameter to OpenSwipeEventArgs and CloseSwipeEventArgs.
Now, can programmatically open and close the swipeView, animated or not.

void Open(OpenSwipeItem openSwipeItem, bool animated = true)
void Close(bool animated = true)

Platforms Affected

  • iOS
  • Android

Behavioral/Visual Changes

None

Before/After Screenshots

fix10563
animated-openclose-swipeview

Testing Procedure

PR Checklist

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

@jsuarezruiz jsuarezruiz added the API-change Heads-up to reviewers that this PR may contain an API change label May 5, 2020
@samhouts samhouts added this to In Review in SwipeView May 5, 2020
@samhouts samhouts added the blocker Issue blocks next stable release. Prioritize fixing and reviewing this issue. label May 7, 2020
@samhouts samhouts added this to Ready for Review (PRs) in Sprint 170 May 7, 2020
@samhouts samhouts added this to Ready for Review (PRs) in Sprint 171 May 21, 2020
@samhouts samhouts moved this from Ready for Review (PRs) to Continued in next sprint in Sprint 170 May 26, 2020
@samhouts samhouts requested review from PureWeen and hartez and removed request for StephaneDelcroix May 28, 2020 00:07
PureWeen
PureWeen suggested changes Jun 3, 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.

Since the initial issue this is based on fixes a bug

Open method should open view and reveal right items.
I tested the same but with TopItems and that did not work either. :(

Can some automated tests be added to check for these?

@samhouts samhouts changed the base branch from 4.6.0 to 4.7.0 June 8, 2020 21:53
@samhouts samhouts moved this from Ready for Review (PRs) to Returned to backlog in Sprint 171 Jun 14, 2020
hartez
hartez suggested changes Jun 16, 2020
View reviewed changes
Xamarin.Forms.Core/SwipeEventArgs.cs Outdated
@@ -12,14 +12,26 @@ protected BaseSwipeEventArgs(SwipeDirection swipeDirection)
public SwipeDirection SwipeDirection { get; set; }
}

public class CloseSwipeEventArgs : EventArgs
Copy link
Contributor

Choose a reason for hiding this comment

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

I think these should be named SwipeClosedRequestEventArgs and SwipeOpenRequestEventArgs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Renamed.

Copy link
Contributor

Choose a reason for hiding this comment

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

Now that I look at this again, we should match the event names with the event arguments names as closely as we can. So now I think it should be CloseRequestedEventArgs and OpenRequestedEventArgs.

@samhouts samhouts added this to In Progress in 4.7.0 Jun 20, 2020
@jsuarezruiz
Copy link
Contributor Author

@PureWeen Added UITests.

hartez
hartez suggested changes Jun 30, 2020
View reviewed changes
Xamarin.Forms.Core/SwipeEventArgs.cs Outdated
@@ -12,14 +12,26 @@ protected BaseSwipeEventArgs(SwipeDirection swipeDirection)
public SwipeDirection SwipeDirection { get; set; }
}

public class CloseSwipeEventArgs : EventArgs
Copy link
Contributor

Choose a reason for hiding this comment

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

Now that I look at this again, we should match the event names with the event arguments names as closely as we can. So now I think it should be CloseRequestedEventArgs and OpenRequestedEventArgs.

@samhouts samhouts added this to Ready for Review (PRs) in Sprint 172 Jun 30, 2020
@PureWeen PureWeen requested a review from hartez July 4, 2020 01:20
@samhouts samhouts added this to Ready for Review (PRs) in Sprint 173 via automation Jul 6, 2020
@jsuarezruiz jsuarezruiz mentioned this pull request Oct 28, 2020
Closed
@jsuarezruiz jsuarezruiz removed DO-NOT-MERGE-!!! 🛑 This is in progress and needs to be updated before it can be merged. retarget-branch-required PR or associated issues target a milestone. Please target this PR to the matching branch. labels Oct 28, 2020
@PureWeen PureWeen moved this from In Progress to In Review in vNext+1 (5.0.0) Nov 2, 2020
@PureWeen
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

No pipelines are associated with this pull request.

@PureWeen PureWeen added the DO-NOT-MERGE-!!! 🛑 This is in progress and needs to be updated before it can be merged. label Nov 20, 2020
PureWeen
PureWeen suggested changes Nov 20, 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.

UI Tests are failing

@rmarinho
Copy link
Member

@jsuarezruiz tests are failing .. can you review please?

@jsuarezruiz
Copy link
Contributor Author

@rmarinho All the tests are passing now.

@jsuarezruiz jsuarezruiz removed the DO-NOT-MERGE-!!! 🛑 This is in progress and needs to be updated before it can be merged. label Dec 15, 2020
@rmarinho
Copy link
Member

@jsuarezruiz seems its failing on iOS still..

@jsuarezruiz
Copy link
Contributor Author

Captura de pantalla 2020-12-17 a las 18 42 55

@rmarinho rmarinho merged commit 50e6fbb into 5.0.0 Dec 18, 2020
SwipeView automation moved this from In Review to Done Dec 18, 2020
vNext+1 (5.0.0) automation moved this from In Review to Done Dec 18, 2020
@rmarinho rmarinho deleted the fix-10563 branch December 18, 2020 11:05
@Only-Xam Only-Xam mentioned this pull request Jan 9, 2021
Open
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@hartez hartez hartez approved these changes

@rmarinho rmarinho rmarinho approved these changes

@PureWeen PureWeen Awaiting requested review from PureWeen

Assignees

@hartez hartez

@PureWeen PureWeen

Labels
a/swipeview API-change Heads-up to reviewers that this PR may contain an API change blocker Issue blocks next stable release. Prioritize fixing and reviewing this issue. ControlGallery p/Android p/iOS 🍎 p/Tizen t/bug 🐛
Projects
SwipeView
  
Done
Sprint 170
  
Continued in next sprint
Sprint 171
  
Returned to backlog
Sprint 172
  
Continued in next sprint
Sprint 173
  
Continued in next sprint
Sprint 174
  
Ready for Review (PRs)
vNext+1 (5.0.0)
Done
Milestone
5.0.0
Development

Successfully merging this pull request may close these issues.

[Bug] SwipeView Open methods does not work for RightItems
5 participants
@jsuarezruiz @hartez @PureWeen @rmarinho @samhouts