add profile image #139

diegoalzate · 2021-12-07T05:33:29Z

Overview

This PR adds an upload profile image functionality to the create-profile page
ISSUE

In order to create this i installed the following packages:

web3.storage

How the AddAvatar component works

Firstly, i checked the supabase client and hooks were being created, very cool btw. So i followed that arcihtecture and stored the web3StorageClient in the common folder

import { Web3Storage } from 'web3.storage';

export const makeStorageClient = () => {
  return new Web3Storage({ token: process.env.WEB3STORAGE_TOKEN ?? '' });
};

note: this whole functionality requires a web3Storage token to be added to doppler, as of right now this token is associated to my account, ill leave this up to you guys @carlomigueldy @angeljgomezc @Dhaiwat10

AddAvatarComponent

This component recieves a src as a prop which will be the image that is rendered initially.

In this component we switch from a Image to Div with a background image using the state variable hoverImage.
The variable hoverImage changes with the toggleHover function that is triggered by the events onMouseLeave while it is a div with a background image and onMouseEnter while it is just an image component, in order to show a button while mouse is over the image and remove it when it is no longer there.
In order to upload the image we use the function submitProfilePicture, this function shows what we send to web3Storage and what they return, i added a todo so the result is updated in db

vercel · 2021-12-07T05:33:33Z

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/developdao/dao-job-board/5ytcsqcrxCAtx2kwHbJHr7kGVKGK
✅ Preview: https://dao-job-board-git-feature-add-profile-image-developdao.vercel.app

carlomigueldy · 2021-12-07T05:40:12Z

@allcontributors please add @diegoalzate for code

allcontributors · 2021-12-07T05:40:20Z

@carlomigueldy

I've put up a pull request to add @diegoalzate! 🎉

carlomigueldy

Looks good overall thank you @diegoalzate 🎉

I've requested some minor changes. But I also noticed that yarn.lock was removed and package-lock.json is added, I am not entirely sure if this is a problem but would like to request your review 👀 on this PR @with-heart @Dhaiwat10

client/pages/create-profile/add-avatar.tsx

client/package.json

…emoved style components from package.json

JoviDeCroock · 2021-12-07T12:56:56Z

client/common/web3Storage.ts

+import { Web3Storage } from 'web3.storage';
+
+export const makeStorageClient = () => {
+  return new Web3Storage({ token: process.env.WEB3STORAGE_TOKEN ?? '' });


Currently this token will be exposed in our front-end bundle, meaning any malicious actor can take this and put an infinite amount of files. Do we want to make this part of user-retrieval, ...? Or is this token scoped to only read?

Hi @JoviDeCroock, you're completely right about it being available to malicious actors. How can i not expose the token? I was not aware that it would be exposed in our front end bundle.

We can do this as a follow-up issue as we're not live yet but I'd suggest incorporating this in our api-routes so /ap/upload-avatar which will stream fe --> api-route --> web3.storage this makes our tokens safely stored in a serverless function and away from our users

Yeah definitely make an API route for this. You don't wanna expose it to the browser.

https://nextjs.org/docs/api-routes/introduction

Created #143

diegoalzate · 2021-12-07T12:57:15Z

Looks good overall thank you @diegoalzate 🎉

I've requested some minor changes. But I also noticed that yarn.lock was removed and package-lock.json is added, I am not entirely sure if this is a problem but would like to request your review 👀 on this PR @with-heart @Dhaiwat10

@carlomigueldy True i didnt notice that I removed the yarn.lock, i did a back merge to fix conflicts with the new changes that were uploaded to main but something obviously went wrong along the way. Just let me know how i should fix it :)

client/pages/create-profile/index.tsx

Dhaiwat10 · 2021-12-07T14:36:41Z

client/pages/create-profile/index.tsx

-                src="/DevDAO.png"
-                alt="developer"
-              />
+              <AddAvatar src="/DevDAO.png" />


A page file should not accept any props. Please see my comment above. Basically, if anyone vistis the create-profile/add-avatar route, will render automatically since it's the default export from the file named add-avatar.tsx.

You should be just redirecting the user to that route.

I get that pages shouldn't accept any props, in this case I thought a component was the solution to the issue

Dhaiwat10 · 2021-12-07T14:39:22Z

client/styles/ui-components/Chakra-Button.tsx

+export const ButtonGreen = ({ children, as, onClick }: ChakraButtonProps) => {
+  return (
+    <Button
+      size="sm"
+      bgColor="#38A169"
+      textColor="white"
+      _hover={{ cursor: 'pointer', bg: '#2F855A' }}
+      m="5px"
+      ms="0px"
+      w="auto"
+      fontSize="14px"
+      onClick={onClick}
+      as={as}
    >
      {children}
    </Button>
+  );
 };


I'm seeing a lot of repetition in this file. Most of these variants of the Chakra Button component share most of their props. Can we clean this up?

True, I don't remember changing this page it might just be style changes that show up in the git log, I'll look into it either way 😃

These buttons will probably be changed, but you're right about cleaning the code

Dhaiwat10

Clean up the Buttons file and you're good to go!

carlomigueldy · 2021-12-08T02:52:02Z

Looks good overall thank you @diegoalzate 🎉
I've requested some minor changes. But I also noticed that yarn.lock was removed and package-lock.json is added, I am not entirely sure if this is a problem but would like to request your review 👀 on this PR @with-heart @Dhaiwat10

@carlomigueldy True i didnt notice that I removed the yarn.lock, i did a back merge to fix conflicts with the new changes that were uploaded to main but something obviously went wrong along the way. Just let me know how i should fix it :)

Yo @diegoalzate ! Just delete all package-lock.json files and run yarn install this creates the yarn.lock files. Then that should be it for the quick fix.

And then we're ready to merge this in!

diegoalzate · 2021-12-08T04:09:43Z

Looks good overall thank you @diegoalzate 🎉
I've requested some minor changes. But I also noticed that yarn.lock was removed and package-lock.json is added, I am not entirely sure if this is a problem but would like to request your review 👀 on this PR @with-heart @Dhaiwat10

@carlomigueldy True i didnt notice that I removed the yarn.lock, i did a back merge to fix conflicts with the new changes that were uploaded to main but something obviously went wrong along the way. Just let me know how i should fix it :)

Yo @diegoalzate ! Just delete all package-lock.json files and run yarn install this creates the yarn.lock files. Then that should be it for the quick fix.

And then we're ready to merge this in!

@carlomigueldy sounds great, just pushed these changes, not sure why it says i dismissed @Dhaiwat10 stale review :/

carlomigueldy · 2021-12-08T06:34:40Z

Looks good overall thank you @diegoalzate 🎉
I've requested some minor changes. But I also noticed that yarn.lock was removed and package-lock.json is added, I am not entirely sure if this is a problem but would like to request your review 👀 on this PR @with-heart @Dhaiwat10

@carlomigueldy True i didnt notice that I removed the yarn.lock, i did a back merge to fix conflicts with the new changes that were uploaded to main but something obviously went wrong along the way. Just let me know how i should fix it :)

Yo @diegoalzate ! Just delete all package-lock.json files and run yarn install this creates the yarn.lock files. Then that should be it for the quick fix.
And then we're ready to merge this in!

@carlomigueldy sounds great, just pushed these changes, not sure why it says i dismissed @Dhaiwat10 stale review :/

Whoops sorry, I think it was the branch rules that I put. I removed that rule instead so it doesn't get dismissed unintentionally

carlomigueldy

Well done mate @diegoalzate 🥂

diegoalzate added 3 commits November 22, 2021 20:10

added add-avatar component

13db37a

merge with main

0730adf

added AddAvatar Component to create-profile index

71db37d

diegoalzate requested review from carlomigueldy and 4gnle December 7, 2021 05:33

diegoalzate self-assigned this Dec 7, 2021

allcontributors bot mentioned this pull request Dec 7, 2021

docs: add diegoalzate as a contributor for code #140

Merged

carlomigueldy suggested changes Dec 7, 2021

View reviewed changes

client/pages/create-profile/add-avatar.tsx Outdated Show resolved Hide resolved

client/package.json Outdated Show resolved Hide resolved

carlomigueldy requested review from with-heart and Dhaiwat10 December 7, 2021 08:11

changed typescript type variable from camel case to pascal case and r…

9d0fc4d

…emoved style components from package.json

vercel bot deployed to Preview December 7, 2021 12:52 View deployment

diegoalzate requested a review from carlomigueldy December 7, 2021 12:52

JoviDeCroock reviewed Dec 7, 2021

View reviewed changes

Dhaiwat10 reviewed Dec 7, 2021

View reviewed changes

client/pages/create-profile/index.tsx Outdated Show resolved Hide resolved

Dhaiwat10 reviewed Dec 7, 2021

View reviewed changes

moved AddAvatar from pages folder to components folder

aafe813

vercel bot deployed to Preview December 7, 2021 18:10 View deployment

diegoalzate requested a review from Dhaiwat10 December 7, 2021 18:10

Dhaiwat10 previously approved these changes Dec 8, 2021

View reviewed changes

removed package lock and added yarn lock

ec2021f

diegoalzate dismissed Dhaiwat10’s stale review via ec2021f December 8, 2021 04:07

vercel bot deployed to Preview December 8, 2021 04:07 View deployment

carlomigueldy mentioned this pull request Dec 8, 2021

Move Web3Storage to API routes #143

Open

carlomigueldy approved these changes Dec 8, 2021

View reviewed changes

carlomigueldy merged commit badd2ea into main Dec 8, 2021

carlomigueldy deleted the feature/add-profile-image branch December 8, 2021 06:36

4gnle mentioned this pull request Dec 27, 2021

Adding Profile Image/Avatar #60

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add profile image #139

add profile image #139

diegoalzate commented Dec 7, 2021

vercel bot commented Dec 7, 2021 •

edited

Loading

carlomigueldy commented Dec 7, 2021

allcontributors bot commented Dec 7, 2021

carlomigueldy left a comment

JoviDeCroock Dec 7, 2021

diegoalzate Dec 7, 2021

JoviDeCroock Dec 7, 2021

Dhaiwat10 Dec 7, 2021

Dhaiwat10 Dec 7, 2021

carlomigueldy Dec 8, 2021

diegoalzate commented Dec 7, 2021

Dhaiwat10 Dec 7, 2021

diegoalzate Dec 7, 2021

Dhaiwat10 Dec 7, 2021

diegoalzate Dec 7, 2021

4gnle Dec 7, 2021

Dhaiwat10 left a comment

carlomigueldy commented Dec 8, 2021 •

edited

Loading

diegoalzate commented Dec 8, 2021

carlomigueldy commented Dec 8, 2021

carlomigueldy left a comment

add profile image #139

add profile image #139

Conversation

diegoalzate commented Dec 7, 2021

Overview

How the AddAvatar component works

AddAvatarComponent

vercel bot commented Dec 7, 2021 • edited Loading

carlomigueldy commented Dec 7, 2021

allcontributors bot commented Dec 7, 2021

carlomigueldy left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

diegoalzate commented Dec 7, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Dhaiwat10 left a comment

Choose a reason for hiding this comment

carlomigueldy commented Dec 8, 2021 • edited Loading

diegoalzate commented Dec 8, 2021

carlomigueldy commented Dec 8, 2021

carlomigueldy left a comment

Choose a reason for hiding this comment

vercel bot commented Dec 7, 2021 •

edited

Loading

carlomigueldy commented Dec 8, 2021 •

edited

Loading