From 600aed2c1e6b792f79dd030042915f2d9d90d4ad Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Mon, 22 May 2023 11:53:53 -0700 Subject: [PATCH 1/3] deps: Upgrade react-native-image-picker to 5.3.1, the latest This has changes that look interesting for #3624: https://github.com/react-native-image-picker/react-native-image-picker/commit/fcb8aa641 (More details in an upcoming commit with our fix for that issue.) Also, a small change that should let us offer video capture on iOS: https://github.com/react-native-image-picker/react-native-image-picker/commit/4de5e5a39 If we end up having to revert this upgrade because of something unrelated, that change should be easy to patch in with `patch-package`; see b546b0785 for an example of using patch-package. --- ios/Podfile.lock | 4 ++-- package.json | 2 +- ...ypeof-to-type-in-TurboModule-imports.patch} | 18 ++++++++++++++++-- .../platforms/NativeImagePicker.js.flow | 12 ++++++++++++ .../lib/typescript/types.js.flow | 1 + yarn.lock | 8 ++++---- 6 files changed, 36 insertions(+), 9 deletions(-) rename types/patches/{0029-tsflower-Fix-a-typeof-to-type-in-import.patch => 0029-tsflower-Fix-a-typeof-to-type-in-TurboModule-imports.patch} (52%) create mode 100644 types/react-native-image-picker/lib/typescript/platforms/NativeImagePicker.js.flow diff --git a/ios/Podfile.lock b/ios/Podfile.lock index a1548fc2a96..cc2c8881484 100644 --- a/ios/Podfile.lock +++ b/ios/Podfile.lock @@ -314,7 +314,7 @@ PODS: - React-Core - react-native-document-picker (8.2.0): - React-Core - - react-native-image-picker (4.10.3): + - react-native-image-picker (5.3.1): - React-Core - react-native-netinfo (6.0.0): - React-Core @@ -735,7 +735,7 @@ SPEC CHECKSUMS: React-logger: b75b80500ea80457b2cf169427d66de986cdcb29 react-native-cameraroll: cb752fda6d5268f1646b4390bd5be1f27706b9a0 react-native-document-picker: 495c444c0c773c6e83a5d91165890ecb1c0a399a - react-native-image-picker: 60f4246eb5bb7187fc15638a8c1f13abd3820695 + react-native-image-picker: ec9b713e248760bfa0f879f0715391de4651a7cb react-native-netinfo: e849fc21ca2f4128a5726c801a82fc6f4a6db50d react-native-photo-view: ea0ec91bf5991a6843e740b1f47ab355171c996c react-native-safe-area-context: 39c2d8be3328df5d437ac1700f4f3a4f75716acc diff --git a/package.json b/package.json index 5df6b5980a6..3563010015f 100644 --- a/package.json +++ b/package.json @@ -60,7 +60,7 @@ "react-native": "0.68.7", "react-native-document-picker": "^8.1.3", "react-native-gesture-handler": "^2.8.0", - "react-native-image-picker": "4.10.3", + "react-native-image-picker": "^5.3.1", "react-native-open-notification": "^0.1.4", "react-native-photo-view": "alwx/react-native-photo-view#91b873c85", "react-native-reanimated": "^2.2.0 <2.3.0", diff --git a/types/patches/0029-tsflower-Fix-a-typeof-to-type-in-import.patch b/types/patches/0029-tsflower-Fix-a-typeof-to-type-in-TurboModule-imports.patch similarity index 52% rename from types/patches/0029-tsflower-Fix-a-typeof-to-type-in-import.patch rename to types/patches/0029-tsflower-Fix-a-typeof-to-type-in-TurboModule-imports.patch index 5f045cf5ca6..84fbed0dcfc 100644 --- a/types/patches/0029-tsflower-Fix-a-typeof-to-type-in-import.patch +++ b/types/patches/0029-tsflower-Fix-a-typeof-to-type-in-TurboModule-imports.patch @@ -1,12 +1,26 @@ From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 From: Greg Price Date: Tue, 7 Jun 2022 21:19:03 -0700 -Subject: [tsflower] Fix a `typeof` to `type` in import +Subject: [tsflower] Fix a `typeof` to `type` in TurboModule imports --- + .../lib/typescript/platforms/NativeImagePicker.js.flow | 2 +- .../lib/typescript/specs/NativeSafeAreaContext.js.flow | 2 +- - 1 file changed, 1 insertion(+), 1 deletion(-) + 2 files changed, 2 insertions(+), 2 deletions(-) +diff --git types/react-native-image-picker/lib/typescript/platforms/NativeImagePicker.js.flow types/react-native-image-picker/lib/typescript/platforms/NativeImagePicker.js.flow +index 0e0f49f69..1e426ab40 100644 +--- types/react-native-image-picker/lib/typescript/platforms/NativeImagePicker.js.flow ++++ types/react-native-image-picker/lib/typescript/platforms/NativeImagePicker.js.flow +@@ -1,7 +1,7 @@ + /* @flow + * @generated by TsFlower + */ +-import { typeof TurboModule } from 'react-native/Libraries/TurboModule/RCTExport'; ++import { type TurboModule } from 'react-native/Libraries/TurboModule/RCTExport'; + + export interface Spec extends TurboModule { + launchCamera(options: Object, callback: () => void): void; diff --git types/react-native-safe-area-context/lib/typescript/specs/NativeSafeAreaContext.js.flow types/react-native-safe-area-context/lib/typescript/specs/NativeSafeAreaContext.js.flow index 80f3194b4..58f386b04 100644 --- types/react-native-safe-area-context/lib/typescript/specs/NativeSafeAreaContext.js.flow diff --git a/types/react-native-image-picker/lib/typescript/platforms/NativeImagePicker.js.flow b/types/react-native-image-picker/lib/typescript/platforms/NativeImagePicker.js.flow new file mode 100644 index 00000000000..1e426ab404d --- /dev/null +++ b/types/react-native-image-picker/lib/typescript/platforms/NativeImagePicker.js.flow @@ -0,0 +1,12 @@ +/* @flow + * @generated by TsFlower + */ +import { type TurboModule } from 'react-native/Libraries/TurboModule/RCTExport'; + +export interface Spec extends TurboModule { + launchCamera(options: Object, callback: () => void): void; + launchImageLibrary(options: Object, callback: () => void): void; +} + +declare var _default: Spec | null; +export default _default; diff --git a/types/react-native-image-picker/lib/typescript/types.js.flow b/types/react-native-image-picker/lib/typescript/types.js.flow index 5f8a1086a68..d0f3e54ee54 100644 --- a/types/react-native-image-picker/lib/typescript/types.js.flow +++ b/types/react-native-image-picker/lib/typescript/types.js.flow @@ -11,6 +11,7 @@ export interface OptionsCommon { videoQuality?: AndroidVideoOptions | iOSVideoOptions; includeBase64?: boolean; includeExtra?: boolean; + formatAsMp4?: boolean; presentationStyle?: | 'currentContext' | 'fullScreen' diff --git a/yarn.lock b/yarn.lock index b377dc7d326..46b5b94a26b 100644 --- a/yarn.lock +++ b/yarn.lock @@ -10401,10 +10401,10 @@ react-native-gradle-plugin@^0.0.6: resolved "https://registry.yarnpkg.com/react-native-gradle-plugin/-/react-native-gradle-plugin-0.0.6.tgz#b61a9234ad2f61430937911003cddd7e15c72b45" integrity sha512-eIlgtsmDp1jLC24dRn43hB3kEcZVqx6DUQbR0N1ABXGnMEafm9I3V3dUUeD1vh+Dy5WqijSoEwLNUPLgu5zDMg== -react-native-image-picker@4.10.3: - version "4.10.3" - resolved "https://registry.yarnpkg.com/react-native-image-picker/-/react-native-image-picker-4.10.3.tgz#cdc11d9836b4cfa57e658c0700201babf8fdca10" - integrity sha512-gLX8J6jCBkUt6jogpSdA7YyaGVLGYywRzMEwBciXshihpFZjc/cRlKymAVlu6Q7HMw0j3vrho6pI8ZGC5O/FGg== +react-native-image-picker@^5.3.1: + version "5.3.1" + resolved "https://registry.yarnpkg.com/react-native-image-picker/-/react-native-image-picker-5.3.1.tgz#836ab13c228174728a0bd68293b27dc5e1affa0f" + integrity sha512-zRCjtlE3KOeaWDM8gXzTwXfvo3ZeF2XMkHceU7CVCtKRleKxna/E4XWIPu/lXO2qlMdnSx1WvfPSbqzAX0qxpA== react-native-iphone-x-helper@^1.3.0: version "1.3.1" From d026153fd957e4c0621c3abf18efe71b51a07436 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Mon, 22 May 2023 18:16:11 -0700 Subject: [PATCH 2/3] compose: Allow uploading videos from media library (iOS, and Android 13+) This issue was stalled while waiting for react-native-image-picker to include a filename extension in the `fileName` property of the asset payload, specifically in the case of videos on Android. We sent two PRs with reasonable approaches to do that, but they went stale: https://github.com/react-native-image-picker/react-native-image-picker/pull/1881 https://github.com/react-native-image-picker/react-native-image-picker/pull/1895 (We need appropriate filename extensions because Zulip servers want to interpret them.) But in January, a commit landed that actually added the filename extensions (!): https://github.com/react-native-image-picker/react-native-image-picker/commit/fcb8aa641 Or at least, it added the extensions in the case where the Intent gives us a `content://` URL for the asset. (Note that the `.getExtensionFromMimeType` function call is added to `getFileTypeFromMime`, which is a helper for `getAppSpecificStorageUri`, which -- starting in that same commit -- is called for videos when the URL has `content://`.) We can guarantee getting a `content://` URL in at least the following way: restrict this feature to Android SDK 33+. As we know (and can confirm from reading code), the library uses Android's preferred new media-picker UI starting at that version: new Intent(MediaStore.ACTION_PICK_IMAGES) and the doc confirms that that indeed gives "content URI(s)": https://developer.android.com/reference/android/provider/MediaStore#ACTION_PICK_IMAGES It'd be nicer to allow video selection on older Android too. But that seems nontrivial, unless we can guarantee getting `content://` URLs from the ACTION_PICK intents created by this library, and I don't think we can. The alternative is a result we've rejected before: a bad, generic file-picker UI that's not specialized to images/videos, via a library codepath that uses Intent.ACTION_GET_CONTENT. Or I suppose I could basically rebase one of my library PRs I mentioned at the top of this commit message, so we'd get filename extensions irrespective of the `content://` detail. We wouldn't even have to wait for it to land; we found `patch-package` quite usable; see, e.g., b546b0785. But at this point I think I've spent plenty of time on this, and the benefit of the current approach will be felt by a large and growing portion of our users. Fixes: #3624 --- src/compose/ComposeMenu.js | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/src/compose/ComposeMenu.js b/src/compose/ComposeMenu.js index 7a9ed436fa9..65a3118dd0e 100644 --- a/src/compose/ComposeMenu.js +++ b/src/compose/ComposeMenu.js @@ -88,6 +88,18 @@ const kShouldOfferImageMultiselect = // Android 13 || androidSdkVersion() >= 33; +// As of r-n-image-picker v5.3.1, this is needed to ensure that any received +// videos have a `fileName` that includes a filename extension. (Servers +// will interpret the filename extension.) See library implementation. +// +// For older Android, it seems the alternative is a generic file-picker UI +// not specialized to images/videos (in a library codepath that uses +// Intent.ACTION_GET_CONTENT). +const kShouldOfferImagePickerMixedMedia = + Platform.OS === 'ios' + // Android 13 + || androidSdkVersion() >= 33; + type MenuButtonProps = $ReadOnly<{| onPress: () => void | Promise, IconComponent: SpecificIconType, @@ -215,8 +227,7 @@ export default function ComposeMenu(props: Props): Node { const handleImagePicker = useCallback(() => { launchImageLibrary( { - // TODO(#3624): Try 'mixed', to allow both photos and videos - mediaType: 'photo', + mediaType: kShouldOfferImagePickerMixedMedia ? 'mixed' : 'photo', quality: 1.0, includeBase64: false, From 8f2864a72b3d08d5f9a66f43857dee31ec50b981 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Mon, 22 May 2023 18:15:42 -0700 Subject: [PATCH 3/3] compose: Offer video capture on iOS Thanks to the recent upgrade of react-native-image-picker, which had a small change to support 'mixed' in `launchCamera`: https://github.com/react-native-image-picker/react-native-image-picker/commit/4de5e5a39 If we end up having to revert that upgrade because of something unrelated, that change should be easy to patch in with `patch-package`; see b546b0785 for an example of using patch-package. Fixes: #5733 --- ios/ZulipMobile/Info.plist | 2 ++ src/compose/ComposeMenu.js | 20 +++++++++++++++++++- 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/ios/ZulipMobile/Info.plist b/ios/ZulipMobile/Info.plist index 3c5eed6b667..ae62862c9ef 100644 --- a/ios/ZulipMobile/Info.plist +++ b/ios/ZulipMobile/Info.plist @@ -54,6 +54,8 @@ By allowing camera access, you can take photos and send them in Zulip messages. NSLocationWhenInUseUsageDescription + NSMicrophoneUsageDescription + By allowing microphone access, you can take videos and send them in Zulip messages. NSPhotoLibraryAddUsageDescription Save photos you choose from Zulip to your photo library. NSPhotoLibraryUsageDescription diff --git a/src/compose/ComposeMenu.js b/src/compose/ComposeMenu.js index 65a3118dd0e..77774690c83 100644 --- a/src/compose/ComposeMenu.js +++ b/src/compose/ComposeMenu.js @@ -259,7 +259,25 @@ export default function ComposeMenu(props: Props): Node { launchCamera( { - mediaType: 'photo', + // Here's how iOS video recording works in an attempt on iOS 16: + // - iOS gives a native interface with a camera preview. You can + // choose "photo" or "video", with "photo" selected by default. + // - On tapping "video" for the first time, it shows a permissions + // prompt with the NSMicrophoneUsageDescription string. (If we + // didn't declare that string, the app would hang here.) + // - If you grant the permission, the interaction proceeds as + // you'd hope: you take a video (or cancel), then confirm, + // retake, or cancel. + // - If you don't grant the permission, it proceeds similarly, + // except the sent video will have no sound. Later interactions + // will also let you take video without sound, and you won't get + // prompted about permissions (!). If you want to record videos + // with sound, you have to look in Settings by yourself and + // grant the permission there. + // + // 'mixed' is not supported on Android: + // https://github.com/react-native-image-picker/react-native-image-picker#options + mediaType: Platform.OS === 'ios' ? 'mixed' : 'photo', // On Android ≤9 (see above) and on iOS, this means putting up a // scary permission request. Shrug, because other apps seem to save