Swift 4 swipeable #1249

marbetschar · 2017-09-26T13:54:20Z

According to #1106 this pull request now goes to the new swift 4 branch.

And a gracefull fallback to the old iOS7 behaviour which allows swipe actions only on the right.

AAverin · 2017-10-01T12:21:35Z

Right when I was wondering how to add swipeable actions – here is a PR =)
Would be great to have it merged

…ift-4-swipeable

If there are any multivalued sections within the form, Eureka automatically enables editingMode to allow swipe actions to be handled correctly. You have to disable editing manually if you do not want this behaviour in forms containing multivalued sections.

marbetschar · 2017-10-19T08:38:51Z

@mtnbarreto can we get this merged?

marbetschar · 2017-10-31T09:46:49Z

any news on this?

kronik · 2017-11-02T08:55:46Z

waiting for this PR to be merged too

marbetschar · 2017-11-16T12:43:43Z

@mtnbarreto whats missing to get this merged? I'd love merging this swipe feature as soon as possible into the official release, since I need it in my current project. That said, it would be awesome to have this pull request either merged - or getting to know whats wrong or missing, so I can fix/add the stuff needed and to get this merged based upon your suggestions :)

Updates from upstream

In order to allow it to be overwritten by a row which subclasses it. This is needed for the new custom row `SplitRow`. `SplitRow` enables to put two Eureka rows side by side into one UITableViewCell.

Made var section open

marbetschar · 2017-12-04T10:14:37Z

@mats-claassen or @mtnbarreto whats missing to get this swipe feature merged or declined?

Added support for the Eureka build which is Swipe and SplitRow aware. This is a temporary branch till these pull requests are merged: - xmartlabs/Eureka#1338 - xmartlabs/Eureka#1249

m-revetria

@marbetschar thanks for your contribution. I left some minor comments.

We'll be glad to merge this after you address them.

Additionally, I'm wondering if there is a way to define a type for all platformValues variables. Using Any for them doesn't look so good for me. I'll be thinking about meanwhile.

m-revetria · 2017-12-08T18:00:24Z

Example/Example/ViewController.swift

-                    self?.showAlert()
-                }
+
+				<<< ButtonRow("Swipe Actions") { (row: ButtonRow) -> Void in


Please use the same indentation that is used in the whole project (4 spaces)

m-revetria · 2017-12-08T18:16:10Z

Example/Example/ViewController.swift

+		super.viewDidLoad()
+
+		form +++ Section(footer: "Eureka sets table.isEditing = false for SwipeActions.\n\nMultivaluedSections need table.isEditing = true, therefore both can't be used on the same view.")
+			<<< LabelRow("Actions Right: iOS >= 7"){


please always keep a white space before an opening curly brace

m-revetria · 2017-12-08T19:43:21Z

Source/Core/BaseRow.swift

@@ -98,7 +98,18 @@ open class BaseRow: BaseRowType {
    public var isHidden: Bool { return hiddenCache }

    /// The section to which this row belongs.
-    public weak var section: Section?
+    open weak var section: Section?


this change does not seem to be needed? Please revert it back

Right. Done.

m-revetria · 2017-12-08T19:46:06Z

Source/Core/SwipeActions.swift

+
+		if #available(iOS 11, *){
+			self.platformValue = UIContextualAction(style: style.platformValue as! UIContextualAction.Style, title: title){ action, view, completion -> Void in
+				handler(action,view,completion)


please keep arguments separated by spaces

m-revetria · 2017-12-08T19:47:45Z

Source/Core/SwipeActions.swift

+
+	public let platformValue: Any
+
+	public var backgroundColor: UIColor?{


These properties seem that will have some effect just on iOS 11. Can't them be available just for iOS 11+?

Good point! Is changed.

m-revetria · 2017-12-08T19:58:23Z

Source/Core/SwipeActions.swift

+
+public class SwipeAction{
+
+	public let platformValue: Any


I think platformValue is not clear enough. Maybe just contextualAction or rowAction

Used contextualAction as suggested.

m-revetria · 2017-12-08T20:02:25Z

Source/Core/SwipeActions.swift

+	public var performsFirstActionWithFullSwipe: Bool = false
+	public var actions: [SwipeAction] = []
+
+	public var platformValue: Any?{


Seems that this can be available just from iOS 11, right? If so then you can set its type to UISwipeActionsConfiguration

Good point! Fixed.

m-revetria · 2017-12-08T20:05:08Z

Source/Core/SwipeActions.swift

+		case normal = 0
+		case destructive = 1
+
+		var platformValue: Any{


Would be great if all these platformValues can have a type more concrete than Any. @mtnbarreto thoughts?

Good point. Used marker protocols to fix this. See SwipeActions.swift at the bottom of the file.

m-revetria

Awesome, grate work! thanks for fixing the comments

mtnbarreto · 2017-12-11T20:10:13Z

Source/Core/SwipeActions.swift

+
+import Foundation
+
+public typealias SwipeActionHandler = (ContextualAction, ContextualActionSource, ((Bool) -> Void)?) -> Void


I think we should pass the row as closure argument.

ContextualActionSource can be removed since the row is passed as argument now.

That's a brilliant idea! We should ideally use the specific Row type right? Not just BaseRow.

Implemented as public typealias SwipeActionHandler = (ContextualAction, BaseRow, ((Bool) -> Void)?) -> Void

mtnbarreto · 2017-12-11T20:16:10Z

Source/Core/SwipeActions.swift

+	public let contextualAction: ContextualAction
+
+	@available(iOS 11, *)
+	public var backgroundColor: UIColor?{


I would prefer to make UIContextualAction and UITableViewRowAction conform to a protocol like this:

public protocol ContextualAction{ var backgroundColor: UIColor? var image: UIImage? var title: String? }

Of course 👍🏼 makes a whole lot more sense.

Had to add the following in order to make UITableViewRowAction conform to the newly created ContextualAction protocol:

extension UITableViewRowAction: ContextualAction { public var image: UIImage? { get { return nil } set { return } } }

mtnbarreto · 2017-12-11T20:26:18Z

Source/Core/SwipeActions.swift

+		configure(self)
+	}
+
+	public var performsFirstActionWithFullSwipe: Bool = false


remove : Bool

mtnbarreto · 2017-12-11T20:32:44Z

Source/Core/SwipeActions.swift

+
+public class SwipeConfiguration{
+
+	public init(configure: (SwipeConfiguration) -> Void){


I would prefer a struct here and each row should have an instance of the strict already created so Eureka users can do something like that:

row.leftSwipe.actions = [swipeAction1, swipeAction2] row.rightSwipe.actions = [swipeAction1, swipeAction2]

init should be refactored to init() in this case.

This looks amazing. One question: Would you prefer using leftSwipe and rightSwipe instead of leadingSwipe and trailingSwipe ?

If I'll look at PushRow, left/right looks more familiar. But due to the nature of this gesture it might lead to confusions.

Done using leadingSwipe and trailingSwipe.

mtnbarreto · 2017-12-11T20:35:04Z

Source/Core/SwipeActions.swift

+		}
+	}
+
+	public var title: String?{


remove it since we can do contextualAction.title, contextualAction.image and contextualAction. backgroundColor and so on can be accessed directly using the protocol containing these properties.

Despite its true these properties can be accessed directly it looks a bit awkward in the form definition code. That's why I also made SwipeAction conforming to the newly created ContextualAction protocol - and let SwipeAction.contextualAction create the platform specific action on demand.

This way we end up with a nice and clean looking form initialisation code:

let infoAction = SwipeAction(style: .normal, title: "Info", handler: { (action, row, completionHandler) in print("Info") completionHandler?(true) }) infoAction.backgroundColor = .blue $0.leadingSwipe.actions = [infoAction] $0.leadingSwipe.performsFirstActionWithFullSwipe = true

mtnbarreto · 2017-12-11T20:47:51Z

Source/Core/SwipeActions.swift

+			}
+		}
+
+		public init(contextualStyle: ContextualStyle){


We don't need this init.

mtnbarreto · 2017-12-11T20:55:32Z

Source/Core/SwipeActions.swift

+		}
+	}
+
+	public enum Style: Int{


unnecessary : Int here.

Compiler throws Enum case cannot have a raw value if the enum does not have a raw type if : Int is removed

mtnbarreto · 2017-12-11T20:57:33Z

Source/Core/SwipeActions.swift

+@available(iOSApplicationExtension 11.0, *)
+extension UIContextualAction.Style: ContextualStyle{}
+
+public protocol ContextualActionSource{}


no longer needed according my feedback.

Removed ContextualActionSource

mtnbarreto · 2017-12-11T21:19:27Z

@marbetschar We also need to add feature documentation in the readme in order to get this merged.

marbetschar · 2017-12-12T12:58:40Z

@mtnbarreto where to put the feature documentation?

As described in #1249

marbetschar · 2017-12-13T17:03:10Z

@mtnbarreto also added feature documentation to README.md

As described in #1249

mtnbarreto · 2017-12-20T15:22:05Z

@marbetschar Finally it got merged! #1361. Thanks for contributing!

marbetschar · 2017-12-20T15:31:51Z

@mtnbarreto perfect timing, thank you very much!!!! 🎉🎄🌟🎅🏻

matiasvillaverde · 2018-01-16T14:12:39Z

Is possible to update this version by Cocoapods? Now the last version is 4.0.1

Thanks for this cool feature!

Mauricio Cousillas and others added 2 commits June 7, 2017 14:23

Swift 4 migration

a0b7c1b

SwipeAction Support for iOS11

806ba34

And a gracefull fallback to the old iOS7 behaviour which allows swipe actions only on the right.

Marco Betschart added 2 commits October 17, 2017 20:43

Merge remote-tracking branch 'upstream/feature/Xcode9-Swift4' into sw…

2f43a69

…ift-4-swipeable

marbetschar mentioned this pull request Nov 1, 2017

Swipeable "Actions" for Eureka rows? #105

Closed

marbetschar mentioned this pull request Nov 26, 2017

Want to contribute actively? #1334

Closed

Marco Betschart added 4 commits December 2, 2017 15:45

Merge pull request #10 from xmartlabs/master

0e3babc

Updates from upstream

Made var section open

be4c01d

In order to allow it to be overwritten by a row which subclasses it. This is needed for the new custom row `SplitRow`. `SplitRow` enables to put two Eureka rows side by side into one UITableViewCell.

Merge pull request #11 from Mandelkind/SplitRow

ffed289

Made var section open

Merge branch 'SplitRowSwipeable' into swift-4-swipeable

72a3951

m-revetria changed the base branch from feature/Xcode9-Swift4 to master December 8, 2017 18:05

m-revetria requested changes Dec 8, 2017

View reviewed changes

Marco Betschart added 2 commits December 9, 2017 13:42

Fixed issues based upon feedback from xmartlabs

017d4a3

Fixed spacing

4ea0ef5

m-revetria approved these changes Dec 11, 2017

View reviewed changes

fedeojeda95 approved these changes Dec 11, 2017

View reviewed changes

mtnbarreto requested changes Dec 11, 2017

View reviewed changes

Marco Betschart added 2 commits December 13, 2017 17:39

Implemented feedback of @mtnbarreto

8c76499

As described in #1249

Added feature documentation to readme

1da1d35

mtnbarreto pushed a commit that referenced this pull request Dec 19, 2017

Implemented feedback of @mtnbarreto

7cdfacd

As described in #1249

mtnbarreto closed this Dec 20, 2017


		public let platformValue: Any

		public var backgroundColor: UIColor?{


		import Foundation

		public typealias SwipeActionHandler = (ContextualAction, ContextualActionSource, ((Bool) -> Void)?) -> Void


		public class SwipeConfiguration{

		public init(configure: (SwipeConfiguration) -> Void){

Swift 4 swipeable #1249

Swift 4 swipeable #1249

Conversation

marbetschar commented Sep 26, 2017

AAverin commented Oct 1, 2017

marbetschar commented Oct 19, 2017

marbetschar commented Oct 31, 2017

kronik commented Nov 2, 2017

marbetschar commented Nov 16, 2017 • edited

marbetschar commented Dec 4, 2017

m-revetria left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

marbetschar Dec 9, 2017 • edited

Choose a reason for hiding this comment

m-revetria left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mtnbarreto commented Dec 11, 2017

marbetschar commented Dec 12, 2017

marbetschar commented Dec 13, 2017

mtnbarreto commented Dec 20, 2017

marbetschar commented Dec 20, 2017

matiasvillaverde commented Jan 16, 2018

marbetschar commented Nov 16, 2017 •

edited

marbetschar Dec 9, 2017 •

edited