natysoz / expo-images-picker

Multiple Asset Photos Videos selecting package for Expo SDK
MIT License
95 stars 35 forks source link

Add mediaType to modAssets and fix the type following the changes #34

Closed jte0711 closed 2 years ago

jte0711 commented 3 years ago

Resolve #33

jte0711 commented 3 years ago

hey @natysoz , is there anything you want me to add in this PR?

natysoz commented 2 years ago

see if u can update your PR so i can merge it

jte0711 commented 2 years ago

@natysoz which part of it do you want me to update? I don't see any conflict for the branch

natysoz commented 2 years ago

@jte0711 sorry i forgot to push my last version the conflicts are just in package json i believe

jte0711 commented 2 years ago

@natysoz no worries, I updated the PR

natysoz commented 2 years ago

can u also add this to readme and changelog please ?

jte0711 commented 2 years ago

@natysoz Done, I updated the changelog. There's no documentation on what the component will return in the readme so I don't see where I should write down my changes.

natysoz commented 2 years ago

before merging , 1 question

what is the difference between the image type we get back from the image type we are converting into ?

const widgetResize = useMemo( () => ({ width: 512, compress: 0.7, base64: false, saveTo: 'jpeg' }), [] )

at that point i know that all images will return as image/jpeg so why do we need to add it back inside the images object?

we cant just add it if necessary inside our component or state before uploading ?

jte0711 commented 2 years ago

what is the difference between the image type we get back from the image type we are converting into ?

const widgetResize = useMemo( () => ({ width: 512, compress: 0.7, base64: false, saveTo: 'jpeg' }), [] )

at that point i know that all images will return as image/jpeg so why do we need to add it back inside the images object?

yea that's true, but user's gallery is a mix of photo, video, and other mediaType so we need it to specify which mediaType the file is.

I know we can just detect it from the filename, but then it will be so inconsistent. Here is the type of data we normally return

export declare type Asset = {
    id: string;
    filename: string;
    uri: string;
    mediaType: MediaTypeValue;
    mediaSubtypes?: string[];
    width: number;
    height: number;
    creationTime: number;
    modificationTime: number;
    duration: number;
    albumId?: string;
};

Here is the type of data returned for resized image:

export declare type ImageResult = {
    uri: string;
    width: number;
    height: number;
    base64?: string;
};

so adding mediaType will help at least to show what's the type of the file.

natysoz commented 2 years ago

fix conflicts ill merge

jte0711 commented 2 years ago

fix conflicts ill merge

Ok, I just did it.