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주차] 펫플레이트 미션 제출합니다. #7

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

Conversation

Dahn12
Copy link

@Dahn12 Dahn12 commented Jun 26, 2024

배포 링크

🔗 petPlateVote

지인 코멘트

이번에 서버와의 통신을 더욱 편리하게 하려고 react-query와,axios를 처음 사용해봤는데,
확실히 리액트 내 기본 훅만 사용해서 비동기 처리할때보다 유지 보수가 편해진 것 같습니다.
백앤드분과 쿠키가 브라우저에선 접근이 가능한데 코드상에서 접근이 불가능한 문제에 관해 디스코드로 회의하면서 많은 것을 배워갈 수 있었던 것 같습니다.

다희 코멘트

실제 서비스와 같은 회원가입 로그인을 적용하고 여러 페이지를 만들면서 플로우에 대해 더 고민해 볼 수 있는 과제였던 것 같습니다! 마지막 과제이니 만큼 그동안 스터디에서 배운 내용을 복습하는 과제인것 같아 많이 도움이 되었습니다 :) 무엇보다 백파트 분들이 문제 없이 좋은 결과물을 빠른시일 내에 전달해 주셨고, 수정도 바로바로 해주셔서 어려움없이 진행할 수 있었습니다👍

디자인

디자인은 세오스 홈페이지를 많이 참고했습니다.
백그라운드 이미지와 css를 참고해서 디자인하였고, 디자인 시스템 색상도 theme에 저장하여 사용하였습니다.
스크린샷 2024-06-26 오후 10 05 19
스크린샷 2024-06-26 오후 10 11 09

신경 쓴 부분

로그인 접근 제한에 따른 페이지 이동

로그인x- > 투표 페이지 접근 가능 -> 투표랑, 결과 보기는 불가능 (버튼 클릭시 -> 로그인 페이지로 이동)
로그인 시 - > 투표 페이지 접근 가능 -> 투표랑, 결과 보기는 가능 (버튼 클릭시 -> 로그인 페이지로 이동)

토글 버튼

teamVote페이지에서 front와 back을 선택할 수 있는 페이지를 하나 더 만드는 것 보다 한 페이지 내에서 선택 가능하도록 하면 좋겠다고 생각하여 토글 버튼을 적용했습니다.

Dahn12 and others added 30 commits June 21, 2024 17:25
signUp, login 페이지 1차 틀잡기
feat : header 구현, 전체적인 디자인 수정
Copy link

@youdame youdame left a comment

Choose a reason for hiding this comment

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

고생 많으셨습니다~ 리액트 쿼리 처음 써보셔서 쉽지 않으셨을 거 같은데 쿼리키도 잘 설정해주시고 제대로 사용하신 거 같아요 ㅎㅎ UI적인 측면에서 수정이 필요할 거 같고, api 함수 명에서 일관성이 있다면 더 좋을 거 같아요!!

Copy link

Choose a reason for hiding this comment

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

스크린샷 2024-06-28 오후 2 51 19 여기 인풋이 삐져 나와요!

Copy link

Choose a reason for hiding this comment

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

스크린샷 2024-06-28 오후 3 05 24

클릭하기 전 창과 같은 크기로 드롭다운 내부 내용이 보여지는 게 더 좋을 거 같아요~ 그리고 드롭다운 외부를 클릭하면 드롭다운이 닫히는 것도 구현했다면 UX적으로 더 좋았을 거 같네요 ㅎㅎ

Comment on lines +85 to +91
<InputBox
placeholder='이메일 주소'
type='email'
value={email}
onChange={(e: React.ChangeEvent<HTMLInputElement>) =>
setEmail(e.target.value)
}
Copy link

Choose a reason for hiding this comment

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

현재 이메일 양식이 올바르지 않을 때 회원가입 버튼을 눌러야지만 이메일 양식이 올바르지 않다는 경고창이 뜨네요. 백엔드로 api 요청이 가기 전에 프론트엔드에서 이메일 양식이 올바르지 않은 값을 입력 후 onblur가 되면 바로 유효성 검사를 할 수 있도록 구현했다면 더 좋았을 거 같아요!

Choose a reason for hiding this comment

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

동의합니다 비밀번호 일치 유효성 체크하는 방식을 이메일 유효성 체크에도 적용하면 UX 개선에 좋을 것 같아요

Copy link

Choose a reason for hiding this comment

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

모든 input 컴포넌트를 제어 컴포넌트로 동작하게 만들면 하나의 인풋값이 바뀔 때마다 페이지 전체가 리렌더링 되는 현상이 발생하게 돼요! 그래서 이걸 방지하기 위해 react-hook-form 같은 라이브러리를 사용하기도 합니다. 참고하시면 좋을 거 같아요 ~ 리액트 훅폼

Copy link

Choose a reason for hiding this comment

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

인풋창이 하나라도 비어있을 때 회원가입, 로그인 버튼이 disabled 되어있는 걸 회색과 같은 명시적인 색 나타내주는 게 더 좋을 거 같아요~~

}


export const Dropdown: React.FC<DropdownProps> = ({ listsName, lists, onChange }) => {
Copy link

Choose a reason for hiding this comment

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

React.FC를 사용하는 건 안티패턴이에요. 제 기억에는 저번에도 이야기 나왔던 부분인 거 같은데 꼭 개선하는 게 좋을 거 같아요! React.FC 사용 지양하기

import axios, { InternalAxiosRequestConfig, AxiosResponse } from 'axios';
import { useNavigate } from 'react-router-dom';

const BASE_URL = 'https://testeveytime.shop/';
Copy link

Choose a reason for hiding this comment

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

모든 api가 /api/v1을 path로 두고 있어서 api/v1까지 baseURL로 두는 게 좋을 거 같아요!

Copy link

Choose a reason for hiding this comment

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

이 파일 네임에 .과 prettierrc 사이에 띄어쓰기가 있어서 모든 파일에서 프리티어가 제대로 동작하지 않고 있어요!

Comment on lines +71 to +75
<MemberVotePageContainer>
<Title>
Who do you want to vote for <br />
<span onClick={togglePart}>{isFront}</span> Leader?
</Title>
Copy link

Choose a reason for hiding this comment

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

혹시 이 부분이 저만 안 보일까요? ㅠㅠ 백엔드 투표 창으로 넘어가는 부분이 어디인지 모르겠습니다..

Choose a reason for hiding this comment

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

화면 비율을 줄이니까 보이네요! 적절한 여백 설정이 필요해보입니다!

현재 잘리고 있는 페이지 사진 첨부합니다
image
image

Copy link

@CSE-pebble CSE-pebble left a comment

Choose a reason for hiding this comment

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

수고하셨습니다~ 저희 팀도 react query를 사용했는데 query 관련 부분을 아예 따로 빼서 관리해주시는 점이 인상 깊었어요!! 배워갑니다 ㅎㅎ

<meta charset="utf-8" />
<meta name="viewport" content="width=device-width, initial-scale=1" />
<link href="https://fonts.googleapis.com/css2?family=Noto+Sans:wght@400;700&family=Open+Sans:wght@400;700&display=swap" rel="stylesheet">
<title>React App</title>

Choose a reason for hiding this comment

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

사소한 거지만 title 설정 해주시면 좋을 것 같아요!

Comment on lines +23 to +24
<Route path="/teamvote" element={<TeamVotePage />} />
<Route path="/membervote" element={<MemberVotePage />} />

Choose a reason for hiding this comment

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

team-result와 같이 vote 페이지도 케밥케이스 적용해주시는게 통일성 있을 것 같습니당

Choose a reason for hiding this comment

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

오오 이렇게 query 관련 함수를 따로 빼서 관리하는 거 너무 좋은 것 같습니다


const handleSignup = async () => {
const data = { name, username, password, email, team, role, part };
console.log(data);

Choose a reason for hiding this comment

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

콘솔 로그가 아직 남아있어요!

Comment on lines +151 to +156
const InputBoxWrapper = styled.div`
display: flex;
flex-direction: column;
width: 80%;
`;

Choose a reason for hiding this comment

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

image
image

InputBoxWrapper가 SignUpInfoWrapper를 벗어나고 있어요! height 제한도 주는게 어떨까요?


const handleLogin = async () => {
const data = { userID, password };
console.log(data);

Choose a reason for hiding this comment

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

콘솔 로그 남아있어요!

Comment on lines +71 to +75
<MemberVotePageContainer>
<Title>
Who do you want to vote for <br />
<span onClick={togglePart}>{isFront}</span> Leader?
</Title>

Choose a reason for hiding this comment

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

화면 비율을 줄이니까 보이네요! 적절한 여백 설정이 필요해보입니다!

현재 잘리고 있는 페이지 사진 첨부합니다
image
image

Comment on lines +77 to +88
//뒤에 불투명 배경 요소
background-color: transparent;
border: 0.1rem solid rgba(255, 255, 255, 0.3);
border-radius: 20px;
background-image: linear-gradient(
rgba(255, 255, 255, 0.05),
rgba(255, 255, 255, 0.2)
),
linear-gradient(rgba(255, 255, 255, 0.05), rgba(255, 255, 255, 0.01));
background-origin: border-box;
background-clip: border-box, content-box;
backdrop-filter: blur(30px);

Choose a reason for hiding this comment

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

불투명 배경 설정하는 부분이 반복되고 있어서 전역 스타일로 빼서 관리하는게 좋을 것 같아요!

Choose a reason for hiding this comment

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

theme 설정 너무 좋습니다~~

Comment on lines +85 to +91
<InputBox
placeholder='이메일 주소'
type='email'
value={email}
onChange={(e: React.ChangeEvent<HTMLInputElement>) =>
setEmail(e.target.value)
}

Choose a reason for hiding this comment

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

동의합니다 비밀번호 일치 유효성 체크하는 방식을 이메일 유효성 체크에도 적용하면 UX 개선에 좋을 것 같아요

Copy link

@Shunamo Shunamo left a comment

Choose a reason for hiding this comment

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

마지막 과제도 정말 고생 많으셨어요! 디자인보고 예뻐서 깜짝 놀랐어요 ㅎㅎ 지난 과제는 next여서 그런가 리액트 오랜만에 보니까 넘 새롭네요..ㅎㅎㅎ그동안 수고하셨어요! 많이 배워가요!! 👍 👍

export const useLoginMutation = () => {
const navigate = useNavigate();

const QUERY_KEY_LOGIN = 'login';
Copy link

Choose a reason for hiding this comment

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

const LOGIN_SUCCESS_STATUS = 201;
const BAD_REQUEST_STATUS = 400;

이런식으로 상태코드를 상수화해서 관리해도 좋을 것 같아요 ㅎㅎ

localStorage.setItem(
'userInfo',
JSON.stringify(userInfoResponse.data.value)
);
Copy link

Choose a reason for hiding this comment

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

로컬스토리지에 저장하는 로직을 따로 함수로 분리해서 사용하면 중복 코드를 줄일 수 있을 것 같아요!

const saveTokens = (accessToken, refreshToken) => {
  localStorage.setItem('accessToken', accessToken);
  localStorage.setItem('refreshToken', refreshToken);
};

const saveUserInfo = (userInfo) => {
  localStorage.setItem('userInfo', JSON.stringify(userInfo));
};

alert(' 요청입니다.');
} else {
alert('회원가입 오류: ' + res.data.message);
}*/
Copy link

Choose a reason for hiding this comment

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

필요없는 주석은 지워줘도 좋을 것 같아요!!


export const useVoteTeamMutation = () => {
const navigate = useNavigate();
const token = localStorage.getItem('accessToken');
Copy link

Choose a reason for hiding this comment

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

토큰을 가져오는 로직도 함수로 분리하면 재사용하기도 좋을 것 같아요!
const getToken = () => localStorage.getItem('accessToken');


background-color: transparent;
border: 0.1rem solid rgba(255, 255, 255, 0.3);
border-radius: 20px;
Copy link

Choose a reason for hiding this comment

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

rem이나 px중에 통일해서 사용하는게 더 좋을 것 같아요..!!!


const handleOnClick = () => {
setSelected(!selected);
};
Copy link

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