[webkit-reviews] review granted: [Bug 186821] [Fullscreen] Add a pinch-to-exit gesture : [Attachment 343248] Patch

[bugzilla-daemon at webkit.org]

Tim Horton <thorton at apple.com> has granted Jer Noble <jer.noble at apple.com>'s
request for review:
Bug 186821: [Fullscreen] Add a pinch-to-exit gesture
https://bugs.webkit.org/show_bug.cgi?id=186821

Attachment 343248: Patch

https://bugs.webkit.org/attachment.cgi?id=343248&action=review




--- Comment #8 from Tim Horton <thorton at apple.com> ---
Comment on attachment 343248
  --> https://bugs.webkit.org/attachment.cgi?id=343248
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=343248&action=review

> Source/WebKit/UIProcess/ios/fullscreen/WKFullScreenViewController.h:43
> + at property (assign, nonatomic) BOOL animating;

should the getter be ‘isAnimating’? :P

> Source/WebKit/UIProcess/ios/fullscreen/WKFullScreenWindowControllerIOS.mm:787
> +	       anchor = [_interactivePanDismissGestureRecognizer
locationInView:_fullscreenViewController.get().view];
> +	   else if (_interactivePinchDismissGestureRecognizer.get().state ==
UIGestureRecognizerStateBegan)
> +	       anchor = [_interactivePanDismissGestureRecognizer
locationInView:_fullscreenViewController.get().view];

You seem to be repeating yourself. Maybe these cases should be ||’d?

> Source/WebKit/UIProcess/ios/fullscreen/WKFullScreenWindowControllerIOS.mm:984
> +	   if (progress > 0.25 || (progress > 0 && velocity > 5))

Is this what other people usually use? I feel like I’ve seen fancier things.