Page MenuHomePhabricator

[native] Remove `sortBy` property from `fetchPhotos`
ClosedPublic

Authored by abosh on Aug 15 2022, 1:00 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sat, Jun 15, 7:58 AM
Unknown Object (File)
Fri, Jun 14, 10:52 AM
Unknown Object (File)
Fri, Jun 14, 1:43 AM
Unknown Object (File)
Wed, Jun 12, 3:28 PM
Unknown Object (File)
May 14 2024, 2:21 PM
Unknown Object (File)
Apr 20 2024, 2:48 PM
Unknown Object (File)
Apr 20 2024, 2:48 PM
Unknown Object (File)
Apr 20 2024, 2:47 PM
Subscribers

Details

Summary

Relevant Linear issue here. I left detailed research for this diff in my comment there. This diff sorts out (no pun intended) issues with MediaGalleryKeyboard displaying images in a weird order and instead mirrors the "default sorting provided by the platform" (from the Expo docs).

In summary, photos are now displayed in the same order as on a user's Camera Roll.

Test Plan

Tested on physical iOS device and Android simulator and works as expected.

One notable thing I tested was setting the creationTime of a photo to the future and making sure that it wasn't pushed to the front of the photos displayed on MediaGalleryKeyboard. This was the main bug with using creationTime.

Photo set to the future:

image.png (1×828 px, 1 MB)

What Camera Roll looks like (photo from the future is still behind last photo, which is from today):
image.png (1×828 px, 1 MB)

What MediaGalleryKeyboard displays (photo from the future still stays behind last photo in Camera Roll, mirroring the ordering of photos in Camera Roll):
image.png (1×828 px, 817 KB)

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

abosh edited the summary of this revision. (Show Details)

Makes sense!

Does this mean the MediaGallery component could look different on iOS/Android for the "same" photo gallery (images, time created, time modified, etc) depending on how they order things? I guess that's fine because it'll match whatever pattern the OS uses throughout (assuming they aren't just exactly the same).

This revision is now accepted and ready to land.Aug 16 2022, 7:25 AM

@abosh you should always respond to questions before landing, even if the diff is accepted

Sorry! My bad, in the rush of trying to get the issue marked as done before the dev sync I forgot to respond. Definitely won’t do that again. I know the importance of “accepting with comments” so I should have been more diligent.

In response to your comment, @atul, yes I suppose the components could look different. But when I tested this on the Android Emulator, it ordered them in the same way as the Android Photos app did. I figure being symmetric with the user’s platform (Android or iOS) is better than trying to make the gallery look the same across all platforms.

But from what I can tell Android and iOS order photos the same way, so it shouldn’t be a big deal.