1and2papa / CTAssetsPickerController

iOS control that allows picking multiple photos and videos from user's photo library.
MIT License
2.15k stars 550 forks source link

Fixed assets not being displayed #137

Closed ugort closed 9 years ago

ugort commented 9 years ago

Do not check assets count in CTAssetsPickerController but directly call showAssetCollectionViewController:

This is an issue for example when the camera roll is empty put the user manually adds images/albums via iTunes.

1and2papa commented 9 years ago

Hi @ugort,

Thanks for your PR. Regarding to point 1, might I know if you test the picker in iOS 9? As I just found the following note in Apple's pre-release documentation of fetchAssetsWithOptions method:

By default, fetch results do not include photos synced to the device through iTunes or stored in iCloud Shared Albums. To change this behavior, use the includeAssetSourceTypes property in the options parameter.

I think that's why you get different results of checkAssetsCount.

Re the point 2, I prefer to keep additional checking in CTAssetsPickerController so that it can show the "No Photos" screen instead of a list of empty album if the whole library is empty.

ugort commented 9 years ago

Hi @chiunam

no, we were testing on iOS8. I think that the problem is that two different approaches are used to check if there are assets available:

PHFetchResult *fetchResult = [PHAsset fetchAssetsWithOptions:self.assetsFetchOptions];

vs the code in:

[CTAssetCollectionViewController setupFetchResults] 

Point 2: CTAssetCollectionViewController displays the same "No Photos" view as checkAssetsCount when there are no assets. See the check in reloadData.

1and2papa commented 9 years ago

Hi @ugort,

I know what you have confused with. It is intended to use two different approaches for checking as they are actually doing two different tasks:

PHFetchResult *fetchResult = [PHAsset fetchAssetsWithOptions:self.assetsFetchOptions];

tries to find the number of photos matching assetsFetchOptions. By default, the assetsFetchOptions is nil so it tries to find the number of all photos. If there are no photos, it shows the "No Photos" view. Otherwise it shows the list of photo albums by showing the CTAssetCollectionViewController.

[CTAssetCollectionViewController setupFetchResults] 

actually tries to find the number of photo albums defined in assetCollectionSubtypes. By default, the assetCollectionSubtypes includes all photos albums. It only shows the "No Photos" view if there are no photos albums found. For example, the developer wants to show the "Timelapses" album only but the user does not have the "Timelapses" album in the device.

To conclude, it is perfectly fine to keep both checking as it is necessary in my opinion.


However, I agree that using fetchAssetsWithOptions for checking number of photos might not be a good choice. The result of fetchAssetsWithOptions is different from that of fetchAssetsInAssetCollection (which is mainly used for retrieving assets in this picker). It does not include photos synced to the device through iTunes or stored in iCloud Shared Albums. It is the root cause of your initial issue.

Rewriting the method checkAssetsCount is the way to get proper number of photos but it requires traversing all the photos albums. Not sure if it drags the speed of the picker or not.

Please share your thought.

ugort commented 9 years ago

Hi @chiunam

thanks for your thorough analysis and sorry for my late response.

You are right. I noticed in the API documentation of PHAsset:

By default, fetch results do not include photos synced to the device through iTunes or stored in iCloud Shared Albums. 
To change this behavior, use the includeAssetSourceTypes property in the options parameter.

To solve my initial issue I just need to set that option properly. In this case Apples API is misleading since in one case the default options returns all photos the other case not. Maybe it would be helpful to make a hint for that in the documentation of `CTAssetsPickerController.h´

Regarding the best method to get the number of photos, I don't know the framework well enough. But it might at least in the code I modified in this PR the actual number is irrelevant since you only want to know if there are photos.

I will drop this PR.