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주차] Team 엔젤브릿지 권혜인 & 이가빈 미션 제출합니다. #3

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

Conversation

billy0904
Copy link

@billy0904 billy0904 commented Jan 4, 2025

✨배포링크

  • https 브라우저 정책 보안 이슈로 백엔드 친구들이 api 네트워크 오류 수정 중에 있다는 점 감안해주시면 감사하겠습니다! 🙇🙇
  • 25/01/04 23시 35분 기준 저희 서비스 정상 영업합니다~!!🥳🎉

🗳️투표 시스템

✅ 같은 파트끼리 투표 가능
✅ 자기 자신 투표 불가능
✅ 중복 투표 불가능
✅ 같은 팀 투표 불가능

  • 따로 에러 처리는 해주지 않았지만 실사용을 고려한 리팩토링을 하게 된다면 ux 향상을 위해서 불가능한 루트에 alert 정도는 띄워보고 싶네요!

Comments

  • 협업 일정을 타이트하게 잡아서 코드가 다소 너덜너덜(?)하지만.. 사랑하는 팀원들과 같이 결과물을 내는 과정이 즐겁기도 했습니다! 다들 바쁜 와중에도 열심히 피드백해주고 고칠 부분들은 뚝딱뚝딱 해결해주어서 이번 과제도 잘 마무리할 수 있었던 것 같습니다. 마지막 과제 다들 수고 많으셨습니다! 👍👍

Copy link

@hiwon-lee hiwon-lee 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 +19 to +23
const dynamicCandidateStyle =
ranking === 1
? { backgroundColor: "#384084", color: "#FFFFFF" }
: {};

Choose a reason for hiding this comment

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

1등일 경우에 색을 다르게 해준거 조아요~~

Choose a reason for hiding this comment

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

회원가입이니까 signup이라고 해주시면 좋을것같습니다.

Comment on lines +15 to +19
const { data } = useQuery({
queryKey: ["member"],
queryFn: member,
});

Choose a reason for hiding this comment

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

기존 과제들에서는 거의 로컬스토리지를 사용했었는데 이렇게 tanstack를 사용하니까 더 보기좋네요

Comment on lines +2 to +16
import { style } from "@vanilla-extract/css";

export const headerContainer = style([
pretendardSemiBold,
{
display: "flex",
justifyContent: "space-between",
alignItems: "center",
padding: "2rem 4rem",
boxSizing: "border-box",

width: "100%",
}
]);

Choose a reason for hiding this comment

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

styled-component와는 다르게 react에서 태그안에 스타일 넣는방식이랑 유사하네요. 태그의 인라인 스타일을 바로 추출해서 스타일 요소로 바꾸기 용이할것같습니다.

Copy link

@psst54 psst54 left a comment

Choose a reason for hiding this comment

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

마지막 과제 너무 고생하셨습니다~~~

스타일링을 신기한 방법으로 하셨네요! 어떻게 작동하는건가요??

그런데 저는 결과보기 페이지에서 오류가 나는 것 같아요...!

image

const accessToken = (await cookieStore).get("access")?.value;

if (!accessToken) {
throw new Error("어세스토큰 없다");
Copy link

Choose a reason for hiding this comment

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

😯

const accessToken = (await cookieStore).get("access")?.value;

if (!accessToken) {
console.log("어세스토큰 없다");
Copy link

Choose a reason for hiding this comment

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

😯😯

Comment on lines +17 to +32
{BUTTON_LIST.map((text) => {
if (text == "투표하기") {
return (
<Button onClick={onClick} variant="voting" key={text} text={text} />
);
} else {
return (
<Button
onClick={handleResult}
variant="result"
key={text}
text={text}
/>
);
}
})}
Copy link

Choose a reason for hiding this comment

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

이 부분은 BUTTON_LIST에 버튼 텍스트 말고도 onClick이나 variant 등도 함께 넣으면 코드 길이가 많이 줄어들 것 같아요!

return (
<div className={container}>
<Input
text="로그인"
Copy link

Choose a reason for hiding this comment

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

이부분은 아이디가 맞는 것 같아요!


export const metadata: Metadata = {
title: "CEOS 투표 서비스",
description: "엔브가 만들었어용!",
Copy link

Choose a reason for hiding this comment

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

카와잉

Copy link

Choose a reason for hiding this comment

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

저는 HeaderIsOrNot이라는 이름이 처음에 봤을 때 내용을 한 눈에 알기는 어려웠어요! 그냥 Header라고만 쓰셔도 좋지 않을가.. 합니다!

Copy link

@jiwonnchoi jiwonnchoi left a comment

Choose a reason for hiding this comment

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

안녕하세요! 엔브팀 코드리뷰를 맡은 커피딜 최지원입니다 :)
이번 과제에서 tanstack query 사용하신 점이 정말 인상 깊었고, 폴더 구조의 방식이 저희와 달라 새롭기도 했습니다👍
한 학기동안 과제하시느라 정말 고생많으셨습니다!! ❤️‍🔥

const mutation = useMutation({
mutationFn: signin,
onSuccess: async () => {
console.log("성공");

Choose a reason for hiding this comment

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

회원가입 뿐만 아니라 로그인이나 투표 등 모든 동작들에 대한 콘솔이 "성공" 이어서 콘솔을 지워주시거나, 남긴다면 "회원가입 성공"과 같은 식으로 구분해주면 더 좋을 것 같습니다!

Comment on lines +120 to +124
<ButtonContent
onClick={() => dispatch({ type: "SET_TEAM", payload: value })}
value={value}
key={value}
/>

Choose a reason for hiding this comment

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

소속 팀명이나 소속 파트를 선택했을 때 선택된 표시나 어떠한 차이가 나타나지 않아 클릭이 반영된 것인지 아닌지 구분이 잘 되지 않았습니다..! [selectedTeam, setSelectedTeam] 과 같이 useState를 두어 클릭 시에 setter로 선택된 팀을 지정하고 글자 색의 차이를 준다거나, 위에 보니 정의해두신 isTeam으로 선택이 되면 닫힘 효과를 주면 선택되었음을 알릴 수 있어 더 좋을 것 같습니다!

Comment on lines +34 to +35
pwValue: "",
pwcheckValue: "",

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.

저도 회원가입 로그인쪽 하면서 반복되는 부분들이 많았는데 이렇게 버튼도 세세히 컴포넌트화 하신 점 좋은 것 같아요~!

Comment on lines +15 to +24
const mutation = useMutation({
mutationFn: login,
onSuccess: async () => {
console.log("성공");
router.push("/");
},
onError: (error) => {
console.error(error);
},
});

Choose a reason for hiding this comment

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

로그인 여부를 login이라는 api 함수 호출을 통해 받아오시는 것 같은데, 제가 테스트해보았을 때는 [회원가입->로그인->로그아웃->로그인] 했을 때 아이디가 틀리거나 비밀번호가 틀려도 로그인이 되면서 올바른 닉네임이 떴습니다.....!.. 액세스토큰을 지우고서 기존에 회원가입 했던 올바른 아이디/비번으로는 로그인이 되는데, 틀린 아이디/비번으로는 로그인되지 않는 것으로 보아 저장된 액세스 토큰으로 판단하는 로직이 있으신건가,,도 생각해보았는데 정확한 파악은 안 된지라 여러 방식으로 테스트하여 백엔드와 이야기해보시면 좋을 것 같습니다....!!

Comment on lines +15 to +18
if (accessToken) {
console.log("Access Token", accessToken);
(await cookies()).set("access", accessToken);
}

Choose a reason for hiding this comment

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

로그인할 때마다 성공 시 백에서 액세스 토큰을 넣어보내주는 것으로 알고 있는데 콘솔 상에서는 한 번도 찍히는 것을 보지 못해서 언제 찍히는지 궁금합니다!

@@ -0,0 +1,24 @@
"use server";

Choose a reason for hiding this comment

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

next js에서는 기본적으로 서버 컴포넌트를 사용하는 것으로 알고 있는데 따로 써주신 이유가 있는지 궁금합니다!

@jinnyleeis
Copy link

jinnyleeis commented Jan 6, 2025

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

혹시 이부분 확인해보셨을까요? 파트장 투표가 성공적으로 진행된 후에도 항상
image
스크린샷 2025-01-06 오후 4 28 26
이와 같은 에러가 발생하는 것 같습니다. 데이터가 로드되지 않은 상태(즉 undefined)에서 .length를 호출하는 부분이 있는 것 같은데 이러한 경우에 대비해서 예외처리 등이 잘 되어있는지 이 부분 코드도 확인해보시면 좋을 것 같습니다!!

https://duklook.tistory.com/410

그리고
스크린샷 2025-01-06 오후 4 27 47
회원가입시 소속 팀명 / 소속 파트 별도로 선택된 텍스트에 대해서 색깔로 표시할 수 있으면 좋을 것 같습니다! 소속 팀명 선택한 상태에서 소속 파트를 선택하니, 선택된 소속 팀명은 색이 검정색으로 바뀌는 것 같아요 이부분도 수정할 수 있으면 좋을 것 같아요

받아온 팀 투표 결과에 문제가 있는 것 같은데 이부분도 확인해보시면 좋을 것 같아요!!
스크린샷 2025-01-06 오후 4 30 03

마지막 과제까지 수고 많으셨어요 ㅎㅎ

Copy link

@s-uxun s-uxun left a comment

Choose a reason for hiding this comment

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

안녕하세요, 마지막 주차 과제 코드 리뷰를 맡은 송유선입니다!
한 학기 동안의 스터디, 마지막 과제까지 너무 고생 많으셨어요! ✨
저도 한 학기 동안 엔젤브릿지 혜인언니, 가빈언니의 코드를 보면서 참 많이 배울 수 있었습니다. 앞으로 남은 프로젝트 개발도 파이팅 하시길 바라요 🔥

제가 더 많이, 잘 써드리고 싶었는데 제 로컬에서는 왜인지 모르게 빌드가 안 되더라고요 ㅜㅜ 이것저것 다시 설치해봐도 dev 실행이 안 되어서 이번 코드리뷰가 조금 짧게 되었어요.. 미안합니당....

"@tanstack/react-query": "^5.62.11",
"@tanstack/react-query-devtools": "^5.62.8",
"@tanstack/react-query-next-experimental": "^5.62.8",
"@vanilla-extract/css": "^1.17.0",
Copy link

Choose a reason for hiding this comment

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

vanilla-extract를 사용하는 방법은 처음 봤어요..! 이게 성능 최적화 및 타입 안정성을 중시하는 프로젝트에서 좋은 선택이 될 수 있다는 것을 새롭게 배우고 갑니다! 그런데 설정 과정이 복잡하고 동적 스타일 관리가 제한적이라는 단점도 있던데, 혹시 이번 프로젝트에서 선택하신 이유가 있을까요??

@@ -0,0 +1,26 @@
"use server";
Copy link

Choose a reason for hiding this comment

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

혹시 use server를 명시적으로 한 번 더 작성해주신 이유가 있을까요?? 안 써도 문제 없는 것으로 알고 있긴 한데, 쓰면 더 좋은 장점이 있는지 궁금합니다!

Comment on lines +24 to +29
<Button
onClick={handleResult}
variant="result"
key={text}
text={text}
/>
Copy link

Choose a reason for hiding this comment

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

로그인을 하지 않고 나서 이 로직을 통해서 결과 보기를 들어갈 경우에는 "Application error: a client-side exception has occurred (see the browser console for more information)." 와 같은 에러가 뜨고 있습니다! accessToken이 없는 상태에서 요청을 해서 그런 거 같기도 한데, 그렇다면 토큰이 있을 때만 해당 버튼을 노출시켜주시는 게 더 좋을 듯 합니다!

Comment on lines +20 to +42
export default function Container() {
const { data, isLoading, isError } = useQuery<Team[]>({
queryKey: ["teams"],
queryFn: teamlist,
});

const router = useRouter();
const [clickedTeam, setClickedTeam] = useState(0);

function handleClickTeam(teamId: number) {
setClickedTeam(teamId);
}

const mutation = useMutation({
mutationFn: teamvote,
onSuccess: async () => {
console.log("성공");
router.push("/demoresult");
},
onError: (error) => {
console.error(error);
},
});
Copy link

Choose a reason for hiding this comment

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

해당 부분을 실행했을 때, api 요청 및 응답에는 문제가 없었습니다! 다만 사용자 관점에서, 다소 의문스러운 부분이 있었어요. 제가 살펴본 결과, 현재 로그인한 유저가 자신에게 파트장 투표를 하거나, 자신의 팀에게 테모데이 투표를 하면 back에서 무효표로 처리하고 있는 것 같아요. (+1이 아니라 +0) 콘솔에서는 '성공'으로 찍히지만, 바로 다음 결과 화면에선 투표수에 변동이 없을 테니 사용자 입장에서는 오류가 났다고 느껴질 수 있을 듯 합니다. 이를 방지하려면 프론트단에서 사용자가 자신의 이름이나 팀을 선택하려고 할 경우 클릭을 막고 alert를 띄우는 등의 조치를 취해주시거나, 아니면 백과 협의해서 사용자가 자신의 이름이나 팀에 투표할 경우 에러를 반환하도록 수정한 뒤, 해당 오류 코드에 맞춰 프론트에서 안내 모달을 띄우는 방식을 추천드립니다!

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.

7 participants