Exploratory pr for native image picker #509

Chrischuck · 2023-02-26T17:33:20Z

#431

Get native photo picker working on Android
Get native photo picker working on iOS
Ensure native photo picker is used for importing new photos to Group Photos and selecting photos for an existing observation (i.e. on Obs Edit)
Ensure native photo picker allows selection of multiple photos
Check for compatibility issues on older devices. (For example, I believe iOS introduced a new Photos Picker that only works on iOS 15+, so we may need a different solution for phones with an older OS)
Make sure we can still get coordinates from exif data

android

android-test.mov

ios 14.5

old-ios.mov

ios

ios-test.mov

albullington

This is looking pretty good to me. Main thing is that we need to make sure we're getting exif data natively, rather than through the image picker's JS code.

If we can make the navigation feel a little smoother when closing the modal, that would also be a nice improvement.

albullington · 2023-03-07T00:46:29Z

src/sharedHelpers/selectImagesFromGallery.js

+import { Platform } from "react-native";
+import ImagePicker from "react-native-image-crop-picker";
+
+const handleIosImage = image => {


Instead of getting exif data from JavaScript, can we use our native exif reader code?

This exif data should be more reliable and easy to use (per this issue: #144)

Hmm, could explain more on the reliable/easy part?

react-native-image-crop-picker pulls the exif data natively, but there is no normalization layer, so the shape/attributes for ios/android simply come back differently. This js code just normalizes data so the app can use it effectively.

I ask this because native-exif-reader isn't able to use the files from react-native-image-crop-picker, not exactly sure why

@kueda, any thoughts on this? I know Yaron spent a while making sure we can get exif data natively, but I'm not 100% certain why we need that or if that's a dealbreaker here for using react-native-image-crop-picker.

@budowski did this work as a part of #144. I believe we wrote our own EXIF library because we were unable to consistently extract GPSHPositioningError using existing react native libraries. @Chrischuck, this image should have that tag. Are you seeing it getting extracted with your code in this branch on both iOS and Android?

@kueda looks like it isn't able to get that tag. If I recall, react-native-image-picker worked fine w/ exif reader but the resizer didn't like it. Might be easier to go that route and just find a different resizer library

albullington · 2023-03-07T00:50:06Z

src/components/PhotoImporter/PhotoGallery.js

@@ -185,6 +185,7 @@ const PhotoGallery = ( ): Node => {
      navToObsEdit( );
      return;
    }
+
    navigation.navigate( "GroupPhotos", { selectedPhotos } );


not sure if this is an issue specific to this PR, but I noticed that the number of observations never updates on the GroupPhotos screen when combining, separating, or deleting photos. And number of photos is also incorrect.

Yes I noticed that too when I first worked on this. I wasn't exactly sure and didnt look into how they were calculated

albullington · 2023-03-07T00:54:42Z

src/providers/ObsEditProvider.js

  ), [] );

  const createObservationFromGalleryPhoto = useCallback( async photo => {
-    const originalPhotoUri = photo?.image?.uri;


this code should not be removed -- we want to get exif data using these methods.

albullington · 2023-03-07T00:58:08Z

src/components/AddObsModal.js

@@ -32,7 +33,17 @@ const AddObsModal = ( { closeModal }: Props ): React.Node => {
    closeModal( );
  };

-  const navToPhotoGallery = ( ) => navAndCloseModal( "PhotoGallery" );
+  const navToPhotoGallery = async () => {


Is there a way to make the navigation experience smoother after closing the image picker? It seems a little glitchy to me that the modal closes and the user sees MyObservations with the AddObsModal still pulled up before navigating to the next screen.

Not really sure on this one unless there's an intermediary page that's behind the image picker when it's up.

Also if AddObsModal, the call to selectImagesFromGallery gets destroyed.

currently in inat ios, the behavior is similar but when the image picker closes, there is a loading state

I agree this isn't ideal, but I think it's fine for now. We could have some empty intermediate as Chris suggested, but I think this is more a question for Abhas when this is ready for him to look at.

Chrischuck · 2023-03-16T21:19:32Z

@kueda @albullington

I gave react-native-image-picker a go and that did not work with the exif-reader. What I think is happening is react-native-image-picker creates a tmp file copy and loses the exif data. Unfortunately, it doesn't give you direct access to the original image. react-native-camera-roll on the other hand, points you to the original file.

I tried expo-image-picker which is able to get the exif data on ios for the ceanothus? moth. However it does not work on android. So far all of the image pickers that use native make a copy of the asset and lose exif data for our own reader.

I think if this pr goes in, react-native-image-picker would be viable.

kueda

One minor requested fix, but the bigger issue is lack of access to the original so we can use our own exif parser and get at GPSHPositioningError. @Chrischuck do you want to using https://github.com/IceOfSummer/react-native-image-picker as instead of the main repo and seeing if you can get https://github.com/inaturalist/react-native-exif-reader working with the origina file path from that? If that works and that PR never gets merged, maybe it's worth maintaining our own fork.

kueda · 2023-04-04T21:46:44Z

src/sharedHelpers/selectImagesFromGallery.js

+    pickerParams.compressImageMaxHeight = 2048;
+  }
+
+  const selectedImages = await ImagePicker.openPicker( pickerParams );


I think this raises an exception that we need to handle if the user backs out of the image picker in Android.

kueda · 2023-04-04T21:51:23Z

src/components/AddObsModal.js

@@ -32,7 +33,17 @@ const AddObsModal = ( { closeModal }: Props ): React.Node => {
    closeModal( );
  };

-  const navToPhotoGallery = ( ) => navAndCloseModal( "PhotoGallery" );
+  const navToPhotoGallery = async () => {


I agree this isn't ideal, but I think it's fine for now. We could have some empty intermediate as Chris suggested, but I think this is more a question for Abhas when this is ready for him to look at.

Chrischuck · 2023-04-06T18:29:40Z

@kueda I gave another forked repo a try that was supposed to give the original filepath, I forgot which one... But anyway I can give the one IceOfSummer fixed a shot.

I'm also thinking it might be better to fork one of those repos and maintain under inaturalist, unfortunately, all of the maintenance on the photo pickers seems dead.

kueda · 2023-04-07T23:35:51Z

@kueda I gave another forked repo a try that was supposed to give the original filepath, I forgot which one... But anyway I can give the one IceOfSummer fixed a shot.

Cool.

I'm also thinking it might be better to fork one of those repos and maintain under inaturalist, unfortunately, all of the maintenance on the photo pickers seems dead.

Maybe. https://github.com/react-native-image-picker/react-native-image-picker seems somewhat alive, though they don't seem to be reviewing PRs. If you can get that IceOfSummer fork to work with our exif parser, I think that's probably worth maintaining a fork for.

Chrischuck · 2023-04-08T01:41:54Z

@kueda Will probably open up another pr for this. But I'll get to it saturday or sunday

Chrischuck · 2023-04-08T16:46:22Z

@kueda unfortunately, IceOfSummer's fix only works for android. I tried to updating the code to get the original file path for ios, but I don't know enough on that side to get anything going...

I ended up having to fork IceOfSummer's repo and publishing to npm. Trying to install directly from github got some build issues because of TS.

kueda · 2023-05-23T22:46:07Z

I'm going to close this b/c @budowski is working on a custom fork of that package and will do the work in this repo in a separate branch. Definitely appreciate the work here, though, @Chrischuck.

Chrischuck mentioned this pull request Feb 26, 2023

Import photos using native photo chooser #431

Closed

6 tasks

Chrischuck marked this pull request as ready for review March 3, 2023 04:53

albullington requested changes Mar 7, 2023

View reviewed changes

Chrischuck force-pushed the 431-spike-native-image-picker branch 3 times, most recently from 3983591 to e2b4322 Compare March 15, 2023 23:51

Chrischuck added 7 commits March 30, 2023 13:31

Exploratory pr for native image picker

dfe086a

test

9424cf9

Use native image picker

9973c1e

Remove log

d3c834f

Works for android

858ef27

Works for android

37a1718

Fix tests

29420ad

Chrischuck force-pushed the 431-spike-native-image-picker branch from e2b4322 to 29420ad Compare March 30, 2023 18:33

kueda self-assigned this Apr 4, 2023

kueda requested changes Apr 4, 2023

View reviewed changes

kueda mentioned this pull request Apr 12, 2023

Try a fork of react-native-image-picker #569

Closed

5 tasks

kueda closed this May 23, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Exploratory pr for native image picker #509

Exploratory pr for native image picker #509

Chrischuck commented Feb 26, 2023 •

edited

albullington left a comment

albullington Mar 7, 2023

Chrischuck Mar 7, 2023

albullington Mar 8, 2023

kueda Mar 14, 2023

Chrischuck Mar 16, 2023

albullington Mar 7, 2023

Chrischuck Mar 7, 2023

albullington Mar 7, 2023

albullington Mar 7, 2023

Chrischuck Mar 7, 2023

kueda Apr 4, 2023

Chrischuck commented Mar 16, 2023 •

edited

kueda left a comment

kueda Apr 4, 2023

kueda Apr 4, 2023

Chrischuck commented Apr 6, 2023

kueda commented Apr 7, 2023

Chrischuck commented Apr 8, 2023

Chrischuck commented Apr 8, 2023

kueda commented May 23, 2023

Exploratory pr for native image picker #509

Exploratory pr for native image picker #509

Conversation

Chrischuck commented Feb 26, 2023 • edited

android

ios 14.5

ios

albullington left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Chrischuck commented Mar 16, 2023 • edited

kueda left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Chrischuck commented Apr 6, 2023

kueda commented Apr 7, 2023

Chrischuck commented Apr 8, 2023

Chrischuck commented Apr 8, 2023

kueda commented May 23, 2023

Chrischuck commented Feb 26, 2023 •

edited

Chrischuck commented Mar 16, 2023 •

edited