DevSoc-Asansol2023 / DevSoc-Main

Official website of our club "Developer Society"
https://devsocofficial.netlify.app/
MIT License
16 stars 11 forks source link

Profile Created #21

Closed jgyfutub closed 1 year ago

jgyfutub commented 1 year ago

I have created the Profile web page. Although I have not put star background because I was not able to download it from figma.I request to please review my pr.Solves issue #12

Dev Soc and 6 more pages - Personal - Microsoft​ Edge 07-10-2023 21_15_19

jgyfutub commented 1 year ago

@ArnabChatterjee20k please review it. I have added meow image as a sample for cover and profile

ArnabChatterjee20k commented 1 year ago

Please make the header profile section according to the design file Please move the avatar portion to the left side instead of middle like the way present in the figma file Here is the star background

bg-stars-1 webp

Also make tabs interactable

jgyfutub commented 1 year ago

Dev Soc and 5 more pages - Personal - Microsoft​ Edge 07-10-2023 23_58_27 See if this works fine.

ArnabChatterjee20k commented 1 year ago

One question are the tabs working? The issue is this "Implement the user profile system to track the user's participation in various events using horizontal cards(use dummy data)" So please try to implement some cards as well to test the tab switching is working or not

You can use react router and outlet component of react router to do this If any confusion or any help needed msg here

ArnabChatterjee20k commented 1 year ago

Also please try to capture a video as these are more of kind of interactivity problem statements

jgyfutub commented 1 year ago

One question are the tabs working? The issue is this "Implement the user profile system to track the user's participation in various events using horizontal cards(use dummy data)" So please try to implement some cards as well to test the tab switching is working or not

You can use react router and outlet component of react router to do this If any confusion or any help needed msg here

No the tabs working hasn't been implemented yet.

ArnabChatterjee20k commented 1 year ago

One question are the tabs working? The issue is this "Implement the user profile system to track the user's participation in various events using horizontal cards(use dummy data)" So please try to implement some cards as well to test the tab switching is working or not You can use react router and outlet component of react router to do this If any confusion or any help needed msg here

No the tabs working hasn't been implemented yet.

Please implement the tabs working cause that is important

jgyfutub commented 1 year ago

Yes I will implement it👍👍

jgyfutub commented 1 year ago

https://github.com/DevSoc-Asansol2023/DevSoc-Main/assets/97391064/41485c8d-01b0-4b6a-8fdf-06a958187f66

I have implemented functionality of tabs with two horizontal cards. Please review it @ArnabChatterjee20k

ArnabChatterjee20k commented 1 year ago

Thanks @jgyfutub for your valuable contribution. One more question, this is responsive nah?

jgyfutub commented 1 year ago

Background Attachment - Tailwind CSS and 6 more pages - Personal - Microsoft​ Edge 08-10-2023 13_53_21

There is some problem in mobile version.The cover photo is getting too small but although I tried to implement sm class of tailwind css it was not implementing.And the background image is not fixed in background while scrolling even though I coded backgroundAttachement: 'fixed' it was not applying.Please suggest me how to fix this

ArnabChatterjee20k commented 1 year ago

Ah without responsiveness we can't accept this pr cause for that we need to create one issue. Try to fix it. If any help required post here

netlify[bot] commented 1 year ago

Deploy Preview for devsoc-main ready!

Name Link
Latest commit b2ae3825c72a97bcddaaff032e83bd2d3371cd65
Latest deploy log https://app.netlify.com/sites/devsoc-main/deploys/652d238d33ffe800087e2485
Deploy Preview https://deploy-preview-21--devsoc-main.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

jgyfutub commented 1 year ago

Background Attachment - Tailwind CSS and 6 more pages - Personal - Microsoft​ Edge 08-10-2023 16_01_32

I have fixed all the responsive problems.Please review this @ArnabChatterjee20k

ArnabChatterjee20k commented 1 year ago

Yeah its nice But make some improvement In mobile

jgyfutub commented 1 year ago

Dev Soc and 3 more pages - Personal - Microsoft​ Edge 08-10-2023 21_22_24

I have improved as instructed. Please review it @ArnabChatterjee20k

ArnabChatterjee20k commented 1 year ago

The ui is looking nice but please make requested changes

jgyfutub commented 1 year ago

The ui is looking nice but please make requested changes

I have done as you said reducing the profile size ,bringing the name below ,cover photo to full and size of tabs increased

ArnabChatterjee20k commented 1 year ago

The ui is looking nice but please make requested changes

I have done as you said reducing the profile size ,bringing the name below ,cover photo to full and size of tabs increased

Yes you have done that but you have done some mistake in controlling the logic of the tabs handling. You made that logic for only two tabs. What if we have more than 2 tabs. You can see where I requested the changes in the files changed tab

jgyfutub commented 1 year ago

The ui is looking nice but please make requested changes

I have done as you said reducing the profile size ,bringing the name below ,cover photo to full and size of tabs increased

Yes you have done that but you have done some mistake in controlling the logic of the tabs handling. You made that logic for only two tabs. What if we have more than 2 tabs. You can see where I requested the changes in the files changed tab

So I just have to change the tab handling functionality for handling more than 2 tabs.Is every other thing fine?

ArnabChatterjee20k commented 1 year ago

I am reviewing the code step by step. First I checked the ui and responsiveness Now I am checking the code quality. For now yes the tabs thing

jgyfutub commented 1 year ago

I have changed the logic for tab handling such that it can handle multiple tabs.Please review it @ArnabChatterjee20k

jgyfutub commented 1 year ago

@ArnabChatterjee20k the functionality of tabs is such that if there are 4 tabs then they will change the isOpen variable according to their serial number,for eg if 3rd tab is clicked then 'isOpen' will changed to 2 and so on.So, the data of only that tab will be showed whose serial number is equal to isOpen.And for other number <div></div> will be shown.Taking above example,the data of 3rd tab will be shown because isOpen==2 as this variable to set to 2 by clicking the 3rd tab due to which 'setIsOpen(2)' occurs.I have worked upon this logic with below commit for few changes in accordance to the logic explained

ArnabChatterjee20k commented 1 year ago

Again you are doing the same mistake. You are restricting the number of tabs. Dont make specific handlers like this switchTab1 or 2 Make a single handler switchTab which takes an id of the tab and switched to that tab. For content rendering use react router routes. The codes is just getting repeated. In future if we are going to some more features here we need to again copy and paste the code. So follow principles of DRY

jgyfutub commented 1 year ago

Ok so I just have to make a single SwitchTab function rather than SwitchTab1,2 etc.And render each tab content by react router since all these tabs cards code will be in different file?

jgyfutub commented 1 year ago

@ArnabChatterjee20k I tried to better the code quality by making files for each tab data and putting serial number in values of each button .Please check the code and tell where shall I improve the code quality.

ArnabChatterjee20k commented 1 year ago

@ArnabChatterjee20k I tried to better the code quality by making files for each tab data and putting serial number in values of each button .Please check the code and tell where shall I improve the code quality.

yes its fine and you make it really good Just two small changes and it will be great Thanks @jgyfutub

jgyfutub commented 1 year ago

@ArnabChatterjee20k I have made an array where all the tabs components are kept,thus calling them by their index number whenever that particular tab is clicked,thereby making the object in the state with the keys tabLabel,tabData.Please review it

jgyfutub commented 1 year ago

@ArnabChatterjee20k I have done recent changes suggested in new commits. Thus I sincerely request you to please merge this pr or review it so that I can improve this pr

jgyfutub commented 1 year ago

ok thanks for helping!!

ArnabChatterjee20k commented 1 year ago

@jgyfutub any updates?

jgyfutub commented 1 year ago

I will make the commits you requested by the end of day

jgyfutub commented 1 year ago

@ArnabChatterjee20k I have done the changes as you suggested, please review the recent commit

ArnabChatterjee20k commented 1 year ago

Thanks @jgyfutub for your valuable contribution I am merging it

jgyfutub commented 1 year ago

Thanks @jgyfutub for your valuable contribution I am merging it

Thanks for your guidance!