Unprofile: add avatar image upload #37
Conversation
| @@ -17,6 +17,7 @@ | |||
| "ethers": "5.6.8", | |||
| "react": "^17.0.2", | |||
| "react-dom": "^17.0.2", | |||
| "react-image-file-resizer": "^0.4.8", | |||
There was a problem hiding this comment.
This library doesn't add enough value to warrant adding it IMHO.
| const payload = target.type === 'checkbox' ? target.checked : target.value | ||
| dispatch({ type, payload }) | ||
| } catch (error) { | ||
| console.log(error) | ||
| } | ||
| } | ||
|
|
||
| const onAvatarChange = (dispatch, target) => { |
There was a problem hiding this comment.
nit: you should define fns before they are called ex. this is called in change and then defined later. It should be switched.
|
|
||
| import AppConfig from 'App.config' | ||
|
|
||
| const AVATAR_CONFIG = { |
There was a problem hiding this comment.
How does this ensure the size stays below a limit?
There was a problem hiding this comment.
If you mean the file size, it doesn't, and neither would a straightforward canvas implementation. I figure there is an upper bound to how large of a file a 500x500 image can be, and resizing it by height/width would keep it reasonably small in the vast majority of cases (or maybe in all cases if there's not some way that a 500x500 image can be huge). Since you didn't seem to be attached to a specific hard limit, it didn't feel necessary to sink more effort into enforcing both a max height/width and a max file size.
There was a problem hiding this comment.
File size is my primary concern and it should definitely be checked.
|
|
||
| if (type === 'avatar') { | ||
| onAvatarChange(dispatch, target) | ||
| target.value = null // reset so onChange will fire even for same image |
There was a problem hiding this comment.
the dom cannot directly be manipulated in react.
I recommend you read up on controlled vs uncontrolled components:
https://reactjs.org/docs/uncontrolled-components.html
| </label> | ||
|
|
||
| <div className='mb-2 text-sm text-gray-700'> | ||
| Upload a JPEG or PNG. Recommended size is 500 x 500. |
There was a problem hiding this comment.
Those are just the ones I know should render properly for everyone.
| className='px-6 py-3 w-full bg-kernel-eggplant-mid text-white rounded text-center cursor-pointer' | ||
| > | ||
| <input | ||
| type='file' accept='image/png, image/jpeg' |
There was a problem hiding this comment.
we should use image/* so mobile phones allow you to take a picture with the camera
see https://developer.mozilla.org/en-US/docs/Web/HTML/Attributes/accept#unique_file_type_specifiers
|
Closing this as I won't be able to finish it; anyone is welcome to pick up the branch and proceed! |
react-image-file-resizerto auto-resize image to a max of 500x500. I can probably do it without the library (viacanvas), but it would be rather verbose.Addresses #15
Empty state:
Populated state:
Explorer:
