Skip to content
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

[7주차] 바리바리 미션 제출합니다. #2

Open
wants to merge 70 commits into
base: master
Choose a base branch
from

Conversation

YelynnOh
Copy link

@YelynnOh YelynnOh commented Jun 28, 2023

배포 링크

https://react-vote-17th-alwexv2wb-hmuri.vercel.app/

안녕하세요 🍚바리바리🍚의 오예린, 최민주입니다! 이번 과제를 하면서 백엔드와 처음으로 같이 협업을 해보며 실제 프로젝트를 구현하기 전 큰 도움이 되었던 것 같습니다.
바리바리의 빛 디자인 팀의 도움으로 아주 멋진 디자인의 사이트를 만들 수 있었습니다. 디자인 팀 최공~!
https://www.figma.com/file/12WK4MEhjwNmt89HkRu8Gp/%EB%B0%94%EB%A6%AC%EB%B0%94%EB%A6%AC-%ED%88%AC%ED%91%9C?node-id=21%3A70&mode=dev

구현 기능 설명

로그인 안 한 유저는 어느 경로를 거치던 투표 페이지에 접근할 수 없도록 구현했습니다.

필수 요건

  • UI/UX에 대한 감각을 최대한 발휘해 디자인을 적용해 봅니다.
  • HTTPS를 통해 서버와 통신합니다.

선택 사항

  • API Fetch는 어떤 방식을 사용하든 무방합니다 (axios, Fetch API)
  • Promise.then() 보단 async/await를 사용해 보세요. 더 최신 스펙이랍니다.

hmuri and others added 27 commits June 27, 2023 21:55
[chore] nav 회원가입 문구 추가
[fix] team recoil highest 추가
로그인, 로그아웃, 회원가입, 데모 투표, 데모 결과 보기 완성
chore:  최종 dev 머지
chore: 최종 dev 머지
Copy link

@oyatplum oyatplum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

안녕하세요 Repick 이예지입니다😇

백엔드와 처음 협업하는 과제 하시느라 여러 시행착오가 있으셨을텐데 무사히 마무리 하시느라 수고하셨어요~!
많은 노력이 담긴 코드였던 것 같습니다ㅎㅎ

세션 때 봬요🙏🏻

Comment on lines +1 to +12
{
"message": "투표 조회",
"vote_count": [
{
"part": "김현우",
"total": 2
},
{
"part": "최유미",
"total": 1
}
],

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

혹시 이 파일은 어떤 용도로 있는걸까용??

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

음 이건 실제 api 연결하기 전에 더미 데이터 식으로 만들어 놓은 건데 미처 삭제를 하지 않았네요..! 수정하도록 하겠습니다😥

export const fetchUserPart = async () => {
try {
const accessToken = localStorage?.getItem('access')?.replace(/"/g, '');
const response = await axios.get(`https://ceos-vote.kro.kr/accounts/login/`, {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

url은 .env 파일에서 관리해주는 게 좋을 것 같아요!!

Comment on lines +43 to +45
console.log(axiosError.response.data);
console.log(axiosError.response.status);
console.log(axiosError.response.headers);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

console.log 찍은 부분은 지우고 올려도 좋겠어용~!

Comment on lines +21 to +27
return '🥇';
case 1:
return '🥈';
case 2:
return '🥉';
default:
return '🏅';

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

메달 아이디어 귀엽네용ㅎㅎㅎ~

Copy link
Member

@Gaeun-Kwon Gaeun-Kwon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

안녕하세요 권가은입니다 🙌
이번 과제 하시느라 수고 많으셨습니다!!

코드가 많다보니 로직에 대한 자세한 리뷰는 드리지 못 했지만, 코멘트 몇 가지 남겨보았으니 참고만 해주세요 ㅎㅎ
앞으로의 프로젝트도 화이팅입니다~!!

Comment on lines +52 to +58
export const useAllIndividuals = () => {
return useRecoilValue(voteResultList);
};

export const useSetAllIndividualsState = () => {
return useSetRecoilState(voteResultList);
};
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이 부분 두 가지로 나누어서 따로 선언 후 사용하신 이유가 어떤 건지 궁금해요!!

import './App.css';
import React, {ReactNode, ReactElement} from 'react';
import { GlobalStyle } from './styles/GlobalStyle';
import {useLocation, Route, Routes, Navigate} from 'react-router-dom';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

프로젝트에서 useNavigate 과 Navigate을 모두 사용하셨는데, 혹시 용도 구분이 어떻게 될까요 ??

Comment on lines +8 to +10
type DropDownProps = {
onSelectTeam: (option: string) => void;
};
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

프로젝트에서 type 과 interface 를 같이 사용하셨는데, 어떤 사용 기준이 있으셨는지 궁급합니다 !!

Comment on lines +34 to +35
localStorage.clear();
resetStorage();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

전체적으로 코드 중복이 있는 것 같아요 !

Comment on lines +27 to +28
<LogBox isDisabled={isActive ? true : false}>{location === '/' ? '로그인' : '회원가입'}</LogBox>
<InfoBox isDisabled={isActive ? false : true} />
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

요런 부분들은
isDisabled = {isActive}
로 줄일 수도 잇을 것 같아요!

Comment on lines +16 to +22
const [selectPart, setSelectPart] = useState('프론트엔드');
const [selectTeam, setSelectTeam] = useState('');
const [name, setName] = useState('');
const [userId, setUserId] = useState('');
const [password, setPassword] = useState('');
const [confirmPassword, setConfirmPassword] = useState('');
const [email, setEmail] = useState('');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

여러개의 input을 다뤄야 하는 상황이라면 이렇게 state도 여러개를 정의해주어야 하고, 매번 리렌더링도 발생하는 문제가 있는 것 같아요 😥
그래서 저희 팀은 이번에 로그인, 회원가입에 use-hook-form 을 사용했는데 한 번 확인해보세요! ㅎㅎ

Copy link

@flowersayo flowersayo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

안녕하세요! 이번주 코드리뷰 파트너를 맡은 Therapease의 김서연입니다 ~!
짧은 시간안에 이렇게 완성도 높은 프로젝트를 만들어주셔서 깜짝 놀랐습니다!!!!
소통이 많이 필요할듯한 까다로운 추가 기능도 많이 신경써주신 것 같아서 놀랐습니다 👍👍
과제 넘 고생 많으셨습니다! 스터디 시간에 뵙겠습니다 😊.

const formData = new FormData();
formData.append('part', selectedMember);
const accessToken = localStorage?.getItem('access')?.replace(/"/g, '');
const response = await axios.post(`https://ceos-vote.kro.kr/votes/part/`, formData, {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • 보통 formData는 파일, ASCII 형식이 아닌 데이터와 바이너리 데이터를 업로드하는 용도로 주로 사용되는 것 같은데,
    혹시 어떠한 연유로 json 대신 formData를 활용하게 되었는지 궁금합니다..!!!
  • 이 부분 코드는 axios 모듈을 하나 만들어서 매 요청시에 활용한다면 매번 axios의 헤더를 싣는 번거로움을 최소화 할 수 있을 것 같아요! 관련 자료 첨부합니다~~!

Comment on lines +105 to +129
if (userPart === '프론트엔드') {
subtitle = '프론트엔드';
} else if (userPart === '백엔드') {
subtitle = '백엔드';
}
return (
<VoteWrapper onSubmit={onSubmit}>
<HeaderBox>{subtitle} 파트장 투표</HeaderBox>
<VoteList>
{filteredMembers.map((value, index) => (
<div key={index}>
<VoteBox
type="button"
onClick={() => selectMember(value.part)}
isSelected={value.part === selectedMember}
>
{value.part}
</VoteBox>
</div>
))}
</VoteList>
<SubmitButton type="submit">투표완료</SubmitButton>
</VoteWrapper>
);
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. subtitle 은 화면 렌더링에 영향을 주는 변수인데 state형이 아니라 let으로 선언이 되어있는 것 같더라구요!!
  2. 매 렌더링마다 userPart와 subtitle 값이 같게된다면 아래처럼 userPart를 바로 넣어주어도 될 것 같은데 따로 선언해주신 이유가 궁금합니다!!

<HeaderBox>{userPart} 파트장 투표</HeaderBox>

Comment on lines +47 to +68
} catch (error) {
console.error(error);
const axiosError = error as AxiosError<ErrorResponse>;
if (axiosError.response) {
const errorMessage = axiosError.response.data.message; // Now TypeScript knows that `data` has a `message` property
if (errorMessage == '내 팀은 투표 할 수 없습니다!') {
alert(errorMessage);
} else if (errorMessage == '투표는 한 번만!') {
alert(errorMessage);
window.location.replace('/demoResult');
}
console.log(axiosError.response.data);
console.log(axiosError.response.status);
console.log(axiosError.response.headers);
} else if (axiosError.request) {
console.log(axiosError.request);
} else {
console.log('Error', axiosError.message);
}
}
}
};

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

본인팀은 투표할 수 없도록 에러메시지로 분기한 점이 인상깊네요..!! 백엔드 분들과 소통과 협업을 굉장히 잘해주신 결과 같아요!! 😊😊

Comment on lines +26 to +36
const onClickChoosePart = (part: string) => {
setSelectPart(part);
};

const handleSelectTeam = (option: string) => {
setSelectTeam(option);
};

const handleSignUp = async (e: any) => {
e.preventDefault();

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

리액트 함수 네이밍 컨벤션을 잘 지켜서 작성해주셔서 가독성이 높아보여요 👍👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants