8조 과제 제출 (정재현, 이은지, 문대현, 차동민) #6

cdm1263 · 2023-08-10T14:30:44Z

오늘은 내차례

📋 프로젝트 소개

개발 기간 : 2023. 07. 24 ~ 2023. 08. 10

배포 링크 :
오늘은 내차례
오늘은 내차례-관리자

관리자 계정 : [email protected] / abcd1234@

👥 개발 팀원 및 역할

팀원	정재현	이은지	차동민	문대현
깃허브
담당	- 사용자 인증 - Nav 바	- 메인 캘린더 컴포넌트	- 신청 모달 - 신청 현황 모달	- 관리자 페이지 - 승인, 반려

⚒️ 기술 스택 및 개발 환경

Development

Config

Deployment

Environment

Cowork Tools

로컬 스토리지 사용 가정 코드 작성

kledyu

안녕하세요! 훌륭하신 8조 과제를 코드 리뷰하게 되어 영광입니다!!

코드를 보면서 궁금했던 점 적어두었습니다 :-)
과제 하시느라 고생 많으셨습니다!!

kledyu · 2023-08-16T07:05:52Z

src/components/LoginForm.tsx

+        setUser({
+          username: loginResponse.data.username,
+          email: data.email,
+          imageUrl: loginResponse.data.imageUrl,
+          accessToken: loginResponse.data.accessToken,
+        });


로컬스토리지에도 로그인한 유저의 정보와 액세스토큰을 저장하는 방식이던데, useUserStore 훅을 사용하여 zustatnd store로 따로 관리하시는 이유가 궁금합니다!

로컬스토리지에도 로그인한 유저의 정보와 액세스토큰을 저장하는 방식이던데, useUserStore 훅을 사용하여 zustatnd store로 따로 관리하시는 이유가 궁금합니다!

안녕하세요. 코드 리뷰 감사합니다. 로컬스토리지에 유저 정보와 엑세스 토큰을 저장할 때, zustand store 의 persist middleware 를 이용해서 저장하는 방식을 사용하였습니다!

kledyu · 2023-08-16T07:09:05Z

src/components/LoginForm.tsx

+const imageMapping: { [key: string]: string } = {
+  "/src/assets/profile/0.png": image0,
+  "/src/assets/1.png": image1,
+  "/src/assets/2.png": image2,
+  "/src/assets/3.png": image3,
+  "/src/assets/4.png": image4,
+  "/src/assets/5.png": image5,
+  "/src/assets/6.png": image6,
+  "/src/assets/7.png": image7,
+  "/src/assets/8.png": image8,
+  "/src/assets/9.png": image9,
+  "/src/assets/10.png": image10,
+};


이미지맵핑을 LoginForm 컴포넌트에서 해야할 수 밖에 없는 필연적 이유가 있었을까요?

이미지맵핑을 LoginForm 컴포넌트에서 해야할 수 밖에 없는 필연적 이유가 있었을까요?

백엔드 api를 통해 이미지 파일에 대한 url 을 받아올 수 없는 상황이라, 임시적으로 모든 이미지 파일을 각 페이지에서 import 하고, 맵핑을 하였습니다. 백엔드 data를 사용하지 않고 로컬에 위치한 이미지 파일을 사용해야 됐는데, deploy 후에 발생하는 문제 해결을 위해서는 각각의 파일에서 import 하는 방법을 이용하였는데, 말씀 주셨듯이 따로 컴포넌트화 할 수 있는 방법을 알아보고, 중복을 줄일 수 있는 쪽으로 리팩토링 해보겠습니다!

kledyu · 2023-08-17T07:59:12Z

src/pages/SignUp/index.tsx

+    try {
+      const checkEmailResponse = await checkEmail(data.email);
+      // 이메일 중복 체크를 성공적으로 수행했고 중복된 이메일이 없다면 회원 가입 요청
+      if (checkEmailResponse.data.responseType === true) {
+        try {
+          const signUpResponse = await signUp(data.email, data.password, data.name);
+
+          if (signUpResponse.status === "success") {
+            alert(SIGNUP_MESSAGE.SIGN_UP_SUCCESS);
+            //로그인 페이지로 이동
+            navigate("/login");
+          } else {
+            alert(SIGNUP_MESSAGE.SIGN_UP_FAILED);
+          }
+        } catch (err) {
+          alert(SIGNUP_MESSAGE.SIGN_UP_ERROR);
+        }
+      } else {
+        alert(SIGNUP_MESSAGE.EMAIL_FAILED);
+      }
+    } catch (err) {
+      alert(SIGNUP_MESSAGE.EMAIL_ERROR);
+    }
+  };
+


중첩된 try catch문은 콜백지옥에 빠질 수도 있고 가독성이 나빠질 수도 있을 것 같다고 생각이 드는데, 이 부분 의견이 궁금합니다!

중첩된 try catch문은 콜백지옥에 빠질 수도 있고 가독성이 나빠질 수도 있을 것 같다고 생각이 드는데, 이 부분 의견이 궁금합니다!

좋은 의견 감사합니다. 이메일중복체크와 회원가입 과정을 두 개의 함수로 나누어 각각의 api 요청을 진행하여 중첩되지 않게 하면 좋을 것 같습니다 :)

kledyu · 2023-08-17T10:31:09Z

src/pages/SignUp/index.tsx

+            {...register("password", {
+              required: true,
+              minLength: 8,
+              maxLength: 15,
+              pattern: REG_EXP_PW_PATTERN,
+            })}


이 부분에서의 Length는 비밀번호의 최소 자리 수와 최대 자리 수를 나타낸 것이라면 REG_EXP_PW_PATTERN 정규식에 최소-최대 자리 수를 같이 검증할 수 있을 것 같은데 따로 지정하신 이유가 궁금합니다!

이 부분에서의 Length는 비밀번호의 최소 자리 수와 최대 자리 수를 나타낸 것이라면 REG_EXP_PW_PATTERN 정규식에 최소-최대 자리 수를 같이 검증할 수 있을 것 같은데 따로 지정하신 이유가 궁금합니다!

좋은 의견 감사합니다!! react form hook 에서 바로 적용할 수 있는 유효성 검사 부분을 먼저 적용한 뒤에, 정규식으로 추가적으로 필요한 부분을 넣으려고 했는데, 정규식 부분에서 최소-최대 자리수 부분이 중복되었네요! 우선 중복을 없애도록 하고, 한 곳에 몰아 넣는게 나아 보이네요. 다시 한 번 좋은 의견 감사드립니다! :)

min과 max를 지정해두면 대부분 브라우저에서 네이티브로 오류 처리가 가능하니, min과 max로 유지하시는 게 낫지 않을까 싶습니다.
안드로이드 기기에서 해당 attribute이 동작하지 않는 경우가 있으니, 주의는 필요합니다.

96uoow · 2023-08-17T12:30:00Z

src/components/calendar.tsx

+        exit={{ opacity: 0, x: 20 }}
+        transition={{ duration: 0.3 }}
+      >
+        <CustomFullCalendar


캘린더는 풀캘린더를 사용하셨는데, 다른 캘린더 라이브러리와 비교했을 장점이 있을까요? 저도 예전에 캘린더 사용할 때, 여러 라이브러리들을 봤었는데, 풀캘린더는 사용해보지 못해서 혹시 사용하시면서 체감하신 장점이 있다면 어떤 것이 있는지 궁금합니다!

저도 이번에 캘린더 라이브러리를 많이 고민했었는데, 기본적인 이벤트 기능이 많이 제공되었고 커스터마이징 하기에도 엄청 불편하거나 한점이 없어서 만족스럽게 사용한거 같습니다! 아래에 풀캘린더 공식문서의 여러기능이 정리되어있는 링크 남겨두겠습니다 다음에는 풀캘린더 한번 사용해보시길 추천드려요~
https://fullcalendar.io/docs

96uoow · 2023-08-17T12:37:05Z

src/components/common/Modal.tsx

+  $mediumModal?: boolean;
+  $h500Modal?: boolean;
+}
+


공통 모달 부분 아주 인상적으로 보았습니다... 다음 번에 공통 모달 만들어서 쓸 때는 저도 이런 방식으로 해봐야겠네요

공통적으로 사용하는 부분에 있어서는 최대한 재사용 가능한 컴포넌트로 만들어보고자 했습니다! 감사합니다~!

KIMKAEUN · 2023-08-20T11:36:14Z

src/lib/api/eventApi.ts

+  }
+};
+
+// 내 연차/당직 신청 현솽(GET)


오타를 찾았습니다.

KIMKAEUN · 2023-08-20T11:49:47Z

src/components/CharAnimation.tsx

키보드 화살키로 움직이는 캐릭터가 신기하고 귀여워서 한참 봤어요..! 기발한 생각인 것 같아요.

KIMKAEUN

안녕하세요. 8조 분들의 코드를 보고 많이 배우고 갑니다. 미니 프로젝트 기간동안 고생 많으셨고 파이널 프로젝트도 파이팅 입니다 :)

marshallku

고생하셨습니다!
DNS error로 라이브 데모 확인이 안되는 게 아쉽네요 😥

marshallku · 2023-08-22T13:04:22Z

.gitignore

+*.sln
+*.sw?
+
+.env


Posix 명세 준수를 위해, 파일은 항상 줄바꿈 문자로 끝내주세요!

marshallku · 2023-08-22T13:05:18Z

package.json

+    "@fullcalendar/core": "^6.1.8",
+    "@fullcalendar/daygrid": "^6.1.8",
+    "@fullcalendar/interaction": "^6.1.8",
+    "@fullcalendar/react": "^6.1.8",
+    "@fullcalendar/timegrid": "^6.1.8",


라이브러리가 좀 무거울텐데, 나중에 dynamic import 등 기법으로 최적화해보셔도 좋을 것 같아요.

멘토님! 제안해주신대로 동적임포트에 대해 찾아보니 React.lazy를 route경로쪽에서 사용하라고 하여 아래처럼 수정해보았는데 이런 방식이 맞을까요?
아니면 calendar 컴포넌트에서 사용해야할까요..? 처음 사용해보는것이라 헷갈려서 질문드립니다ㅠㅠ

//App.tsx import { Suspense, lazy } from "react"; import { Route, Routes, Navigate } from "react-router-dom"; import Login from "./pages/Login"; import SignUp from "./pages/SignUp"; const Main = lazy(() => import("./pages/Main")); const App = () => { return ( <> <Routes> <Route path="/" element={ <Suspense fallback={<div>Loading...</div>}> <Main /> </Suspense> } /> <Route path="/login" element={<Login />} /> <Route path="/signup" element={<SignUp />} /> <Route path="*" element={<Navigate to="/" replace />} /> </Routes> </> ); }; export default App;

marshallku · 2023-08-22T13:09:16Z

src/components/AddModal.tsx

+import DatePickerComponent from "./DatePicker";
+import useDateStore from "../store/dateStore";
+import { AddEvent, addEvent } from "../lib/api/eventApi";
+import { MODAL_MESSAGE, EVENT_TYPE, TAB_ADD } from "../lib/util/constants";


util에는 유틸리티 함수만 모아두고, 상수는 constants에 넣어둬도 괜찮지 않을까요?
상수를 따로 모아서 관리하신 점은 좋아 보입니다!

marshallku · 2023-08-22T13:09:35Z

src/components/AddModal.tsx

+import ModalTitle from "./common/ModalTitle";
+import { useState } from "react";
+import { css } from "styled-components";
+import { theme } from "../styles/theme";


alias와 barrel file에 대해 찾아보시면 import문을 조금 정리할 수 있을 것 같아요.

초기에 경로 별칭에 대해서 배웠는데 시간이 지나면서 그새 잊어버렸습니다 ㅜㅜ 다시 공부해서 고쳐보겠습니다!

marshallku · 2023-08-22T13:13:28Z

src/components/AddModal.tsx

+
+  const showNotification = (startDate: Date, endDate: Date | null) => {
+    notification.info({
+      message: `${selected === TAB_ADD[0] ? selected + "가" : selected + "이"} 신청되었습니다.`,


const hasFinalConsonant = (word: string) =>(word[word.length - 1].charCodeAt(0) - parseInt('ac00',16)) % 28 > 0;

이런 간단한 함수로 종성 포함 여부를 알아낼 수 있어요.

마지막 글자를 유니코드로 변환해 종성 여부를 확인하는 방법이 있다니...! 다음에는 꼭 유용하게 써보겠습니다 감사합니다!

marshallku · 2023-08-22T15:28:23Z

src/lib/util/functions.ts

+
+// 두 날짜의 기간을 계산하는 함수
+const calcPeriods = (start: Date, end: Date) => {
+  const oneDay = 24 * 60 * 60 * 1000;


이왕 숫자 곱으로 표현하시려면, export const DAY_TO_HOUR = 24;처럼 선언해두시면 여기저기서 사용할 수 있지 않을까요?

24 * 60 * 60 * 1000 를 공용 변수로 만들어서 사용하자는 말씀으로 이해했는데 혹시 맞을까요??!

marshallku · 2023-08-22T15:34:25Z

src/pages/Login/index.tsx

+  display: flex;
+  justify-content: center;
+  align-items: center;
+  height: 100vh;


vh 단위는 모바일에서 주소창 등을 포함해서 계산되기에, 사용에 주의가 필요합니다.

marshallku · 2023-08-22T15:37:30Z

src/pages/SignUp/index.tsx

+            {...register("password", {
+              required: true,
+              minLength: 8,
+              maxLength: 15,
+              pattern: REG_EXP_PW_PATTERN,
+            })}


min과 max를 지정해두면 대부분 브라우저에서 네이티브로 오류 처리가 가능하니, min과 max로 유지하시는 게 낫지 않을까 싶습니다.
안드로이드 기기에서 해당 attribute이 동작하지 않는 경우가 있으니, 주의는 필요합니다.

marshallku · 2023-08-22T15:39:32Z

src/store/calendarState.ts

@@ -0,0 +1,19 @@
+import { create } from "zustand";
+
+type TabState = "전체" | "연차" | "당직" | string;


이렇게 union을 선언하시면 type TabState = string;이 되어버립니다.

이부분 리팩토링할때 수정하겠습니다!

marshallku · 2023-08-22T15:40:21Z

src/styles/theme.ts

+    },
+    black: "#333",
+    white: "#fff",
+    gray: ["#808080", "#b9b9b9", "#F1F1EF"],


gray도 array보다 object를 사용하시는 게 조금 더 명시적이지 않았을까요?

과제 제출

b3ce1d1

cdm1263 changed the title ~~8조 과제 제출~~ 8조 과제 제출 (정재현, 이은지, 문대현, 차동민) Aug 10, 2023

kse-seong-eun pushed a commit that referenced this pull request Aug 11, 2023

Merge pull request #6 from FastCampusGroupFE9/AhnTaeUk/Auth

feba4b4

로컬 스토리지 사용 가정 코드 작성

cdm1263 added 2 commits August 11, 2023 13:37

수정

110c8c3

수정2

cac0128

cdm1263 self-assigned this Aug 11, 2023

cdm1263 added 3 commits August 11, 2023 16:14

수정3

b204510

수정5

8246e8d

수정6

8da2c69

This comment was marked as resolved.

Sign in to view

kledyu reviewed Aug 17, 2023

View reviewed changes

96uoow reviewed Aug 17, 2023

View reviewed changes

KIMKAEUN reviewed Aug 20, 2023

View reviewed changes

src/lib/api/eventApi.ts

}

};

// 내 연차/당직 신청 현솽(GET)

Copy link

KIMKAEUN Aug 20, 2023

Choose a reason for hiding this comment

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

오타를 찾았습니다.

KIMKAEUN reviewed Aug 20, 2023

View reviewed changes

marshallku reviewed Aug 22, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

8조 과제 제출 (정재현, 이은지, 문대현, 차동민) #6

8조 과제 제출 (정재현, 이은지, 문대현, 차동민) #6

cdm1263 commented Aug 10, 2023 •

edited by dhmoon11

Loading

This comment was marked as resolved.

kledyu left a comment •

edited

Loading

kledyu Aug 16, 2023

iskra17 Aug 18, 2023

kledyu Aug 16, 2023

iskra17 Aug 18, 2023

kledyu Aug 17, 2023

iskra17 Aug 18, 2023

kledyu Aug 17, 2023

iskra17 Aug 18, 2023

marshallku Aug 22, 2023

96uoow Aug 17, 2023

dmswl2030 Aug 18, 2023

96uoow Aug 17, 2023

cdm1263 Aug 23, 2023

KIMKAEUN Aug 20, 2023

KIMKAEUN Aug 20, 2023

KIMKAEUN left a comment

marshallku left a comment

marshallku Aug 22, 2023

marshallku Aug 22, 2023

dmswl2030 Aug 24, 2023 •

edited

Loading

marshallku Aug 22, 2023

marshallku Aug 22, 2023

cdm1263 Aug 23, 2023

marshallku Aug 22, 2023

cdm1263 Aug 23, 2023

marshallku Aug 22, 2023

cdm1263 Aug 23, 2023

marshallku Aug 22, 2023

marshallku Aug 22, 2023

marshallku Aug 22, 2023

dmswl2030 Aug 24, 2023

marshallku Aug 22, 2023

+              *.sln
+              *.sw?
+              .env

		@@ -0,0 +1,19 @@
		import { create } from "zustand";

		type TabState = "전체" \| "연차" \| "당직" \| string;

8조 과제 제출 (정재현, 이은지, 문대현, 차동민) #6

Are you sure you want to change the base?

8조 과제 제출 (정재현, 이은지, 문대현, 차동민) #6

Conversation

cdm1263 commented Aug 10, 2023 • edited by dhmoon11 Loading

오늘은 내차례

📋 프로젝트 소개

👥 개발 팀원 및 역할

⚒️ 기술 스택 및 개발 환경

Development

Config

Deployment

Environment

Cowork Tools

This comment was marked as resolved.

kledyu left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

KIMKAEUN left a comment

Choose a reason for hiding this comment

marshallku left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dmswl2030 Aug 24, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cdm1263 commented Aug 10, 2023 •

edited by dhmoon11

Loading

kledyu left a comment •

edited

Loading

dmswl2030 Aug 24, 2023 •

edited

Loading