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조 과제 제출 (양준용, 김진우, 김세연) #11

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

azure0929
Copy link

@azure0929 azure0929 commented Aug 11, 2023

'RE:POST'

Rest + Post : 휴식(연차)에 선택의 유연함을 더하다


연차 당직 관리 시스템을 보다 편리하게 관리하기 위해 해당 서비스를 구현하게 되었습니다.

일반 연차 / 당직 관리 시스템의 차별화된 기능으로는 연차 및 당직 사유를 작성할 수 있고, 관리자가 신청 및 거절을 할 수 있어 보다 체계적인 관리가 가능하고, 특정 사원의 직급 수정 및 잔여 연차를 수정 할 수 있는 서비스를 제공하고자 하였습니다.


• 사용자 배포 주소: I5E1-Client

• 관리자 배포 주소: I5E1-Admin

• FE-사용자 깃허브 주소 Client

• FE-관리자 깃허브 주소 Admin


• BE-사용자 깃허브 주소 Client

• BE-관리자 깃허브 주소 Admin






🗓 프로젝트 기간: 2023.07.24 ~ 2023.08.11


🧔 개발팀

양준용 김진우 김세연
양준용 김진우 김세연

로그인
회원가입
관리자 페이지
-연차/당직관리 부분 승인 및
거절 처리 상태 적용
-사원관리 부분 수정 기능 적용,
검색 결과에 따른 페이지 적용

메인 페이지
마이 페이지
관리자 페이지
-연차/당직관리
-사원관리
-로그아웃

기술 스택

Development

Config

Environment

Cowork Tools



💻 프로젝트 테스트


Clone Project

$ git clone [email protected]:I5E1/I5E1-FE.git
$ git clone [email protected]:I5E1/I5E1-FE-ADMIN.git

Go to Project

$ cd I5E1-FE
# cd I5E1-FE-ADMIN

Install

$ npm i

Start Project

$ npm run dev



🙏 감사합니다

@azure0929 azure0929 self-assigned this Aug 11, 2023
}
}}
>
{Array(parseInt((totalCount / 10 + 1).toString()))
Copy link

Choose a reason for hiding this comment

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

Pagination을 라이브러리를 쓰지 않고 직접 구현하신 부분이 인상깊습니다!
페이지리스트 만드실 때 useMemo등을 사용해서 따로 변수로 빼서 썼다면 좀 더 가독성이 좋을 것 같습니다!

return (
<Contain>
<Duty_Table />
<Pagination>
Copy link

Choose a reason for hiding this comment

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

Pagination 관련 코드가 반복적으로 다른 컴포넌트에서도 사용하고 계신 것 같은데 하나의 컴포넌트로 빼서 공통으로 재사용 할 수 있도록 컴포넌트를 만들면 더 좋을 것 같습니다!

const { method, data, path = '' } = req.body as RequestBody

const { data: responseValue } = await axios({
url: `https://asia-northeast3-heropy-api.cloudfunctions.net/api/todos/${path}`,
Copy link

Choose a reason for hiding this comment

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

해당 파일명이 todos인 이유가 따로 있을까요?
그리고 서버 URL도 환경변수로 같이 관리하면 더 괜찮지 않을까 싶습니다!

@@ -0,0 +1,56 @@
/** @type {import('tailwindcss').Config} */
Copy link

Choose a reason for hiding this comment

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

tailwind와 styled-components를 같이 사용하신 이유가 있을까요?

}

// 연차 데이터 요청
export const useAnnualStore = create<annualStore>()((set) => ({
Copy link

Choose a reason for hiding this comment

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

userStore는 따로 분리하셨는데 해당 파일의 연차와 당직은 분리하지 않을 이유가 있으신가요?
협업 프로젝트이기 때문에 하나의 규칙으로 통일하면 더 좋을 것 같습니다!

Copy link

@EungBug EungBug left a comment

Choose a reason for hiding this comment

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

짧은 기간동안 미니프로젝트 진행 하시느라 고생 많으셨습니다!!
궁금한 부분들과 저만의 작은 의견들을 조심스럽게 코멘트 남겨봤습니다!
수고 많으셨습니다👍 파이널 프로젝트에서도 다들 파이팅하세요!

Copy link
Member

@marshallku marshallku left a comment

Choose a reason for hiding this comment

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

고생 많으셨습니다!

Copy link
Member

Choose a reason for hiding this comment

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

.gitignore에 추가되기 전에 파일이 커밋된 것 같네요~

<Pagination>
<PageUl
onClick={(e) => {
if (e.target instanceof HTMLLIElement) {
Copy link
Member

Choose a reason for hiding this comment

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

event.currentTarget에 대해 알아보셔도 좋을 것 같습니다!

}
}}
>
{Array(parseInt((totalCount / 10 + 1).toString()))
Copy link
Member

Choose a reason for hiding this comment

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

Magic number는 가독성을 해칩니다.
이 부분을 Pagination이라는 컴포넌트로 따로 빼고, 파일 최상단에 const ITEMS_PER_PAGE = 10;과 같이 상수로 지정해두고, props로 해당 값을 기본 값으로 사용하는 변수를 받았더라면, 조금 더 확장성 있는 설계가 가능했을 것 같네요.

{Array(parseInt((totalCount / 10 + 1).toString()))
.fill(0)
.map((i, index) => (
<PageLi key={index} value={i}>
Copy link
Member

Choose a reason for hiding this comment

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

livalue를 0으로 반복해서 넣는 이유가 있을까요?

Comment on lines +36 to +43
<PageBtn
onClick={() => {
setOffset(index)
readAnnual(index + 1)
}}
>
{index + 1}
</PageBtn>
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 +56 to +59
const validateEmail = (email: string) => {
const isValid = /^[^\s@]+@[^\s@]+\.[^\s@]+$/.test(email)
return isValid
}
Copy link
Member

Choose a reason for hiding this comment

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

별도 utility로 만들어두면, 관련 로직이 필요한 다른 곳에서 쉽게 가져다 쓸 수 있지 않을까요?

const response = await login({ email, password })

const { data: resdata, status } = response // response에서 data, status 꺼내올 것이다라는 의미
const { token } = resdata.data // 위의 코드에서 구조분해할당 세번째
Copy link
Member

Choose a reason for hiding this comment

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

const { data: { token }} = resdata처럼도 쓰실 수 있습니다.

Comment on lines +25 to +53
const handleSignUp = useCallback(
async (e: FormEvent) => {
e.preventDefault()

if (!username || !email || !tel || !password || !confirmPassword) {
alert('가입에 필요한 정보를 입력해주세요.')
return
}

if (!validateEmail(email)) {
alert('유효하지 않은 이메일 형식입니다.')
return
}

if (!validatePassword(password)) {
alert('비밀번호는 최소 6자 이상이어야 합니다.')
return
}

if (!validateTel(tel)) {
setError('잘못된 전화 형식입니다. 01XXXXXXXXX 형식을 사용하세요.')
return
}

if (password !== confirmPassword) {
setError('비밀번호가 일치하지 않습니다!')
setPasswordsMatch(false)
return
}
Copy link
Member

Choose a reason for hiding this comment

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

form validation 관련 로직도 공통화해 hook으로 제작하거나, react hook form 등을 사용해 간결하게 표현하실 수 있습니다.

<input
className="text-base font-normal border w-[384px] px-5 py-2.5 rounded-md focus:outline-none focus:border-primary"
type="password"
placeholder="최소 4자 이상 입력해주세요."
Copy link
Member

Choose a reason for hiding this comment

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

minlength attribute도 추가하시면 어떨까요?

@@ -0,0 +1,7 @@
export type CalendarDate = {
Copy link
Member

Choose a reason for hiding this comment

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

interface와 type의 차이에 관해 알아보시면 좋을 것 같습니다.
여기서만 굳이 type이 사용될 특별한 이유가 없어 보이네요.

Copy link

@wngkfla01 wngkfla01 left a comment

Choose a reason for hiding this comment

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

저희조와 비슷한듯 다른듯한 코드를 보는게 무척 흥미로웠습니다! 고생 많으셨어요 💪🏻

<TableArea>
<NameArea>
<No>No</No>
<Name>사원명</Name>

Choose a reason for hiding this comment

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

태그명을 직관적으로 지으셨네요! 헷갈리지 않을 것 같아 좋아보여요 :)

Comment on lines +104 to +105
width: 100vw;
height: 100vh;

Choose a reason for hiding this comment

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

정보 저장을 위해 로컬스토리지를 사용하셨군요! 저희조도 zustand로 상태관리를 했는데, 새로고침 시 상태를 유지하기위해 zustand에 내장되어있는 기능인 'persist'를 사용했어요. 따로 설치할 필요가 없었고 사용법도 매우 간단했기 때문에, 굉장히 편하게 사용했던 기억이 있네요. 강력 추천 드립니다👍🏻👍🏻

data: newDuty
}
})
console.log(res)

Choose a reason for hiding this comment

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

내가 짠 코드가 잘 동작하는지 알기 위해서 콘솔을 마구마구(?) 찍는게 필수이지만, 저도 확인 후 지우는걸 종종 까먹어요👶🏻
저의 경우 최종 PR전에 조원들이랑 다같이 화면공유를 하면서 전체검색을 통해 콘솔을 다 지웁니다..ㅎㅎ
최종검수 하실때 '콘솔 지우기'를 체크리스트에 넣으시는건 어떨까요?👋🏻

@HyunJunPark0
Copy link

너무 잘 하셔서 보면서 많은 공부 됐습니다!!!!😁😄

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