-
Notifications
You must be signed in to change notification settings - Fork 0
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
Added image upload component and backend call #132
Conversation
✅ Deploy Preview for motherhoodbeyondbars ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work! This works well on web and mobile which is perfect. I had some comments about code practices, but I'll fix them for now since we're running low on time, try to keep in mind separating frontend and backend code and returning objects from the backend while completing future tickets!
(snapshot.bytesTransferred / snapshot.totalBytes) * 100; | ||
}, | ||
(error) => { | ||
console.error("Upload failed:", error); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Return an object instead of printing the error or showing an alert so that displaying errors can be handled by the frontend, should be in the form {success: false, error: [error message]}
}); | ||
if (!response.ok) { | ||
const result = await response.json(); | ||
alert(result.error || "Error uploading image metadata"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Return an object here as well instead of an error
alert("An error occurred while uploading the image."); | ||
} | ||
}; | ||
return <input type="file" onChange={saveFile} />; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The backend and frontend should be separated where the backend endpoint is in a separate file from the frontend so that it can be re-used
web/pages/api/save-image.ts
Outdated
req: NextApiRequest, | ||
res: NextApiResponse | ||
) { | ||
if (req.method === "POST") { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
API isn't needed in this case since Firebase can do client-side database calls, so to speed up code fetching we should call the Firebase functions as endpoints rather than in the API. There are some cases where we need to handle things on the server-side, but we're good in this case
What does this Pull Request change?
Created a frontend component ImageUpload.tsx for caretakers to upload baby pictures. It is currently unstyled, only uses a basic input component. When the user selects an image, in the frontend the image is uploaded to firebase. Then if the upload is successful, a newly implemented backend call saves the image metadata to the firestore path
/babies/{babyID}/book/{caregiverID_timestamp}
Important Changes
Related PRs
Anything else?