turbolinks / turbolinks-ios

Native iOS adapter for building hybrid apps with Turbolinks 5
MIT License
881 stars 92 forks source link

Use Optional type for screenshot. #62

Closed Neilos closed 8 years ago

Neilos commented 8 years ago

Fixes #63 The problem is that if the underlying type returned by webView.snapshotViewAfterScreenUpdates(false) is something other than a UIView it blows up. Using the as? operator will safely cast, assigning nil if what is returned is not a UIView.

gjhami commented 8 years ago

I made this change myself. It resolves the error but now gives the warning

Conditional cast from 'UIView' to 'UIView' always succeeds

I also found moving the second let to its own line outside the guard let resolves the problem. guard let webView = self.webView where !isShowingScreenshot else { return } let screenshot = webView.snapshotViewAfterScreenUpdates(false)

The warning/error seem to be saying that webView.snapshotViewAfterScreenUpdates(false) can only return UIView (not nil or any other type) so moving it outside the guard statement seems to make the most sense and gives no warnings or errors. I'm a bit new to this so I could be wrong, but that's how it seems to me.

Neilos commented 8 years ago

D'oh. Silly mistake. Well spotted. The code amendment I made will always pass the guard, and, if this were acceptable, it might as well be placed on the next line, as you suggest.

I'm not sure if the return value of snapshotviewafterscreenupdates(false) can ever be anything other than a UIView. Perhaps it would be best to leave the let screenshot = ... as part of the guard statement.

With all that in mind I think what we want is a simple cast:

guard let webView = self.webView where !isShowingScreenshot, let screenshot: UIView = webView.snapshotViewAfterScreenUpdates(false) else { return }

I've amended my commit accordingly.

zachwaugh commented 8 years ago

I commented on the issue, but this is due to a change in iOS 10 where the method does return an optional UIView - https://developer.apple.com/reference/uikit/uiview/1622531-snapshotviewafterscreenupdates. When compiling with Xcode 8, this works correctly on iOS 9 and iOS 10

Neilos commented 8 years ago

Thanks. That worked.