Skip to content

Commit

Permalink
ComposeMenu.js: Handle null for fileName in response from ImagePicker.
Browse files Browse the repository at this point in the history
Fixes: #3813.

On iOS, it appears that the fileName is not included in the response
given by ImagePicker.launchCamera if the user has not granted read
access to their photo library. The case where fileName is undefined
was handled correctly, but it turns out it's coming through as null,
which was not handled, so, handle null.

Neither the types nor the docs from react-native-image-picker are
clear on this point, so that was filed as
react-native-image-picker/react-native-image-picker#1271.

Do this without creating a local copy of react-native-image-picker
with our own, correct type, since it's very likely that we'll need
to upgrade to later versions frequently as we're on an early version
of this library which is known to have limitations and bugs.

Add tests for fileName being undefined and being null.
  • Loading branch information
Chris Bobbe committed Jan 30, 2020
1 parent cb9cea0 commit ed1080c
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 5 deletions.
13 changes: 8 additions & 5 deletions src/compose/ComposeMenu.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,10 @@ type Props = $ReadOnly<{|
* actual format. The clue we get in the image-picker response is the extension
* found in `uri`.
*
* Also if `fileName` is undefined, default to the last component of `uri`.
* Also if `fileName` is null or undefined, default to the last component of `uri`.
*/
export const chooseUploadImageFilename = (uri: string, fileName: string | void): string => {
const name = fileName !== undefined ? fileName : uri.replace(/.*\//, '');
export const chooseUploadImageFilename = (uri: string, fileName: ?string): string => {
const name = fileName ?? uri.replace(/.*\//, '');

/*
* Photos in an iPhone's camera roll (taken since iOS 11) are typically in
Expand All @@ -51,7 +51,7 @@ export const chooseUploadImageFilename = (uri: string, fileName: string | void):
};

class ComposeMenu extends PureComponent<Props> {
uploadFile = (uri: string, fileName: string | void) => {
uploadFile = (uri: string, fileName: ?string) => {
const { dispatch, destinationNarrow } = this.props;
dispatch(uploadFile(destinationNarrow, uri, chooseUploadImageFilename(uri, fileName)));
};
Expand All @@ -63,7 +63,10 @@ class ComposeMenu extends PureComponent<Props> {
// https://github.com/react-native-community/react-native-image-picker/blob/master/docs/Reference.md
error?: string | void | null | false,
uri: string,
fileName: string | void,
// Upstream docs are wrong (fileName may indeed be null, at least on iOS);
// surfaced in https://github.com/zulip/zulip-mobile/issues/3813:
// https://github.com/react-native-community/react-native-image-picker/issues/1271
fileName: ?string,
}>,
) => {
if (response.didCancel) {
Expand Down
10 changes: 10 additions & 0 deletions src/compose/__tests__/ComposeMenu-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,4 +16,14 @@ describe('chooseUploadImageFilename', () => {
).toBe(`${fileNameWithoutExtension}.jpeg`);
},
);

test('Uses the last component of uri if fileName is null', () => {
const fileName = null;
expect(chooseUploadImageFilename('some/path/something.jpg', fileName)).toBe('something.jpg');
});

test('Uses the last component of uri if fileName is undefined', () => {
const fileName = undefined;
expect(chooseUploadImageFilename('some/path/something.jpg', fileName)).toBe('something.jpg');
});
});

0 comments on commit ed1080c

Please sign in to comment.