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주차] 댄서포트(Dan-support) 미션 제출합니다. #5

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

Conversation

sujinRo
Copy link

@sujinRo sujinRo commented Jun 28, 2023

안녕하세요! 💃댄서포트(Dan-support)팀의 노수진, 신유진입니다.

이번 과제에서는 백엔드와 첫 협업을 하며, 이 후 있을 팀 프로젝트에서 어떻게 개발을 해 나가면 좋을 지 생각할 수 있었습니다!
디자인은 이상하게 ,, figma에서 디자인하며 생각한 크기와 실제 화면에서 크기가 달라서 개발을 하며 크기를 바로바로 수정할 수 밖에 없었습니다…

이번 과제 모두 수고하셨습니다! 😃


구현:

  • async/wait을 이용하여 api 연결을 하였습니다.
  • recoil을 이용하여 현재 user의 name, team, part, accessToken을 확인할 수 있도록 하였습니다.
  • refresh token과 access token을 이용하여 저장을 하고자 하였는데, 로그인은 되지만,, 현재 token refresh가 되지 않는 상태로,, 미완성 단계에 있습니다. 최대한 빨리 완성시키도록 하겠습니다!

링크:

figma 링크:
https://www.figma.com/file/wyqt2bDhisefhfQNUXXvQl/CEOS-_-front%26back?type=design&node-id=0%3A1&mode=design&t=qNbiSe3WM6Exga7B-1

배포링크:
https://react-vote-17th-k6i9p2ykw-dansupport.vercel.app/

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의 김서연입니다 ~!
만들어주신 프로젝트로 직접 로그인, 회원가입, 투표까지 진행해보았는데
디자인도 깔끔하고 완성도도 높고 추가 기능도 많이 신경써주신 것 같아서 놀랐습니다 👍👍
스터디 시간에 뵙겠습니다 😊

@@ -0,0 +1,8 @@
import axios from 'axios';

Choose a reason for hiding this comment

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

만들어주신 axios 모듈에 토큰을 싣는 코드를 작성해주시면,
나머지 api 요청들을 서비스화 한 코드들에서 중복된 코드를 훨씬 줄일 수 있을 것 같아요!

headers: {
      Authorization: `Bearer ${accessToken}`,
    },
    

Comment on lines +1 to +54
import client from './client';

//파트장 투표
export const votePartLeader = async (info: any) => {
const response = await client.post(
'/votes/candidates/',
{
cname: info.name,
part: info.part,
},
{
headers: {
Authorization: `Bearer ${info.accessToken}`,
},
},
);
return response.data;
};

//파트장 투표 중복 제한(권한 확인)
export const checkPartLeaderVoteAuthority = async (accessToken: string) => {
const response = await client.get('/votes/candidates/authority/', {
headers: {
Authorization: `Bearer ${accessToken}`,
},
});
return response.data;
};

//데모데이 투표
export const demoDayVote = async (info: any) => {
const response = await client.post(
'/votes/teams/',
{
tname: info.tname,
},
{
headers: {
Authorization: `Bearer ${info.accessToken}`,
},
},
);
return response.data;
};

//데모데이 투표 중복 제한(권한 확인)
export const demoDayAuthority = async (info: any) => {
const response = await client.get('/votes/teams/authority/', {
headers: {
Authorization: `Bearer ${info.accessToken}`,
},
});
return response.data;
};

Choose a reason for hiding this comment

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

오옴.. 각 API 요청을 깔끔하게 잘 분리해서 서비스화해주신 것 같아요 👍
매 api 요청마다 유저 정보를 매개변수로 넘겨주도록 되어있는데,
앞선 코멘트처럼 모듈 자체에 토큰을 싣거나,

유저 정보를 본 파일에서 공통으로 불러온다면, 매번 번거롭게 매 api 요청마다 유저 정보를
파라미터로 넘겨주지 않을 수도 있을 것 같습니다!

Comment on lines +31 to +44
//click유무에 따라 alert
const voteMutationDemo = useMutation(demoDayVote, {
onSuccess: data => {
alert('투표되었습니다');
router.push('/result/demo-day');
},
onError: data => {
if (clickIndex == 5) {
alert('투표할 팀을 선택해주세요');
} else {
alert('자신의 팀은 투표가 불가합니다');
}
},
});

Choose a reason for hiding this comment

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

react-query 로 데이터 fetching 을 하니 훨씬 코드가 가독성 높아보이네요!!!👍

Comment on lines +20 to +28
//파트장 투표 중복 제한(권한 확인)
export const checkPartLeaderVoteAuthority = async (accessToken: string) => {
const response = await client.get('/votes/candidates/authority/', {
headers: {
Authorization: `Bearer ${accessToken}`,
},
});
return response.data;
};

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.

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

이번 과제에 next를 사용하시다니 !! 코드도 정말 깔끔해서 리뷰하기 좋았습니다!
코드 스타일도 굉장히 통일되어있고 .. 최고의 협업이네요

앞으로의 프로젝트도 화이팅입니다~!!

@@ -0,0 +1,26 @@
import '@/styles/globals.css';
import type { AppProps } from 'next/app';
import { QueryClient, QueryClientProvider } from '@tanstack/react-query';
Copy link
Member

Choose a reason for hiding this comment

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

react-query 를 사용하셨네요 !!
여러 유저에 의해 투표결과가 달라질 수 있는 상황에서 server state 관리하기 좋은 선택지인 것 같아요 👍👍

@@ -0,0 +1,93 @@
import { useState } from 'react';
import { Ingroup } from '../../interface/interface';
import styles from '../../styles/DemoDayPage.module.css';
Copy link
Member

Choose a reason for hiding this comment

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

이번 과제에 CSS-in-CSS 를 선택하셨던 히스토리가 궁금합니다!!

Comment on lines +8 to +31
const [name, setName] = useState<string>('');
const [id, setId] = useState<string>('');
const [pwd, setPwd] = useState<string>('');
const [pwdConfirm, setPwdConfirm] = useState<string>('');
const [email, setEmail] = useState<string>('');
const [team, setTeam] = useState<string>('댄서포트');
const [part, setPart] = useState<string>('fe'); //'fe'/'be'

//에러 메시지,확인 메시지 state
const [nameMsg, setNameMsg] = useState<string>('');
const [idMsg, setIdMsg] = useState<string>('');
const [pwdMsg, setPwdMsg] = useState<string>('');
const [pwdConfirmMsg, setPwdConfirmMsg] = useState<string>('');
const [emailMsg, setEmailMsg] = useState<string>('');

//유효성 검사 state (Checked->형식에 맞는지 안맞는지, Valid->중복인지 아닌지)
const [isNameChecked, setIsNameChecked] = useState<boolean>(false);
const [isIdChecked, setIsIdChecked] = useState<boolean>(false);
const [isPwdChecked, setIsPwdChecked] = useState<boolean>(false);
const [isPwdConfirmChecked, setIsPwdConfirmChecked] =
useState<boolean>(false);
const [isEmailChecked, setIsEmailChecked] = useState<boolean>(false);
const [isIdValid, setIsIdValid] = useState<boolean>(false);
const [isEmailValid, setIsEmailValid] = useState<boolean>(false);
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
Member

@moong23 moong23 left a comment

Choose a reason for hiding this comment

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

안녕하세요 Hooking팀입니다~!

광기의 112커밋 보고 깜짝놀랐습니다...

많은 고생하셨구나 싶었어요

과제하느라 고생많으셨습니다~!!!

"name": "react-vote-17th",
"version": "0.1.0",
"private": true,
"proxy": "https://takgyun.shop",
Copy link
Member

Choose a reason for hiding this comment

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

api 주소는 배포시에는 가려주시는게 좋을 것 같아요~!

return (
<>
{groups.map((group, idx) => (
<div key={idx} onClick={() => onClick(group)}>
Copy link
Member

Choose a reason for hiding this comment

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

key에 idx값 가능하면 안넣기로 해요~!!

const token = user.accessToken;

//팀 클릭했을 때
const onClick = (group: Ingroup) => {
Copy link
Member

Choose a reason for hiding this comment

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

onClick과 같은 경우는 아래 html에서 사용할 메서드 이름과 동일하기도 하고, 만약 onClick eventhandler가 여럿 필요한 경우 헷갈릴 수 있겠다는 생각이 들어요. onClickTeam과 같은 경우로 바꿔주시는것 추천드려요!

const logoutMutation = useMutation(logout, {
onSuccess: data => {
console.log(data);
setUser({ ...user, accessToken: '', name: '', team: '', part: '' });
Copy link
Member

Choose a reason for hiding this comment

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

useResetRecoilState method를 사용하면

Suggested change
setUser({ ...user, accessToken: '', name: '', team: '', part: '' });
useResetRecoilState(userState);

로도 쓸 수 있어요!
userState의 초기값을 기억해두고 일일히 초기화하지않아도 되는 방법이라 추천드립니다~!

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