NativeScript / plugins

@nativescript plugins to help with your developments.
https://docs.nativescript.org/plugins/index.html
Apache License 2.0
187 stars 104 forks source link

[@nativescript/imagepicker] Awaiting image selection on iOS doesn't also wait for the picker to be fully closed #556

Closed hendrickson-tyler closed 2 weeks ago

hendrickson-tyler commented 7 months ago

The Issue

Consider the following common use case:

I'm currently implementing this using @nstudio/nativescript-loading-indicator (v4.3.4) and @nativescript/imagepicker (v3.0.1):

// Get the image
const imagePicker = create({
  mode: 'single',
  mediaType: ImagePickerMediaType.Image,
  prompt: 'Select a photo',
});
await imagePicker.authorize();
const selection = await imagePicker.present();
const selectedImages = selection.map((selectedImage) => selectedImage.asset);

// Upload the image
const loadingIndicator = new LoadingIndicator();
loadingIndicator.show({
  message: 'Uploading photo...',
});
await uploadPhoto(selectedImages[0]);
loadingIndicator.hide();

You would expect the loading indicator to display right after the image is selected, but it never does. From what I can tell, this is because the loading indictor is displaying right away, but it displays in the context of the image picker modal which is in the process of closing. Thus, you never see it back in the view that presented the image picker.

Workaround

By using a setTimeout to wait for the modal to fully close, the loading indicator can display correctly:

...
const selection = await imagePicker.present();
const selectedImages = selection.map((selectedImage) => selectedImage.asset);

setTimeout(async () => {
  const loadingIndicator = new LoadingIndicator();
  loadingIndicator.show({
    message: 'Uploading photo...',
  });
  await uploadPhoto(selectedImages[0]);
  loadingIndicator.hide();
}, 250); // Wait for the iOS image picker to close.

Reproduction

Minimal example on StackBlitz Notice that there is no loading indicator. Uncomment lines 23 and 37 in main-view-model.ts to re-enable the setTimeout. Re-run and notice that the loading indicator now displays correctly.

Expected Behavior

ImagePicker.present waits for the modal to be fully closed before resolving the Promise, thus avoiding this type of issue altogether.

hendrickson-tyler commented 2 weeks ago

Closing, as this has been fixed in v3.2.1 with PR #581 using the new resolveWhenDismissed option (thank you, @farfromrefug 🙂).

@NathanWalker, could we make this option the default going forward? It seems to me that it should always wait for the modal to close before resolving (similar to @nativescript/camera). Unless there's a scenario I'm not thinking of

NathanWalker commented 2 weeks ago

Thank you @hendrickson-tyler mainly to avoid backwards compat breaking change.