fixes #5 Feature: Sending Images #6
Conversation
| if(imageUrlInput.current.files && imageUrlInput.current.files.length === 1) { | ||
| const file = imageUrlInput.current.files[0]; |
There was a problem hiding this comment.
This will limit the user to send only one image at a time, we want to give the user the ability to upload multiple images at once 👍
There was a problem hiding this comment.
Thank you for all the great feedback!
| const formData = new FormData(); | ||
| formData.append("file", file); | ||
| formData.append("upload_preset", "zm201ktq"); |
There was a problem hiding this comment.
I would suggest hiding sensitive information in a .env file.
| const OPTIONS = { | ||
| method: "POST", | ||
| body: formData | ||
| } | ||
| fetch("https://api.cloudinary.com/v1_1/hatchypicturesticket/image/upload", OPTIONS) | ||
| .then(response => response.json()) | ||
| .then(json => { | ||
| setPhotoUrl(json); | ||
| }) | ||
| } | ||
| }; |
There was a problem hiding this comment.
Instead of using .then() I suggest using async/await to stay consistent with the codebase
Instead of using fetch I suggest using axios with the transformRequest to stay consistent with the rest of the codebase :)
There was a problem hiding this comment.
Consistency is key! I'll change that over
|
|
||
| const setPhotoUrl = (response) => { | ||
| photoUrls.current = [...photoUrls.current, response.secure_url]; | ||
| console.log(photoUrls.current); |
There was a problem hiding this comment.
You should avoid having unnecessary console.log statements in your PRs 👍
There was a problem hiding this comment.
Whoops! Taken care of!
| /> | ||
| </FormControl> | ||
| </form> | ||
| <div className={classes.attatchFileContainer}> |
There was a problem hiding this comment.
You should not mix JSX with Material UI components. Every div should be replaced by Box or Grid.
| { modalOpen && ( | ||
| <form type="submit" onSubmit={(e) => handleUpload(e)} className={classes.photoForm}> | ||
| <input ref={imageUrlInput} type="file" accept="image/png, image/jpg" className={classes.chooseFilesButton}/> | ||
| <button className={classes.sendButton}>Upload</button> |
There was a problem hiding this comment.
I was able to change out the <form> and <button> elements to use MUI but I wasn't able to find a solution for the <input> element within MUI for uploading files. Here is a link to the MUI GitHub page regarding the issue:
Dropzone, an external npm package is apparently an option, but the brief mentioned no external packages were allowed:
https://www.npmjs.com/package/material-ui-dropzone
So I just used the default <input> element instead.
| <SenderBubble key={message.id} text={message.text} time={time} /> | ||
| <SenderBubble key={message.id} text={message.text} attachments={message.attachments} time={time} /> | ||
| ) : ( | ||
| <OtherUserBubble key={message.id} text={message.text} time={time} otherUser={otherUser} /> | ||
| <OtherUserBubble key={message.id} text={message.text} attachments={message.attachments} time={time} otherUser={otherUser} /> |
| <Box className={classes.bubble}> | ||
| <Typography className={classes.text}>{text}</Typography> | ||
| </Box> | ||
| <div className={classes.paperContainer}> |
| { attachments && attachments.length > 0 && | ||
| <Box className={classes.attachmentsBox}> | ||
| { attachments.map(photoUrl => { | ||
| return ( | ||
| <Paper variant="outlined" key={photoUrl} className={classes.attachmentsPaper}> | ||
| <img src={photoUrl} | ||
| className={classes.imageThumbnail} | ||
| alt="thumbnail of sent" | ||
| aria-label="thumbnail of sent"/> | ||
| </Paper> | ||
| ) | ||
| })} | ||
| </Box> | ||
| } | ||
| { text !== "" && | ||
| <Box className={classes.bubble}> | ||
| <Typography className={classes.text}>{text}</Typography> | ||
| </Box> | ||
| } |
There was a problem hiding this comment.
When sending multiple images, the message text should be displayed above the messages.
When sending multiple images, the image size should be reduced.
There was a problem hiding this comment.
I took screenshots of the changes but I forgot to add them! Here's a screenshot of sending the images:
Here's a screenshot of receiving those same images:
| <div className={classes.paperContainer}> | ||
| { attachments && attachments.length > 0 && | ||
| <Box className={classes.attachmentsBox}> | ||
| { attachments.map(photoUrl => { | ||
| return ( | ||
| <Paper variant="outlined" key={photoUrl} className={classes.attachmentsPaper}> | ||
| <img src={photoUrl} | ||
| className={classes.imageThumbnail} | ||
| alt="thumbnail of sent" | ||
| aria-label="thumbnail of sent"/> | ||
| </Paper> | ||
| ) | ||
| })} | ||
| </Box> | ||
| } | ||
| { text !== "" && | ||
| <Box className={classes.bubble}> | ||
| <Typography className={classes.text}>{text}</Typography> | ||
| </Box> | ||
| } | ||
| </div> |
…ns. Removed conflicting JSX from render. Changed Fetch to axios for consistency. Added ability to upload multiple files at once.
Description
fixes #5
We can now attach and send multiple or single image messages along with any accompanying text.
Some other ways we could have handled this
Drawbacks: This would have complicated the render() function and made it a bit harder to read.
Drawbacks: This would have created the necessity to pass urls as props back up to the component via a function prop, which would have made it harder to track what was happening functionally.
Screenshot