Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: multiple image upload issue using native image picker and generic improvements for upload #2638

Merged
merged 5 commits into from
Aug 30, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions package/expo-package/src/optionalDependencies/takePhoto.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@ export const takePhoto = ImagePicker
// since we only support single photo upload for now we will only be focusing on 0'th element.
const photo = assets && assets[0];

console.log(photo);

if (canceled === false && photo && photo.height && photo.width && photo.uri) {
khushal87 marked this conversation as resolved.
Show resolved Hide resolved
let size: Size = {};
if (Platform.OS === 'android') {
Expand Down Expand Up @@ -74,6 +76,7 @@ export const takePhoto = ImagePicker

return {
cancelled: false,
size: photo.fileSize,
source: 'camera',
uri: photo.uri,
...size,
Expand Down
6 changes: 4 additions & 2 deletions package/native-package/src/optionalDependencies/pickImage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,9 @@ try {
export const pickImage = ImagePicker
? async () => {
try {
const result = await ImagePicker.launchImageLibrary({ mediaType: 'mixed' });
const result = await ImagePicker.launchImageLibrary({
mediaType: 'mixed',
});
const canceled = result.didCancel;
const errorCode = result.errorCode;

Expand All @@ -20,7 +22,7 @@ export const pickImage = ImagePicker
if (!canceled) {
const assets = result.assets.map((asset) => ({
...asset,
duration: asset.duration * 1000, // in milliseconds
duration: asset.duration ? asset.duration * 1000 : undefined, // in milliseconds
name: asset.fileName,
size: asset.fileSize,
source: 'picker',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ export const takePhoto = ImagePicker
}
return {
cancelled: false,
size: photo.size,
source: 'camera',
uri: photo.path,
...size,
Expand Down
109 changes: 65 additions & 44 deletions package/src/components/MessageInput/MessageInput.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ import {
useTranslationContext,
} from '../../contexts/translationContext/TranslationContext';

import { triggerHaptic } from '../../native';
import { isImageMediaLibraryAvailable, triggerHaptic } from '../../native';
import type { Asset, DefaultStreamChatGenerics } from '../../types/types';
import { AutoCompleteInput } from '../AutoCompleteInput/AutoCompleteInput';

Expand Down Expand Up @@ -118,7 +118,6 @@ type MessageInputPropsWithContext<
| 'FileUploadPreview'
| 'fileUploads'
| 'giphyActive'
| 'hasImagePicker'
| 'ImageUploadPreview'
| 'imageUploads'
| 'Input'
Expand Down Expand Up @@ -185,7 +184,6 @@ const MessageInputWithContext = <
FileUploadPreview,
fileUploads,
giphyActive,
hasImagePicker,
ImageUploadPreview,
imageUploads,
Input,
Expand Down Expand Up @@ -349,46 +347,71 @@ const MessageInputWithContext = <
imagesToRemove.forEach((image) => removeImage(image.id));
};

const uploadFilesHandler = async () => {
const fileToUpload = selectedFiles.find((selectedFile) => {
const uploadedFile = fileUploads.find(
(fileUpload) =>
fileUpload.file.uri === selectedFile.uri || fileUpload.url === selectedFile.uri,
);
return !uploadedFile;
});
if (fileToUpload) await uploadNewFile(fileToUpload);
};

const removeFilesHandler = () => {
const filesToRemove = fileUploads.filter(
(fileUpload) =>
!selectedFiles.find(
(selectedFile) =>
selectedFile.uri === fileUpload.file.uri || selectedFile.uri === fileUpload.url,
),
);
filesToRemove.forEach((file) => removeFile(file.id));
};

/**
* When a user selects or deselects an image in the image picker using media library.
*/
useEffect(() => {
if (imagesForInput) {
if (selectedImagesLength > imageUploadsLength) {
/** User selected an image in bottom sheet attachment picker */
uploadImagesHandler();
} else {
/** User de-selected an image in bottom sheet attachment picker */
removeImagesHandler();
const uploadOrRemoveImage = async () => {
if (imagesForInput) {
if (selectedImagesLength > imageUploadsLength) {
/** User selected an image in bottom sheet attachment picker */
await uploadImagesHandler();
} else {
/** User de-selected an image in bottom sheet attachment picker */
removeImagesHandler();
}
}
}
};
// If image picker is not available, don't do anything
if (!isImageMediaLibraryAvailable()) return;
uploadOrRemoveImage();
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [selectedImagesLength]);
khushal87 marked this conversation as resolved.
Show resolved Hide resolved

/**
* When a user selects or deselects a video in the image picker using media library.
*/
useEffect(() => {
if (selectedFilesLength > fileUploadsLength) {
/** User selected a video in bottom sheet attachment picker */
const fileToUpload = selectedFiles.find((selectedFile) => {
const uploadedFile = fileUploads.find(
(fileUpload) =>
fileUpload.file.uri === selectedFile.uri || fileUpload.url === selectedFile.uri,
);
return !uploadedFile;
});
if (fileToUpload) uploadNewFile(fileToUpload);
} else {
/** User de-selected a video in bottom sheet attachment picker */
const filesToRemove = fileUploads.filter(
(fileUpload) =>
!selectedFiles.find(
(selectedFile) =>
selectedFile.uri === fileUpload.file.uri || selectedFile.uri === fileUpload.url,
),
);
filesToRemove.forEach((file) => removeFile(file.id));
}
const uploadOrRemoveFile = async () => {
if (selectedFilesLength > fileUploadsLength) {
/** User selected a video in bottom sheet attachment picker */
await uploadFilesHandler();
} else {
/** User de-selected a video in bottom sheet attachment picker */
removeFilesHandler();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This also covers the case where selectedFilesLength === fileUploadsLength, is this the desired behaviour ? Just curious about this, not a remark

}
};
uploadOrRemoveFile();
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [selectedFilesLength]);
Comment on lines 407 to 408
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here


/**
* This is for image attachments selected from attachment picker.
*/
useEffect(() => {
if (imagesForInput && hasImagePicker) {
if (imagesForInput && isImageMediaLibraryAvailable()) {
if (imageUploadsLength < selectedImagesLength) {
// /** User removed some image from seleted images within ImageUploadPreview. */
const updatedSelectedImages = selectedImages.filter((selectedImage) => {
Expand All @@ -401,9 +424,7 @@ const MessageInputWithContext = <
setSelectedImages(updatedSelectedImages);
} else if (imageUploadsLength > selectedImagesLength) {
/**
* User is editing some message which contains image attachments OR
* image attachment is added from custom image picker (other than the default bottomsheet image picker)
* using `uploadNewImage` function from `MessageInputContext`.
* User is editing some message which contains image attachments.
**/
setSelectedImages(
imageUploads
Expand All @@ -418,10 +439,13 @@ const MessageInputWithContext = <
}
}
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [imageUploadsLength, hasImagePicker]);
}, [imageUploadsLength]);
khushal87 marked this conversation as resolved.
Show resolved Hide resolved

/**
* This is for video attachments selected from attachment picker.
*/
useEffect(() => {
if (hasImagePicker) {
if (isImageMediaLibraryAvailable()) {
if (fileUploadsLength < selectedFilesLength) {
/** User removed some video from seleted files within ImageUploadPreview. */
const updatedSelectedFiles = selectedFiles.filter((selectedFile) => {
Expand All @@ -434,9 +458,7 @@ const MessageInputWithContext = <
setSelectedFiles(updatedSelectedFiles);
} else if (fileUploadsLength > selectedFilesLength) {
/**
* User is editing some message which contains video attachments OR
* video attachment is added from custom image picker (other than the default bottom-sheet image picker)
* using `uploadNewFile` function from `MessageInputContext`.
* User is editing some message which contains video attachments.
**/
setSelectedFiles(
fileUploads.map((fileUpload) => ({
Expand All @@ -450,9 +472,10 @@ const MessageInputWithContext = <
}
}
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [fileUploadsLength, hasImagePicker]);
}, [fileUploadsLength]);

const editingExists = !!editing;

useEffect(() => {
if (editing && inputBoxRef.current) {
inputBoxRef.current.focus();
Expand Down Expand Up @@ -1036,7 +1059,6 @@ export const MessageInput = <
FileUploadPreview,
fileUploads,
giphyActive,
hasImagePicker,
ImageUploadPreview,
imageUploads,
Input,
Expand Down Expand Up @@ -1117,7 +1139,6 @@ export const MessageInput = <
FileUploadPreview,
fileUploads,
giphyActive,
hasImagePicker,
ImageUploadPreview,
imageUploads,
Input,
Expand Down
19 changes: 8 additions & 11 deletions package/src/contexts/messageInputContext/MessageInputContext.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -657,7 +657,7 @@ export const MessageInputProvider = <
);
}
if (!photo.cancelled) {
setSelectedImages((images) => [...images, photo]);
await uploadNewImage(photo);
}
};

Expand All @@ -677,14 +677,11 @@ export const MessageInputProvider = <
);
}
if (result.assets && result.assets.length > 0) {
result.assets.forEach((asset) => {
result.assets.forEach(async (asset) => {
if (asset.type.includes('image')) {
setSelectedImages((prevImages) => [...prevImages, asset]);
await uploadNewImage(asset);
} else {
setSelectedFiles((prevFiles) => [
...prevFiles,
{ ...asset, mimeType: asset.type, type: FileTypes.Video },
]);
await uploadNewFile({ ...asset, mimeType: asset.type, type: FileTypes.Video });
}
});
}
Expand Down Expand Up @@ -740,15 +737,15 @@ export const MessageInputProvider = <
});

if (!result.cancelled && result.assets) {
result.assets.forEach((asset) => {
result.assets.forEach(async (asset) => {
/**
* TODO: The current tight coupling of images to the image
* picker does not allow images picked from the file picker
* to be rendered in a preview via the uploadNewImage call.
* This should be updated alongside allowing image a file
* uploads together.
*/
uploadNewFile(asset);
await uploadNewFile(asset);
});
}
};
Expand Down Expand Up @@ -1291,7 +1288,7 @@ export const MessageInputProvider = <
]);

if (isAllowed) {
uploadFile({ newFile });
await uploadFile({ newFile });
}
};

Expand Down Expand Up @@ -1333,7 +1330,7 @@ export const MessageInputProvider = <
]);

if (isAllowed) {
uploadImage({ newImage });
await uploadImage({ newImage });
}
};

Expand Down
Loading