-
Notifications
You must be signed in to change notification settings - Fork 1
커스텀 캘린더 구현 #144
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
base: develop
Are you sure you want to change the base?
커스텀 캘린더 구현 #144
Conversation
* chore: dev 서버 cd 구축 * fix: 브랜치 main에서 dev로 수정 * Update .github/workflows/frontend-dev-deploy.yml Co-authored-by: Luna <[email protected]> * chore: 개발환경, 운영환경 환경변수 다르게 적용되도록 설정 --------- Co-authored-by: Luna <[email protected]>
* chore: sentry 세팅 * chore: gitignore에서 sentry config file 제거 * chore: prettierignore에서 sentry config file 제거 * fix: sentry config project name 수정 및 automaticVercelMonitors 삭제, deleteSourcemapsAfterUpload 추가 * feat: global-error 추가 * feat: withSentryConfig에 authToken 추가 * fix: sentry.edge.config.ts 제거 * chore: global-error log 제거
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
프룬 고생하셨어요~~
덕분에 기본 캘린더를 제공하지 않고 예쁜 캘린더를 제공할 수 있게 되었네요! (편안)
PR 올라오고 변경사항을 확인했는데 라이브러리 의존성이 안 바뀐 것을 보고 헉 이걸 직접 구현했다고? 놀랐어요..!
제 기준으로는 디자인 깔끔하게 잘 나온 것 같은데 토다리와 루나는 어떻게 생각하실지 궁금하네요..! 저는 디자인 이대로도 좋습니다.
논의점에 대한 제 생각을 달아볼게요.
-
설계 저는 적절한 것 같아요. 캘린더 컴포넌트를 사용하는 입장에서 value와 onChange만 넣어주면 기본 동작이 되니깐 잘 설계하신 것 같아요.
근데 useEffect 의존성을 확인해보려고 했는데 useEffect가 click outside 말곤 없어서 문제 없는 것 같아요. -
날짜 범위가 추가되어도 저는 크게 문제가 되지 않는다고 생각해요. 날짜 범위를 선택한다고 하면 시작과 끝을 입력할텐데, 그래도 현재 보이는 날짜들 안에서 선택될테고, 달을 넘기는 것은 기존 동작과 동일하니깐 잘 설계되지 않았나 생각합니다.
다만 범위선택을 하려면 value가 (state, end) 두 개 들어와야 할 것 같은데 이건 지금 요구사항은 아니니깐..! -
prop은 지금 당장의 요구사항에서는 충분하다고 봐요. 언제 이전, 언제 이후로는 선택이 불가능해야한다. 이런 요구사항이 있다면 추가되면 좋은데 지금은 간단하게 하루 선택을 하는 것 밖에 없으니 충분하다고 생각합니다!!
고생하셨습니당~ 추가로 UI 제안하고 싶은 점 2가지 있는데 별도 코멘트로 남길게요
value: Date | null; | ||
className?: string; | ||
placeholder?: string; | ||
invalid?: boolean; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
invalid는 어떤 역할을 하나요?
사실 prop을 true로 넣고 실행해봤는데 아무 변화가 없어서요ㅜㅜ
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
}; | ||
}, []); | ||
|
||
const handleDateChange = (day: number) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
handleDateChange를 calendarDays 안에서 선언해서 사용해도 좋을 것 같아요.
calendarDays에서만 사용되는 메서드이고 아래 useMemo의 의존성 배열 경고로 이런 메시지가 나오는데
React Hook useMemo has a missing dependency: 'handleDateChange'. Either include it or remove the dependency array.eslintreact-hooks/exhaustive-deps
calendarDays 안에 넣으면 해결될 것 같아요
|
||
const changeMonth = (offset: number) => { | ||
setDisplayDate(prev => { | ||
const newDate = new Date(prev.getFullYear(), prev.getMonth() + offset, 1); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
offset으로 받아서 전 달의 -1, +1 해주어서 계산하는 방식 좋은 것 같아요!
const newDate = new Date(displayDate.getFullYear(), displayDate.getMonth(), day); | ||
|
||
if (value && formatDate(newDate) === formatDate(value)) { | ||
onChange(null); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이 부분 궁금해요!
현재 선택된 날짜를 다시 선택할 때 선택을 해제하도록 구현하신 이유가 궁금합니다~
const days: JSX.Element[] = []; | ||
|
||
for (let i = 0; i < firstDayOfMonth; i++) { | ||
days.push(<div key={`prev-${i}`} className="h-10 w-10" />); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
전 달 영역이나 다음 달 영역을 클릭했을 때 달이 이동되어도 좋을 것 같아요.
단 이건 전 달 영역과 다음 달 영역이 빈 칸이 아니라 날짜가 보여져야 될 것 같긴 하지만..!
이건 번거로우시면 스킵해도 좋습니다! 지금도 충분히 좋아서요
const selectedClasses = isSelected | ||
? 'bg-primary text-white font-bold bg-primary-primary' | ||
: 'text-grayscale-700 active:bg-primary-container md:hover:bg-primary-container'; | ||
const finalClasses = `${baseClasses} ${isSelected ? selectedClasses : `${todayClasses} ${selectedClasses}`}`; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
final이라서 위의 모든 경우에 해당이 안되는 경우인가 생각했었는데 보니깐 base부터 selected까지 모두 통합한 변수였군요
type="text" | ||
value={value ? formatDate(value) : ''} | ||
placeholder={placeholder} | ||
readOnly |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
키보드로 날짜 입력을 막고 클릭으로만 날짜를 선택하기 위해 readOnly를 설정하신거죠? 좋습니다
</button> | ||
</div> | ||
|
||
<div> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이 div 태그는 어떤 역할을 하나요? 아무 css 속성이 적용되어있지 않아서 질문드립니다.
@@ -0,0 +1,6 @@ | |||
export const formatDate = (date: Date): string => { | |||
const year = date.getFullYear(); | |||
const month = String(date.getMonth() + 1).padStart(2, '0'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
01, 02 디테일 넘 좋습니다~
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
저는 month가 항상 0부터 시작해서 헷갈리는데 프룬은 구현하면서 안 헷갈리셨을지 궁금해요ㅋㅋㅋㅋ
UI 관련 제안
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
우선 디자인/사용성 피드백 먼저 남깁니다! (코드에 대한 코멘트는 추가로 남길게요)
-
일요일부터 시작하는 달력 익숙해서 아주 편안하고 좋았습니다
☺️
주말도 각각 파랑 빨강으로 색깔처리해서 구분하기 편했고, 요일과 날짜의 색상도 그레이스케일로 톤 다르게 해준 디테일도 좋았어요!! -
모바일 텍스트 크기
모바일에서 연도와 달이 작게 느껴져서 text-md
로 키우면 좋을 것 같아요!
추가 제안(반영 안하셔도 됩니다!!)
- 연도와 달(h2 태그)을 클릭했을 때 오늘 날짜로 이동되는 기능이 있으면 편할 것 같아요.
- 이건 고민되는 부분인데, 이미 지나간 일정에 대해 타임라인을 남긴다고 가정하면, 달력에서 오늘을 기준으로 한 이후의 달(ex. 오늘이 10월 12일이면 11월부터 접근 불가하도록)은 선택되지 않게 만들어야하나? 생각이 드네요🤔
- 날짜 직접 입력하기

기획 회의 때 날짜(value)가 보여지는 부분을 사용자가 직접 입력하는걸 제한하기로 했던 것 같아요. 사용자들이 날짜 입력 포맷을 25-01-01, 250101, 25.01.01
등등 다양하게 입력할 것 같다는 이유로 일부러 달력에서 클릭으로만 하도록 논의한 것으로 기억하는데요!
지금처럼 YYYY.MM.YY 형태로 고정된다면 사용자가 입력했을 때 .
을 기준으로 자동 포맷되는 기능이 있어도 좋을 것 같다는 생각을 해봤습니다ㅋㅋㅋ
저도 커스텀 캘린더 구현해봤을 때 신경쓸게 한 두가지가 아니라 머리가 복잡했거든요😂 예쁘게 구현해주셔서 감사하고 잘 가져다 쓰겠습니다..ㅎㅅㅎ
이 캘린더 좀 더 다듬어서 npm에 배포해보시면 어떤가요?ㅋㅋㅋㅋ
@@ -0,0 +1,6 @@ | |||
export const formatDate = (date: Date): string => { | |||
const year = date.getFullYear(); | |||
const month = String(date.getMonth() + 1).padStart(2, '0'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
저는 month가 항상 0부터 시작해서 헷갈리는데 프룬은 구현하면서 안 헷갈리셨을지 궁금해요ㅋㅋㅋㅋ
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
코드 확인했습니다! 몇가지 변경하면 좋을 점 코멘트 남겼는데 확인 부탁드려요!
캘린더 직접 만든다고 고생 많으셨어요~~🗓️💞
import {JSX, useEffect, useMemo, useRef, useState} from 'react'; | ||
import {twMerge} from 'tailwind-merge'; | ||
|
||
type CustomCalendarProps = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
저희 props 타입을 선언할 때는 interface
를 사용하기로 컨벤션을 정했던 것 같아요!
const month = displayDate.getMonth(); | ||
const firstDayOfMonth = new Date(year, month, 1).getDay(); | ||
const lastDateOfMonth = new Date(year, month + 1, 0).getDate(); | ||
const days: JSX.Element[] = []; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
여기에 JSX.Element[]
타입을 추가한 이유는 무엇인가요??
<button | ||
onClick={toggleCalendar} | ||
className="absolute inset-y-0 right-0 flex items-center pr-3" | ||
aria-label="달력 열기" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
접근성까지!!🚀🔥🔥 요새 미션으로 배운 내용이라서 반갑네요ㅋㅋㅋㅋ
const baseClasses = 'flex items-center justify-center h-10 w-10 text-sm rounded-full cursor-pointer select-none'; | ||
const todayClasses = isToday ? 'font-bold bg-grayscale-50' : ''; | ||
const selectedClasses = isSelected | ||
? 'bg-primary text-white font-bold bg-primary-primary' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
? 'bg-primary text-white font-bold bg-primary-primary' | |
? 'text-white font-bold bg-primary-primary' |
클래스가 중복으로 들어가 있어서 확인해보니 bg-primary
를 삭제해도 제대로 보이네요!
issue
구현 사항
외부에서 전달받는 value와 내부 표시용 displayDate가 있습니다. displayDate는 value로 초기화되고, 이전 달/다음 달 버튼을 클릭하면서 displayDate가 업데이트 됩니다.
displayDate를 기준으로 화면에 보여질 개별 날짜 (days)가 만들어집니다.
firstDayOfMonth
는 매달 1일이 시작되기 전 공백을 넣어주기 위한 변수이고,firstDayOfMonth
와lastDateOfMonth
값에 맞춰 각각의 for문을 돌며 days를 채웁니다.선택된 날짜를 다시 클릭하면 값이 초기화됩니다.
외부 영역을 클릭하면 캘린더가 닫힙니다.
X.mp4
default.mp4
default.mp4
중점적으로 리뷰받고 싶은 부분(선택)
onChange
를 진행하는 흐름을 의도하며 설계했습니다. 이때 month를 변경하면서 displayDate가 함께 업데이트 되며 UI가 동기화됩니다. 이 로직이 적절한지와 useEffect 의존성 배열이 현재 로직에서 최선인지, 더 나은 방법은 없을지 조언을 구합니다!CustomCalendar
의 prop이 적절한지🫡 참고사항
디자인 변경하고 싶은 부분이 있다면 자유롭게 말씀해주세요~!!!!격한 환영!!!!!