Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

iCloud Support #79

Closed
diegof29 opened this issue Dec 29, 2015 · 11 comments
Closed

iCloud Support #79

diegof29 opened this issue Dec 29, 2015 · 11 comments

Comments

@diegof29
Copy link

The photo selection doesn't work if the user picks a photo from iCloud.

@zenangst
Copy link
Contributor

Hey @diegof29, thanks for bringing this to our attentions, we will do some digging to see what we come up with.

What happens when try to select something from iCloud? Do you get any warnings or errors in the logs?

@diegof29
Copy link
Author

Hi @zenangst!

I did a quick debug and I found that in ImagePicker/ImageGalleryView.swift in collectionView:didSelectItemAtIndexPath the block is called more than one time if I select an iCloud image and that results on a selection and a quick deselection of the image.

@zenangst
Copy link
Contributor

zenangst commented Feb 7, 2016

@diegof29 still haven't had the time to look into this properly I'm afraid. If you do solve it before we do, feel free to make a pull request.

@Kalzem
Copy link

Kalzem commented May 3, 2016

+1 for support

@aitornillooo
Copy link

app crash if I pick a picture from iCloud. Does this library have support for getting iCloud pictures?

@weakfl
Copy link
Contributor

weakfl commented Sep 1, 2016

+1

@onmyway133
Copy link
Contributor

onmyway133 commented Sep 1, 2016

@aitornillooo @weakfl Hi, what crash log is it?

@weakfl
Copy link
Contributor

weakfl commented Sep 1, 2016

@onmyway133 Sorry if my answer was misleading. The app doesn't crash, but images get immediately deselected as @diegof29 has described.

@weakfl
Copy link
Contributor

weakfl commented Sep 21, 2016

I did a little debugging and it turns out the AssetManager indeed calls the completion block in resolveAsset() multiple times.
The reason is the default PHImageRequestOptionsDeliveryMode.opportunistic:

If the isSynchronous property is false, Photos may call the resultHandler block (that you specified in the requestImage(for:targetSize:contentMode:options:resultHandler:) method more than once.

It turns out that the completion block is executed 3 times if the image is on the device, so the image ends up selected by favourable circumstances ;)
I guess the completion block is executed twice if the image is in the cloud, which results in immediate deselection of the image.

What's the reason you're resolving assets on didSelectItemAtIndexPath in ImageGalleryView anyway?

Unfortunately I couldn't really test with iCloud images because for some reason they're not included in the gallery view on iOS 10. I don't know if PHAsset.fetchAssetsWithMediaType() doesn't return iCloud images on iOS 10 or if it's just because I have just set up my iPhone 7. I'll have to do some more digging...

EDIT:
It turned out that iCloud images are still included, but the order is reversed. Looks like the order gets messed up when restoring from an iCloud backup.

I would suggest to add an NSSortDescriptor to the fetch() function in AssetManager and order by creation date:

public static func fetch(completion: (assets: [PHAsset]) -> Void) {
    let fetchOptions = PHFetchOptions()
    fetchOptions.sortDescriptors = [NSSortDescriptor(key:"creationDate", ascending: true)]
    let authorizationStatus = PHPhotoLibrary.authorizationStatus()
    var fetchResult: PHFetchResult?

    guard authorizationStatus == .Authorized else { return }

    if fetchResult == nil {
      fetchResult = PHAsset.fetchAssetsWithMediaType(.Image, options: fetchOptions)
    }

    if fetchResult?.count > 0 {
      var assets = [PHAsset]()
      fetchResult?.enumerateObjectsUsingBlock { object, index, stop in
        if let asset = object as? PHAsset {
          assets.insert(asset, atIndex: 0)
        }
      }

      dispatch_async(dispatch_get_main_queue(), {
        completion(assets: assets)
      })
    }
  }

I've also removed the resolveAsset()call in didSelectItemAtIndexPath in ImageGalleryView to avoid multiple calls of the completion block:

public func collectionView(collectionView: UICollectionView, didSelectItemAtIndexPath indexPath: NSIndexPath) {
  guard let cell = collectionView.cellForItemAtIndexPath(indexPath)
    as? ImageGalleryViewCell else { return }

  let asset = assets[indexPath.row]

  if cell.selectedImageView.image != nil {
    UIView.animateWithDuration(0.2, animations: {
      cell.selectedImageView.transform = CGAffineTransformMakeScale(0.1, 0.1)
      }) { _ in
        cell.selectedImageView.image = nil
    }
    self.selectedStack.dropAsset(asset)
  } else if self.imageLimit == 0 || self.imageLimit > self.selectedStack.assets.count {
    cell.selectedImageView.image = AssetManager.getImage("selectedImageGallery")
    cell.selectedImageView.transform = CGAffineTransformMakeScale(0, 0)
    UIView.animateWithDuration(0.2) { _ in
      cell.selectedImageView.transform = CGAffineTransformIdentity
    }
    self.selectedStack.pushAsset(asset)
  }
}

Images from the iCloud photo library do work with these changes, but imho this is only a partial solution.
It might take some time to load images from the cloud when resolving assets on doneButtonDidPress(), so some kind of progress/activity indicator might be a good idea.

@weakfl
Copy link
Contributor

weakfl commented Sep 22, 2016

@zenangst @onmyway133 Take a look at my previous comment for a partial solution.

@onmyway133
Copy link
Contributor

@weakfl Hi, sorry for late reply

About deliveryMode, it is solved in #207
About iCloud photos in gallery view, it is strange that it appear there, but anyway, I think this #262 can solve it
Feel free to make any PR to improve this 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants