Skip to content

Commit

Permalink
compose: Do try to save photos to device camera roll, after all
Browse files Browse the repository at this point in the history
In 2c2141b, we stopped passing `saveToPhotos: true` to
launchCamera, since it seemed unnecessary, and also awkward to have
to request storage permissions.

But Greg found that it's quite normal for apps with messaging to
save photos to storage when capturing from the camera [1] [2].

We also found that Android's "scoped storage" feature, which we've
enabled on Android 10+, means that this won't come with the extra
storage-permission request on Android 10+.

We do need a permissions check for Android 9 and below, though. And
since the react-native-image-picker upgrade in 36d3644, that
package hasn't been doing that permissions check for us. So we have
to do it ourselves; we reuse androidEnsureStoragePermission from
src/lightbox/download.js for that. For what this UI looks like, see
  https://chat.zulip.org/#narrow/stream/48-mobile/topic/saving.20photos.20to.20device.20on.20capture/near/1271686.

It will also mean a permission check on iOS, regardless of iOS
version; but again, other apps are doing this.

So, bring back `saveToPhotos: true`.

[1] https://chat.zulip.org/#narrow/stream/48-mobile/topic/saving.20photos.20to.20device.20on.20capture/near/1271585
[2] https://chat.zulip.org/#narrow/stream/48-mobile/topic/saving.20photos.20to.20device.20on.20capture/near/1271597
  • Loading branch information
chrisbobbe committed Oct 26, 2021
1 parent 973e091 commit 79f2703
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 1 deletion.
26 changes: 25 additions & 1 deletion src/compose/ComposeMenu.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import {
} from '../common/Icons';
import AnimatedComponent from '../animation/AnimatedComponent';
import { uploadFile } from '../actions';
import { androidEnsureStoragePermission } from '../lightbox/download';

type OuterProps = $ReadOnly<{|
expanded: boolean,
Expand Down Expand Up @@ -134,10 +135,33 @@ class ComposeMenuInner extends PureComponent<Props> {
);
};

handleCameraCapture = () => {
handleCameraCapture = async () => {
const _ = this.context;

if (Platform.OS === 'android') {
// On Android ≤9, in order to save the captured photo to storage, we
// have to put up a scary permission request. We don't have to do that
// when using "scoped storage", which we do on later Android versions.
await androidEnsureStoragePermission({
title: _('Storage permission needed'),
message: _(
'Zulip will save a copy of your photo on your device. To do so, Zulip will need permission to store files on your device.',
),
});
}

launchCamera(
{
mediaType: 'photo',

// On Android ≤9 (see above) and on iOS, this means putting up a
// scary permission request. Shrug, because other apps seem to save
// to storage, and it seems convenient; see
// https://chat.zulip.org/#narrow/stream/48-mobile/topic/saving.20photos.20to.20device.20on.20capture/near/1271633.
// TODO: Still allow capturing and sending the photo, just without
// saving to storage, if storage permission is denied.
saveToPhotos: true,

includeBase64: false,
},
this.handleImagePickerResponse,
Expand Down
2 changes: 2 additions & 0 deletions static/translations/messages_en.json
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
{
"Error": "Error",
"Storage permission needed": "Storage permission needed",
"Zulip will save a copy of your photo on your device. To do so, Zulip will need permission to store files on your device.": "Zulip will save a copy of your photo on your device. To do so, Zulip will need permission to store files on your device.",
"Something went wrong, and your message was not sent.": "Something went wrong, and your message was not sent.",
"Message not sent": "Message not sent",
"Please specify a topic.": "Please specify a topic.",
Expand Down

0 comments on commit 79f2703

Please sign in to comment.