-
Couldn't load subscription status.
- Fork 20
WIP: Updated for 2.6.0-preview2 APIs #132
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks pretty good! Just minor feedback.
Since this is the first PR into the feature branch we should also cross-link back to master in README.md, and link directly to the preview docs URLs.
| self.previewView.mirror = (device.position == AVCaptureDevicePositionFront); | ||
| } | ||
| }]; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What should we do if the front camera is not available? I think there might have been one iPod Touch that we don't actually support like this...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we should assert or otherwise end the program? Or print an error, or show a UIAlertView? It seems like a case we were not expecting. See also flipCamera below.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What ever we decide to do, we also need to do in the Swift code as well. It has the same behavior as this.
Podfile
Outdated
|
|
||
| abstract_target 'TwilioVideo' do | ||
| pod 'TwilioVideo', '~> 2.5' | ||
| pod 'TwilioVideo', '~> 2.6.0-preview1' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
2.6.0-preview2
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dang it.. Copy/Paste error. lol.
I had to start with preview1 so I could at least have it have most of the header file links that CocoaPods does, and then copied the RC in manually. Nice catch.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree with @ceaglest 's comments. Lets address them and keep the PR ready to merge fore preview2 release.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, my only remaining feedback is about handling devices without a front camera (an edge case, to be sure).
|
|
||
| * [Getting Started](https://www.twilio.com/docs/video/ios-v2-getting-started) | ||
| * [Docs](https://twilio.github.io/twilio-video-ios/docs/latest/index.html) | ||
| * [Docs](https://twilio.github.io/twilio-video-ios/docs/2.6.0-preview2/index.html) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
| self.previewView.mirror = (device.position == AVCaptureDevicePositionFront); | ||
| } | ||
| }]; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we should assert or otherwise end the program? Or print an error, or show a UIAlertView? It seems like a case we were not expecting. See also flipCamera below.
No description provided.