Add support for swiping from the specified edge of the screen #1286

Fitzmaz · 2023-06-29T07:16:34Z

As discussed in issue #1281, I tried to address the problem by adding a new method to the tester. This method simulates a screen edge pan gesture by manually setting the _edgeType property. I wonder if there's any better solution, any feedback is greatly appreciated.

justinseanmartin

Couple of discussion points on the approach, but what you have here also seems reasonable. We appreciate the contribution and adding test coverage of the scenario.

justinseanmartin · 2023-06-29T13:58:10Z

Sources/KIF/Classes/KIFUITestActor.m

@@ -389,6 +378,49 @@ - (void)tapScreenAtPoint:(CGPoint)screenPoint
    }];
 }

+- (void)swipeFromEdge:(UIRectEdge)edge atY:(CGFloat)y


I think I'd make this a % or the screen height to make it the API work more easily with different device screen sizes, like the dragFromPoint API.

Do you have a use case where you care about the Y coordinate used for the edge swipe? If not, I think we should start by omitting the atY: parameter and default to using 50% of the screen height.

justinseanmartin · 2023-06-29T14:03:16Z

Sources/KIF/Additions/UIView-KIFAdditions.m

@@ -692,6 +701,7 @@ - (void)dragPointsAlongPaths:(NSArray<NSArray<NSValue *> *> *)arrayOfPaths {
                point = [self convertPoint:point fromView:self.window];
                UITouch *touch = [[UITouch alloc] initAtPoint:point inView:self];
                [touch setPhaseAndUpdateTimestamp:UITouchPhaseBegan];
+                [touch setIsFromEdge:isFromEdge];


I wonder if we should alternatively come up with a heuristic here for what constitutes an edge swipe and do it implicitly. Being explicit seems reasonable as well though.

I think with this we can make an API specifically for swiping at an edge, similarly to like tapping on a button or status bar instead of using x,y coordinatotes. As well as implicitly doing it from the edge if we can detect it being around where it should be. I think that'd be in line with other APIs of KIf

I thought about doing it implicitly like

if (startPoint.x < 1 || startPoint.x > screen.size.width - 1) { [touch setIsFromEdge:isFromEdge]; }

But this could be a breaking change for users who utilize the dragFromPoint method without intending to trigger a screen edge pan gesture.

I have tried to implement an alternative way based on the suggestions, which one do you prefer?

justinseanmartin · 2023-06-29T14:07:49Z

Sources/KIF/Additions/UITouch-KIFAdditions.m

+- (void)setIsFromEdge:(BOOL)isFromEdge
+{
+    NSInteger edgeType = isFromEdge ? 4 : 0;
+    [self _setEdgeType:edgeType];
+}


What would happen if we directly called _UITouchSetBasicTouchPropertiesFromEvent? Does Apple already have the appropriate heuristics to set this properly?

Apple derives the value of the property from the HIDEvent. We would need to synthesize a different HIDEvent for edge pan before calling _UITouchSetBasicTouchPropertiesFromEvent. This introduces additional complexity and I lack the necessary knowledge to implement it.

I wonder if we could replace these magic numbers with Apple enums.

justinseanmartin · 2023-06-29T14:10:01Z

Sources/KIF/Additions/UIView-KIFAdditions.m

@@ -630,10 +630,15 @@ - (void)dragFromPoint:(CGPoint)startPoint toPoint:(CGPoint)endPoint steps:(NSUIn
 }

 - (void)dragFromPoint:(CGPoint)startPoint displacement:(KIFDisplacement)displacement steps:(NSUInteger)stepCount;
+{
+    [self dragFromPoint:startPoint displacement:displacement steps:stepCount isFromEdge:NO];


This is kind of a breaking change, in that it would require code changes in existing tests if they had working edge gestures. As you'd called out in the issue, this has been broken since iOS 11, so seems reasonable IMO.

KIF Tests/GestureTests.m

justinseanmartin

Sorry for taking so long to get back here, and thanks for making the revisions! One other thing that we should do before landing this is add an equivalent API to KIFUIViewTestActor. That is the more flexible replacement of the KIFUITestActor API, though there is still a lot of documentation indicating to use the older API.

justinseanmartin · 2023-07-11T06:57:52Z

Sources/KIF/Classes/KIFUITestActor.m

@@ -389,6 +378,49 @@ - (void)tapScreenAtPoint:(CGPoint)screenPoint
    }];
 }

+- (void)swipeFromEdge:(UIRectEdge)edge atY:(CGFloat)y


Do you have a use case where you care about the Y coordinate used for the edge swipe? If not, I think we should start by omitting the atY: parameter and default to using 50% of the screen height.

justinseanmartin · 2023-07-11T07:00:07Z

Sources/KIF/Additions/UIView-KIFAdditions.m

+    CGFloat height = self.bounds.size.height;
+    CGFloat edgeInset = 0.5;
+    NSDictionary *edgeToPoint = @{
+        @(UIRectEdgeTop): @(CGPointMake(width / 2, 0)),


Should we also use edgeInset here for the top and height - edgeInset for the bottom?

I'v made some revisions as suggested, please review again.

1. Omit the atY: parameter of swipeFromEdge 2. Add an equivalent API to KIFUIViewTestActor and add tests 3. dragFromEdge now uses edgeInset for the top and height - edgeInset for the bottom

justinseanmartin · 2023-07-11T15:36:36Z

Thanks again for the iterations here and the contribution! I'll get this merged once CI reports green.

justinseanmartin · 2023-07-12T08:59:00Z

The tests appear to be extremely flaky right now in AccessibilityIdentifierTests.testClearingAndEnteringTextIntoViewWithAccessibilityLabel. I'm rerunning them and it's making progress on ensuring they're all green before merging.

dostrander · 2023-07-12T12:08:31Z

The tests appear to be extremely flaky right now in AccessibilityIdentifierTests.testClearingAndEnteringTextIntoViewWithAccessibilityLabel. I'm rerunning them and it's making progress on ensuring they're all green before merging.

Merged as they all went green

Add support for swiping from the specified edge of the screen

29b6746

justinseanmartin approved these changes Jun 29, 2023

View reviewed changes

Fitzmaz added 3 commits June 30, 2023 23:00

Fix Gestures scene layout

ebf5a0a

Add tests for verifying that dragFromPoint doesn't trigger

f582ccd

Add dragFromEdge

26b1a96

justinseanmartin reviewed Jul 11, 2023

View reviewed changes

Address review comments

d07657e

1. Omit the atY: parameter of swipeFromEdge 2. Add an equivalent API to KIFUIViewTestActor and add tests 3. dragFromEdge now uses edgeInset for the top and height - edgeInset for the bottom

justinseanmartin approved these changes Jul 11, 2023

View reviewed changes

dostrander merged commit 10352c9 into kif-framework:master Jul 12, 2023
5 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for swiping from the specified edge of the screen #1286

Add support for swiping from the specified edge of the screen #1286

Fitzmaz commented Jun 29, 2023

justinseanmartin left a comment

justinseanmartin Jun 29, 2023

justinseanmartin Jul 11, 2023

justinseanmartin Jun 29, 2023

dostrander Jun 30, 2023

Fitzmaz Jun 30, 2023

Fitzmaz Jun 30, 2023

justinseanmartin Jun 29, 2023

Fitzmaz Jun 30, 2023

Fitzmaz Jun 30, 2023

justinseanmartin Jun 29, 2023

justinseanmartin left a comment

justinseanmartin Jul 11, 2023

justinseanmartin Jul 11, 2023

Fitzmaz Jul 11, 2023

justinseanmartin commented Jul 11, 2023

justinseanmartin commented Jul 12, 2023

dostrander commented Jul 12, 2023

Add support for swiping from the specified edge of the screen #1286

Add support for swiping from the specified edge of the screen #1286

Conversation

Fitzmaz commented Jun 29, 2023

justinseanmartin 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

justinseanmartin 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

justinseanmartin commented Jul 11, 2023

justinseanmartin commented Jul 12, 2023

dostrander commented Jul 12, 2023