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.

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Jump to bottom

fix: video upload issue through image picker #2204

Merged
merged 5 commits into from
Aug 3, 2023
Merged

fix: video upload issue through image picker #2204

Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
a00225b
fix: video upload issue through image picker
khushal87 Aug 2, 2023
091cd31
fix: video upload issue through image picker
khushal87 Aug 2, 2023
2dfa6a0
refactor: add relevant comment
khushal87 Aug 2, 2023
f58de9e
fix: check of video type in AttachmentPickerItem.tsx
khushal87 Aug 3, 2023
29ec548
fix: check of video type in AttachmentPickerItem.tsx
khushal87 Aug 3, 2023
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
5 changes: 4 additions & 1 deletion package/src/components/AttachmentPicker/components/AttachmentPickerItem.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,14 +70,16 @@ const AttachmentVideo: React.FC<AttachmentVideoProps> = (props) => {
Alert.alert('Maximum number of files reached');
return files;
}
// Since Expo MediaLibrary doesn't return the mimetype of the image/video, we have to derive the mimeType.
const mimeType = lookup(asset.filename) || 'multipart/form-data';
Copy link
Member

Choose a reason for hiding this comment

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

if expo is the only that doesnt return mime type why is it being done for both here ?

Copy link
Member

Choose a reason for hiding this comment

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

as per expo docs, mimetype is optionally present in the asset type

https://docs.expo.dev/versions/latest/sdk/document-picker/#documentpickerasset

Copy link
Member

Choose a reason for hiding this comment

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

also present in older versions

https://docs.expo.dev/versions/v47.0.0/sdk/document-picker/#documentresult

Copy link
Member Author

@khushal87 khushal87 Aug 3, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

If you read the comment above, I am not saying expo doesn't do that(send mimeType in response). Expo MediaLibrary doesn't do that, Expo DocumentPicker does. In our SDK we don't do a lookup for the code for the document picker or file picker, we do it for the image picker/media library @santhoshvai

Copy link
Member

@santhoshvai santhoshvai Aug 3, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I thought its related to document picker, because you said that this was broken by the docu,ent picker version update

Copy link
Member Author

Choose a reason for hiding this comment

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

No, what I meant was the previous PR of the document picker that was merged yesterday caused this issue of video upload because mime-type was compulsory.

Copy link
Member Author

Choose a reason for hiding this comment

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

There's a better way to fix this and that we discussed yesterday, handling lookup in the expo-package and native-package and sending mimetype through the package itself

return [
...files,
{
duration: durationLabel,
id: asset.id,
mimeType,
name: asset.filename,
size: asset.fileSize,
type: 'video',
uri: asset.uri,
},
];
Expand Down Expand Up @@ -176,6 +178,7 @@ const AttachmentImage: React.FC<AttachmentImageProps> = (props) => {

const getFileType = (asset: Asset) => {
const { filename } = asset;
// Since Expo MediaLibrary doesn't return the mimetype of the image/video, we have to derive the mimeType from filename.
if (filename) {
const contentType = lookup(filename) || 'multipart/form-data';
return contentType.startsWith('image/') ? 'image' : 'video';
Expand Down
Loading