search

carousell / pickle

Carousell flavoured image picker with multiple photo selections.
https://carousell.github.com/pickle
Apache License 2.0
60 stars 25 forks source link

Live Preview #39

Closed daveluong closed 5 years ago

daveluong commented 5 years ago
  1. 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 }
....
}
  1. This PR introduces a 2 way communication between custom camera view controller and PhotoGalleryViewController that allows selectedAssets to be passed between the 2, using:

IMG_0042

carouselljenkins commented 5 years ago

@hungnguyenvn, can you review this pull request?

carouselljenkins commented 5 years ago
1 Error
:no_entry_sign: Please rebase to get rid of the merge commits in this PR
:white_check_mark: Please update CHANGELOG.md.

Generated by :no_entry_sign: Danger

daveluong commented 5 years ago 
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

daveluong commented 5 years ago

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 commented 5 years ago

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