New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Touch Manager Gesture Cancellation #727

Merged

joeljfischer merged 9 commits into release/5.0.0 from feature/issue_673_touch_manager_gesture_cancellation

Sep 12, 2017

Contributor

NicoleYarroch commented Sep 8, 2017

Fixes #673

This PR is ready for review.

Risk

This PR makes minor API changes.

Testing Plan

Test cases were added to the SDLTouchManagerSpec class to test canceling tap, pan, and pinch gestures.

Summary

Adds support for detecting a SDLTouchType of type CANCEL. The SDLTouchManager handles parsing the SDLOnTouchType notification from Core and issues a cancel delegate callback for pinch and pan gestures. If a single or double tap gesture is canceled, the tap is simply canceled.

Changelog

Enhancements

Added support for detecting a SDLTouchType of type CANCEL in the SDLTouchManager class.
Added two new optional methods to the SDLTouchManagerDelegate:
- touchManager:manager pinchCanceledAtCenterPoint:point
- touchManager:manager panningCanceledAtPoint:point.

Bug Fixes

Added SDLRectangle.h to both of the .podspec files.

CLA

I have signed the CLA

NicoleYarroch added 7 commits

September 6, 2017 13:24


          Added SDLRectangle.h to .podspec files

edf6921

Signed-off-by: NicoleYarroch <nicole@livio.io>


          Added logic for sdl_handleTouchCanceled

Signed-off-by: NicoleYarroch <nicole@livio.io>


          Add optional protocol delegates

ddce172

- Added test cases for onTouchEvent CANCEL

Signed-off-by: NicoleYarroch <nicole@livio.io>


          Cancel onTouchEvent test cases written for taps and pan

917a9c5

- Test cases written for single tap, double tap, and pan gesture cancellation

Signed-off-by: NicoleYarroch <nicole@livio.io>


          Refactored the SDLTouchManager unit tests

e69d6be

- got rid of unnecessary repeated code

Signed-off-by: NicoleYarroch <nicole@livio.io>


          test cases for tap, pinch, and pan gesturest completed

960831b

Signed-off-by: NicoleYarroch <nicole@livio.io>


          Updated documentation

4d34109

Signed-off-by: NicoleYarroch <nicole@livio.io>

NicoleYarroch requested a review from joeljfischer

September 8, 2017 18:56

NicoleYarroch self-assigned this

NicoleYarroch added the enhancement label

NicoleYarroch added this to the 5.0.0 milestone

joeljfischer changed the base branch from master to release/5.0.0

September 8, 2017 19:36

joeljfischer changed the title ~~Issue 673 Touch Manager Gesture Cancellation~~ Touch Manager Gesture Cancellation

joeljfischer added the proposal label

joeljfischer suggested changes

View reviewed changes

SmartDeviceLink/SDLTouchManager.m Outdated

@@ @@ -141,39 +153,42 @@ - (void)sdl_handleTouchBegan:(SDLTouch *)touch { @@
                   _performingTouchType = SDLPerformingTouchTypeSingleTouch;
                   switch (touch.identifier) {
-                      case SDLTouchIdentifierFirstFinger:
+                          case SDLTouchIdentifierFirstFinger:

Contributor

joeljfischer Sep 12, 2017

This indentation for the whole switch is incorrect, should look like:

switch (touch.identifier) {
    case SDLTouchIdentifierFirstFinger: {
        self.previousTouch = touch;
    } break;
    case SDLTouchIdentifierSecondFinger: {
        _performingTouchType = SDLPerformingTouchTypeMultiTouch;
        self.currentPinchGesture = [[SDLPinchGesture alloc] initWithFirstTouch:self.previousTouch secondTouch:touch];
        self.previousPinchDistance = self.currentPinchGesture.distance;
        if ([self.touchEventDelegate respondsToSelector:@selector(touchManager:pinchDidStartAtCenterPoint:)]) {
            [self.touchEventDelegate touchManager:self pinchDidStartAtCenterPoint:self.currentPinchGesture.center];
        }
    } break;
}

Contributor Author

NicoleYarroch Sep 12, 2017

Fixed

SmartDeviceLink/SDLTouchManager.m Outdated

               - (void)sdl_handleTouchMoved:(SDLTouch *)touch {
                   if ((touch.timeStamp - self.previousTouch.timeStamp) <= (self.movementTimeThreshold * NSEC_PER_USEC) || !self.isTouchEnabled) {
                       return; // no-op
                   }
                   switch (self.performingTouchType) {
-                      case SDLPerformingTouchTypeMultiTouch:
+                          case SDLPerformingTouchTypeMultiTouch:

Contributor

joeljfischer Sep 12, 2017

Also indented incorrectly:

switch (self.performingTouchType) {
    case SDLPerformingTouchTypeMultiTouch: {
        switch (touch.identifier) {
            case SDLTouchIdentifierFirstFinger: {
                self.currentPinchGesture.firstTouch = touch;
            } break;
            case SDLTouchIdentifierSecondFinger: {
                self.currentPinchGesture.secondTouch = touch;
            } break;
        }

        if ([self.touchEventDelegate respondsToSelector:@selector(touchManager:didReceivePinchAtCenterPoint:withScale:)]) {
            CGFloat scale = self.currentPinchGesture.distance / self.previousPinchDistance;
            [self.touchEventDelegate touchManager:self
                     didReceivePinchAtCenterPoint:self.currentPinchGesture.center
                                        withScale:scale];
        }

        self.previousPinchDistance = self.currentPinchGesture.distance;
    } break;
    case SDLPerformingTouchTypeSingleTouch: {
        _performingTouchType = SDLPerformingTouchTypePanningTouch;
        if ([self.touchEventDelegate respondsToSelector:@selector(touchManager:panningDidStartAtPoint:)]) {
            [self.touchEventDelegate touchManager:self
                           panningDidStartAtPoint:touch.location];
        }
    } break;
    case SDLPerformingTouchTypePanningTouch: {
        if ([self.touchEventDelegate respondsToSelector:@selector(touchManager:didReceivePanningFromPoint:toPoint:)]) {
            [self.touchEventDelegate touchManager:self
                       didReceivePanningFromPoint:self.previousTouch.location
                                          toPoint:touch.location];
        }
    } break;

    case SDLPerformingTouchTypeNone: break;
}

Contributor Author

NicoleYarroch Sep 12, 2017

Fixed

SmartDeviceLink/SDLTouchManager.m Outdated

-                                  break;
-                          }
+                          case SDLPerformingTouchTypeMultiTouch:

Contributor

joeljfischer Sep 12, 2017

Also indented incorrectly:

switch (self.performingTouchType) {
    case SDLPerformingTouchTypeMultiTouch: {
        [self sdl_setMultiTouchFingerTouchForTouch:touch];
        if (self.currentPinchGesture.isValid) {
            if ([self.touchEventDelegate respondsToSelector:@selector(touchManager:pinchDidEndAtCenterPoint:)]) {
                [self.touchEventDelegate touchManager:self pinchDidEndAtCenterPoint:self.currentPinchGesture.center];
            }
            self.currentPinchGesture = nil;
        }
    } break;
    case SDLPerformingTouchTypePanningTouch: {
        if ([self.touchEventDelegate respondsToSelector:@selector(touchManager:panningDidEndAtPoint:)]) {
            [self.touchEventDelegate touchManager:self panningDidEndAtPoint:touch.location];
        }
    } break;
    case SDLPerformingTouchTypeSingleTouch: {
        if (self.singleTapTimer == nil) {
            // Initial Tap
            self.singleTapTouch = touch;
            [self sdl_initializeSingleTapTimerAtPoint:self.singleTapTouch.location];
        } else {
            // Double Tap
            [self sdl_cancelSingleTapTimer];

            NSUInteger timeStampDelta = touch.timeStamp - self.singleTapTouch.timeStamp;
            CGFloat xDelta = fabs(touch.location.x - self.singleTapTouch.location.x);
            CGFloat yDelta = fabs(touch.location.y - self.singleTapTouch.location.y);

            if (timeStampDelta <= self.tapTimeThreshold * NSEC_PER_USEC && xDelta <= self.tapDistanceThreshold && yDelta <= self.tapDistanceThreshold) {
                CGPoint centerPoint = CGPointCenterOfPoints(touch.location,
                                                            self.singleTapTouch.location);
                if ([self.touchEventDelegate respondsToSelector:@selector(touchManager:didReceiveDoubleTapAtPoint:)]) {
                    [self.touchEventDelegate touchManager:self
                               didReceiveDoubleTapAtPoint:centerPoint];
                }
            }

            self.singleTapTouch = nil;
        }
    } break;

    case SDLPerformingTouchTypeNone: break;
}

Contributor Author

NicoleYarroch Sep 12, 2017

Fixed

SmartDeviceLink/SDLTouchManager.m Outdated

+                  }
+                  switch (self.performingTouchType) {
+                          case SDLPerformingTouchTypeMultiTouch:

Contributor

joeljfischer Sep 12, 2017

Also indented incorrectly:

switch (self.performingTouchType) {
    case SDLPerformingTouchTypeMultiTouch: {
        [self sdl_setMultiTouchFingerTouchForTouch:touch];
        if (self.currentPinchGesture.isValid) {
            if ([self.touchEventDelegate respondsToSelector:@selector(touchManager:pinchCanceledAtCenterPoint:)]) {
                [self.touchEventDelegate touchManager:self pinchCanceledAtCenterPoint:self.currentPinchGesture.center];
            }
            self.currentPinchGesture = nil;
        }
    } break;
    case SDLPerformingTouchTypePanningTouch: {
        if ([self.touchEventDelegate respondsToSelector:@selector(touchManager:panningCanceledAtPoint:)]) {
            [self.touchEventDelegate touchManager:self panningCanceledAtPoint:touch.location];
        }
    } break;
    case SDLPerformingTouchTypeSingleTouch: // fallthrough
    case SDLPerformingTouchTypeNone: break;
}

Contributor Author

NicoleYarroch Sep 12, 2017

Fixed

SmartDeviceLink/SDLTouchManager.m Outdated

+               *  @param touch    Gesture information
+               */
+              - (void)sdl_handleTouchCanceled:(SDLTouch *)touch {
+                  if (!self.isTouchEnabled) {

Contributor

joeljfischer Sep 12, 2017

This already exists in the calling method and should not be repeated here or in the other sub-handlers.

Contributor Author

NicoleYarroch Sep 12, 2017

Fixed. Removed from all sub-handlers

SmartDeviceLink/SDLTouchManager.m Outdated

+                          break;
+                          case SDLPerformingTouchTypeNone:
+                          case SDLPerformingTouchTypeSingleTouch:

Contributor

joeljfischer Sep 12, 2017

Reverse this and the above case, they should be in the same order as other sub-handlers (this is done in the code provided in the above comment)

Contributor Author

NicoleYarroch Sep 12, 2017

Fixed

SmartDeviceLink/SDLTouchManager.m Outdated

+                  switch (touch.identifier) {
+                      case SDLTouchIdentifierFirstFinger:
+                          self.currentPinchGesture.firstTouch = touch;
+                          break;

Contributor

joeljfischer Sep 12, 2017

Add braces as in above cases

Contributor Author

NicoleYarroch Sep 12, 2017

Fixed

NicoleYarroch added 2 commits

September 12, 2017 11:28


          Merge branch 'release/5.0.0' into feature/issue_673_touch_manager_ges…

e8c6121

…ture_cancellation


          Code refactor

1d5aba9

- fixed braces on switch statements
- removed unnecessary checking for isTouchEnabled
- added documentation to private method

Signed-off-by: NicoleYarroch <nicole@livio.io>

joeljfischer approved these changes

View reviewed changes

joeljfischer merged commit 10ce67a into release/5.0.0

joeljfischer deleted the feature/issue_673_touch_manager_gesture_cancellation branch

September 12, 2017 19:29

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels