Live Preview #39

daveluong · 2019-04-04T05:49:44Z

This PR introduces a new configurable to enable live camera view in place of the camera icon view in PhotoGalleryViewController if there is camera access permission

public protocol ImagePickerConfigurable {
....
 /// Configure to enable/disable live camera view in place of the camera icon cell
    var liveCameraViewEnabled: Bool { get }
....
}

This PR introduces a 2 way communication between custom camera view controller and PhotoGalleryViewController that allows selectedAssets to be passed between the 2, using:

new delegate method in ImagePickerControllerDelegate

 @objc optional func imagePickerController(_ picker: ImagePickerController, didFinishLaunchingCameraWith assets: [PHAsset])

new function call updateSelectedAssets(_ assets: [PHAsset]) that will replace existing assets in ImagePickerControlelr since custom camera controller can modify selected assets as well

carouselljenkins · 2019-04-04T05:56:47Z

@HungnguyenVN, can you review this pull request?

carouselljenkins · 2019-04-04T05:56:51Z

	1 Error
🚫	Please rebase to get rid of the merge commits in this PR
✅	~~Please update CHANGELOG.md.~~

Generated by 🚫 Danger

daveluong · 2019-04-04T11:02:50Z

26.66s$ make -B carthage
set -o pipefail && carthage build --no-skip-current --verbose | bundle exec xcpretty -c
A shell task (/usr/bin/xcrun xcodebuild -project /Users/travis/build/carousell/pickle/Example/Pods/Pods.xcodeproj CODE_SIGNING_REQUIRED=NO CODE_SIGN_IDENTITY= CARTHAGE=YES -list) failed with exit code 74:
xcodebuild: error: Unable to read project 'Pods.xcodeproj' from folder '/Users/travis/build/carousell/pickle/Example/Pods'.
	Reason: Project /Users/travis/build/carousell/pickle/Example/Pods/Pods.xcodeproj cannot be opened because it is missing its project.pbxproj file.

@bcylin It seems that the project directories update broke Carthage

bcylin

Carthage build fails due to the /Users/travis/build/carousell/pickle/Example/Pods/* cache. Example/Pods/* are now ignored on the master branch.

CI should pass after removing the ignored directories:
https://travis-ci.org/bcylin/pickle/builds/515880016 @ f9fc024

Pickle/Classes/PhotoGalleryLiveViewCell.swift

Pickle/Classes/ImagePickerConfigurable.swift

Pickle/Classes/PhotoGalleryLiveViewCell.swift

Pickle/Assets/Images.xcassets/camera-icon.imageset/Contents.json

Pickle.podspec

bcylin

Looks good 👍 just left some minor suggestions.

Pickle/Classes/LiveView.swift

Call to AVCaptureDeviceInput will trigger user prompt, which is not what we want here So this code need to move from intiazlier to startSession()

daveluong · 2019-04-08T03:18:17Z

hey @bcylin, thank you for taking your time to review the PR, I've made the changes requested. In addition, I've modified a few other things that need your opinion on. I have commented in the code

daveluong · 2019-04-08T03:18:36Z

Pickle/Classes/ImagePickerController.swift

@@ -122,7 +122,7 @@ open class ImagePickerController: UINavigationController {
        }
    }

-    fileprivate var selectedAssets: [PHAsset]
+    public private(set) var selectedAssets: [PHAsset]


~~Our new gallery design requires passing selected assets from photo gallery view to camera view so I need to make selectedAssets in ImagePickerViewController publicly accessible~~

Basically we need a way to pass selectedAssets between a custom camera controller and ImagePickerController, please refer to the PR's description for details. Let me know if you have a better way to do this, thanks!

daveluong · 2019-04-08T03:29:01Z

Pickle/Classes/CameraSessionHandler.swift

+    func startSession() throws {
+        guard hasPermssion else {
+            throw CameraSessionError.noPermission
+        }
        guard
            let input = AVCaptureDevice.default(for: .video),
            let deviceInput = try? AVCaptureDeviceInput(device: input) else {
                throw CameraSessionError.noDeviceInput


Accessing AVCaptureDeviceInput in the initializer will prematurely prompt user for permission, so I have moved it to startSession() instead. We only use the live view when user has already granted permission previously

bcylin

Quite an interesting camera flow!

bcylin · 2019-04-08T21:43:04Z

Pickle/Classes/ImagePickerControllerDelegate.swift

@@ -19,6 +19,9 @@ public protocol ImagePickerControllerDelegate: UINavigationControllerDelegate {
    /// Asks the delegate if the image picker should launch camera with certain permission status.
    func imagePickerController(_ picker: ImagePickerController, shouldLaunchCameraWithAuthorization status: AVAuthorizationStatus) -> Bool

+    /// Tells the delegate that picker has finished launching camera with an array of selected assets
+    @objc optional func imagePickerController(_ picker: ImagePickerController, didFinishLaunchingCameraWith assets: [PHAsset])


Should this be imagePickerControllerdidFinishLaunchingCamera(_:) since the selectedAssets is now accessible from ImagePickerController?

Oh no, I have changed it back to private since we already have this delegate

bcylin · 2019-04-08T21:45:55Z

Pickle/Classes/PhotoGalleryViewController.swift

        if traitCollection.forceTouchCapability == .available {
            registerForPreviewing(with: self, sourceView: collectionView)
        }
+        if configuration?.isLiveCameraViewEnabled == .some(true) {
+            sessionHandler = try? CameraSessionHandler()


The AVCaptureDeviceInput is moved to startSession(), maybe the initializer doesn't have to throw an error?

It's throwing .noPermission error if there is no permission, should I change it to just return nil with init?()?

bcylin · 2019-04-08T21:56:02Z

Pickle/Classes/ImagePickerController.swift

@@ -209,6 +209,10 @@ open class ImagePickerController: UINavigationController {
        showPermissionErrorIfNeeded?()
    }

+    public func updateSelectedAssets(with assets: [PHAsset]) {


It'd be good to have the documentation of this method, also worth mentioning that this method won't trigger the didSelect and didDeselectImageAsset delegate methods.

I have updated README

daveluong · 2019-04-09T05:55:31Z

Thanks @bcylin! I'll merge it and create a new release

daveluong requested a review from bcylin April 4, 2019 05:50

daveluong force-pushed the feature/live-preview branch from 801aef0 to d1deed0 Compare April 4, 2019 08:15

daveluong mentioned this pull request Apr 4, 2019

Update Project Directories #34

Merged

daveluong force-pushed the feature/live-preview branch from e7d9023 to f9ab52c Compare April 4, 2019 10:07

daveluong added 4 commits April 4, 2019 18:13

Add new camera button

87f2273

Add PhotoGalleryLiveViewCell to show live preview of camera stream

f502787

Make live preview a configurable

ed2b84e

Fix setting wrong cell type & other minor code presentation

d52ddce

daveluong force-pushed the feature/live-preview branch from f9ab52c to bcfe885 Compare April 4, 2019 10:13

Update version to 1.4.1 & add missing files

faddd5f

daveluong force-pushed the feature/live-preview branch from bcfe885 to faddd5f Compare April 4, 2019 10:27

daveluong added 2 commits April 4, 2019 18:34

Show old camera cell if there is no camera permission

7c762bd

Update PhotoGalleryViewController.swift

c16a310

bcylin reviewed Apr 4, 2019

View reviewed changes

daveluong added 8 commits April 5, 2019 10:44

Remove Example/Pods/*

23498fb

Refactor to decouple camera session handling from the cell

f485fbb

Set version to 1.4.0

88db04e

Rename property

d05ca38

Remove unused code

3f1cbfd

Remove codesign config

d0a003e

Missing scheme

5d4a624

Update CHANGELOG.md

cfd9113

bcylin approved these changes Apr 6, 2019

View reviewed changes

Pickle/Classes/LiveView.swift Show resolved Hide resolved

Pickle/Classes/LiveView.swift Outdated Show resolved Hide resolved

daveluong added 4 commits April 8, 2019 10:56

Merge branch 'master' into feature/live-preview

4682bb9

Make property private and fix typo

fd69328

Initialize session handler only if config has livePreviewEnabled = true

e1deb1e

Only access capture device when starting a session

dc7d981

Call to AVCaptureDeviceInput will trigger user prompt, which is not what we want here So this code need to move from intiazlier to startSession()

Make selectedAssets publicly accessible

929302a

daveluong commented Apr 8, 2019

View reviewed changes

Don't initialize the session handler if don't have permission

4e6e808

daveluong commented Apr 8, 2019

View reviewed changes

daveluong added 2 commits April 8, 2019 13:02

Use delegate instead of making the property publicly accessible

680c6e6

Add method to update selectedAsset from camera to photoGallery

28d2ed4

daveluong requested a review from bcylin April 8, 2019 06:22

bcylin reviewed Apr 8, 2019

View reviewed changes

Update README.md

9ae0bf8

daveluong merged commit 889000b into master Apr 9, 2019

daveluong deleted the feature/live-preview branch April 9, 2019 05:55

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Live Preview #39

Live Preview #39

daveluong commented Apr 4, 2019 •

edited

Loading

carouselljenkins commented Apr 4, 2019

carouselljenkins commented Apr 4, 2019 •

edited

Loading

daveluong commented Apr 4, 2019

bcylin left a comment

bcylin left a comment

daveluong commented Apr 8, 2019

daveluong Apr 8, 2019 •

edited

Loading

daveluong Apr 8, 2019 •

edited

Loading

bcylin left a comment

bcylin Apr 8, 2019

daveluong Apr 9, 2019

bcylin Apr 8, 2019

daveluong Apr 9, 2019

bcylin Apr 8, 2019

daveluong Apr 9, 2019

daveluong commented Apr 9, 2019

Live Preview #39

Live Preview #39

Conversation

daveluong commented Apr 4, 2019 • edited Loading

carouselljenkins commented Apr 4, 2019

carouselljenkins commented Apr 4, 2019 • edited Loading

daveluong commented Apr 4, 2019

bcylin left a comment

Choose a reason for hiding this comment

bcylin left a comment

Choose a reason for hiding this comment

daveluong commented Apr 8, 2019

daveluong Apr 8, 2019 • edited Loading

Choose a reason for hiding this comment

daveluong Apr 8, 2019 • edited Loading

Choose a reason for hiding this comment

bcylin left a comment

Choose a reason for hiding this comment

bcylin Apr 8, 2019

Choose a reason for hiding this comment

daveluong Apr 9, 2019

Choose a reason for hiding this comment

bcylin Apr 8, 2019

Choose a reason for hiding this comment

daveluong Apr 9, 2019

Choose a reason for hiding this comment

bcylin Apr 8, 2019

Choose a reason for hiding this comment

daveluong Apr 9, 2019

Choose a reason for hiding this comment

daveluong commented Apr 9, 2019

daveluong commented Apr 4, 2019 •

edited

Loading

carouselljenkins commented Apr 4, 2019 •

edited

Loading

daveluong Apr 8, 2019 •

edited

Loading

daveluong Apr 8, 2019 •

edited

Loading