olofd/react-native-photos-framework

NSSortDescriptor displays unexpected assets when sortDescriptors are sorted by creationDate or modificationDate

Closed this issue · 8 comments

I'm trying to list assets in my project in the same way the native photo app displays assets. (Most recent assets first including assets that have been modified)

When I sort assets by creationDate, edited photos do not appear at the top of the list. For example, a photo edited and exported using Adobe Lightroom for iOS appears next to the original photo instead at the top of the list since the createdDate remains the same. I would expect this newly created edited photo to be displayed at the top of my list, but it is still listed next to the original photo since the creationDate property did not change.

I've tried sorting by modificationDate, but this also leaves me with unexpected results since activities like favoriting or un-favoriting a photo changes the modificationDate and moves that asset to the top of the list, which doesn't happen in the native Photos app and is an unexpected behavior for sorting photos.

Just curious to know if you were aware of this issue and if you can think of a fix. This stackoverflow question discusses this issue and provides a possible solution by removing the sortDescriptors to achieve the same sorting as the native Photos app. I tried implementing this solution, which sort of worked, but the assets were sorted oldest to newest instead of my desired functionality of newest to oldest. Can you think of a way to implement this functinoality?

Cheers!

http://stackoverflow.com/questions/31696701/sorting-photos-exactly-like-native-photoapp-using-photokit

olofd commented

Hmm.. interesting. So how would you solve it? Would this work:

getAssets({ fetchOptions : { sortDescritors : [{ key : 'asPhotos', ascending : false }] } });

This special sort-descriptor, if ascending false (newest to oldest), we reverse the startIndex and endIndex of the users input. To basically load the assets in reverse. Would that work?
If the user says asPhotos, ascending: true, we simply ignore that sort descriptor, because this is like submitting a query without sort descriptors??

Quite early here, so bear with me if I'm overlooking something.

(And asPhotos is just an example name, might be original.
It should possibly not be a sortDescrriptor at all, maybe just an option to load in reverse?)

olofd commented

@justinsherwood Did a fast implementation.. but still don't know if this solves your issue.
Take a look: #11

I don't know if you use the change-tracking capabilities of this library? But I will need to update that as well to account for the reversed assets in this case.. easy however.

I like the idea of a special sortDescriptor within fetchOptions. That way this custom "native" sort can be used when using the getAssets method or retrieving previewAssets.

I would suggest naming this sortDescriptor "native" since I think "asPhotos" might be confusing since a developer could assume only photos will be returned. I like the name "native" as opposed to "original" as well since a developer might not know what the original sort would be. In my mind "native" assumes the default sort used by the OS.

Also, I tried reversing the user input of the startIndex and endIndex, but I received an empty result. Perhaps there is another way to achieve that reverse functionality.. I think you're on the right track. I'll be thinking.

Commented before seeing your latest comment. I'll test PR #11 an let you know my results.

olofd commented

Yes. saw that there was a bug in my code. Will fix it later tonight.

olofd commented

But you get the gist of it.

olofd commented

And: It's a bit confusing having this option as a sortDescriptor.. it's really a revere enumeration. The reason it's confusing is because it's applied after the sortDescriptors has been applied. It will therefore give unexpected behaviour when being combined with any other sortDescriptor. sortDescriptors is an array and the results will be sorted according the the order of that array.. this does not fit into that model.

This is if we implement it as my PR #11 suggests.. might be totally wrong.

olofd commented

@justinsherwood This just went into master. I'm expecting a bit of follow-up-work on this. but feel free to try out 0.0.63. I've also added a lot of tests associated with this feature, mostly around change-tracking. Trying to get a travis CI-env. setup for this right now.

I'm closing this for now. Open new issue if you find any bugs.