[FE] 런칭데이에서 받은 달력 UI 피드백 반영 #346

hwinkr · 2024-09-15T08:12:22Z

작업 내용

오늘을 표시하는 방법을 오늘 텍스트로 변경

기존에는 초록색 점을 활용해서 오늘 인것을 표현했었습니다. 런칭데이 피드백을 받으면서,

아래 초록색 점으로 인해서 오늘이 이미 선택된 줄 알았어요.

의 내용이 있었습니다. 해결 방법을 함께 논의하면서 오늘 텍스트로 변경하자는 의견으로 결정되었습니다.

const DATE_INFO_TEXTS = {
  today: '오늘',
  rangeStart: '시작',
  rangeEnd: '끝',
  default: '\u00A0',
};

날짜의 추가 정보를 알려주는 텍스트를 상수화 한 객체를 정의했습니다.

export default function DateExtraInfo({
  isToday,
  isRangeStart,
  isRangeEnd,
  holidayName,
}: DateInfoProps) {
  return (
    <span css={s_dateExtraInfoText}>
      {getDateInfoText({ isToday, isRangeStart, isRangeEnd, holidayName })}
    </span>
  );
}

그리고 날짜 추가 정보를 텍스트로 알려주는 DateExtraInfo 컴포넌트를 구현했어요.

이제 사용자는 오늘이 어떤 날짜인지를 더 확실하게 인지할 수 있게 되었어요.

holidays-kr 라이브러리를 도입하여 실제 대한민국 공휴일을 표시하는 기능 추가

런칭데이 피드백 중,

공휴일도 함께 표시되었으면 좋겠어요.

피드백이 있었습니다. 실제로 저희 서비스에 약속 주최자가 접속해서 약속을 생성할 때, 어떤 날이 공휴일인지를 함께 알려주면 다른 서비스의 달력을 확인하러 가지 않아도 서비스 내부에서 약속 후보 날짜들을 결정할 수 있게 됩니다.
다른 서비스, 모모 서비스를 왔다갔다 하면서 날짜를 비교하는 것은 사용자 입장에서 불편하다고 생각되어 반영할 만한 가치가 있는 피드백이라 판단했어요. 자바스크립트 Date 객체는 공휴일 정보까지 제공해주지 않기에 일단 holidays-kr 라이브러리를 활용해서 공휴일을 빠르게 그릴 수 있더록 했어요!

시작 날짜, 끝 날짜만 선택하여 연속된 날짜를 편하게 선택할 수 있도록 하는 기능 추가

런칭데이 피드백 중 아래와 같은 피드백도 있었습니다.

긴 범위의 연속된 날짜를 선택할 때, 하나씩 다 눌러야 하니 불편해요...ㅠ

그래서, 시작과 끝 날짜만 클릭하면 사이의 모든 날짜가 선택되도록 하는 해결책을 떠올렸고 서비스에 적용해 보기로 했어요.

const useDateSelect = () => {
  const [selectedDates, setSelectedDates] = useState<string[]>([]);
  const [dateSelectMode, setDateSelectMode] = useState<DateSelectMode>('single');
  const [rangeStartDate, setRangeStartDate] = useState<string | null>(null);
  const [rangeEndDate, setRangeEndDate] = useState<string | null>(null);

useDateSelect 훅은 아래와 같은 책임을 가집니다.

선택된 날짜들을 상태로 관리
하나씩 선택하는 모드인지, 범위로 선택하는 모드인지 상태로 관리
범위로 선택할 경우의 시작, 끝 날짜를 상태로 관리
시작, 끝 날짜가 모두 선택될 경우 그 사이 모든 날짜를 구하고 상태에 반영

추가로, 시작 날짜를 선택한 후 끝 날짜를 선택했는데 끝 날짜가 시작 날짜보다 이른 경우는 해당 끝 날짜가 다시 시작 날짜가 되도록 예외 처리를 했어요. 그리고 시작과 끝 날짜를 더 확실하게 표현해줄 수 있는 UI를 고민하던 중 오른쪽 왼쪽 삼각형을 활용해서 시작과 끝 사이에 특정 날짜들이 포함된다는 컨텍스트를 쉽게 제공할 수 있도록 해줬습니다. 그리고 삼각형 UI도 부족할 수 있을 것 같아서, DateExtraInfo 컴포넌트를 활용해서 시작, 끝 텍스트를 추가해줬어요.

특이 사항

달력 컴포넌트 구조 변경

feat: 재사용할 수 있는 달력 공통 컴포넌트 구현을 보면 확인할 수 있겠지만, 달력 컴포넌트 구조를 변경했어요. render props, Context API + 합성 컴포넌트 패턴을 활용해서 조금 더 유연하게 달력 컴포넌트를 구현했어요. 최대한 날짜 관련 데이터만 다루는 책임만 가지고 마크업, 디자인, 달력 내부에서 일어날 수 있는 사용자의 행동과 관련된 요소들은 외부에서 결정할 수 있도록 해봤습니다. 아마 여기서 조금만 더 재사용성을 고려해 본다면 @Largopie 가 구현해야 하는 날짜만 선택하는 약속 기능에도 재사용할 수 있을 것 같아요.

스토리북 에러 임시 해결

변경된 달력 컴포넌트를 스토리북 환경에서 UI 테스트를 진행하려고 하니, React is not defined 예외가 발생했습니다. 여러 해결책을 찾던 중, 달력 컴포넌트를 구성하는 컴포넌트들에 import React from 'react'를 추가해서 임시 방편으로 해결했어요. 분명 이 문제는 @Largopie 가 해결해준 것 같은데 다시 에러가 발생하더군요...ㅠ

리뷰 요구사항 (선택)

github-actions · 2024-09-15T08:13:18Z

Test Results

5 tests 5 ✅ 3s ⏱️
1 suites 0 💤
1 files 0 ❌

Results for commit 45ab6ca.

♻️ This comment has been updated with latest results.

Largopie

오우.. 해리 굉장히 많은 작업을 해주셨네요..! 일단 굉장히 고생하셨습니다 👏👏👏
제가 꼼꼼하게 리뷰를 하려고했지만, 제 실력이 아직은 부족한 탓에 완벽히 로직을 파악하지는 못한 것 같아요! 그래도 최대한 이해한 선에서 리뷰를 남겨보았습니다!

추가적으로 하나만 고르는 컴포넌트에서 선택되는 모양이 디자인적으로 은근 어색하게 느껴진 것 같습니다. 물론 사용하기에 불편하지 않은 수준이라서 우선순위로 개선할 필요는 없겠지만, 나중에 디자인을 개선할 시간이 주어진다면 한 번 개선해봐도 좋을 것 같아요!

그리고, Storybook에서 react is not defined가 나오는 이유는 제가 import를 자동으로 포함시키는 것을 webpack에서만 설정해주었기 때문인 것 같아요. Stroybook 설정에서 별도로 react 선언과 관련된 설정을 건드리거나, 별도로 전체적으로 적용될 수 있게 설정해야 할 것 같네요!

// webpack.common.js
new webpack.ProvidePlugin({
      React: 'react',
    }),

일단 한 번의 RC는 드리겠지만, 이펙티브한 부분은 없어서 확인 한 번 부탁드리겠습니다용~ 고생했어요 해리 🍀

Largopie · 2024-09-18T08:42:39Z

frontend/src/hooks/useCalendar/useCalendar.ts

+const TODAY = new Date();
+
+const useCalendar = (): useCalendarReturn => {
+  const [currentFullDate, setCurrentFullDate] = useState(new Date());


앞의 commit에서 useCalendar 훅이 구현되어 있었는데, 커밋이 뒤쪽에 있어서 살짝 당황스럽긴 했습니다요.. (TMI)

[P3]

TODAY 상수로 넣어줘도 괜찮을 것 같네요!

[C]

ㅋㅋㅋㅋㅋㅋㅋㅋㅋㅋㅋㅋ 저도 리뷰할 때 커밋 단위로 봐서 공감되네요🐪🐫

커밋 흐름 신경쓰도록 하겠습니다~ 👍👍👍 저도 커밋 단위로 리뷰 하는데 신경쓰지 못했네요...😢

Largopie · 2024-09-18T09:02:49Z

frontend/src/hooks/useCalendarContext/useCalendarContext.ts

[P3]

useCalendarContext는 hook의 관심사와는 별개라고 생각드는데요!
contexts라는 별도의 폴더 내부로 옮기는 것은 어떨까요?

Calendar 컴포넌트 내부에 있는 CalendarProvider도 마찬가지로요!

동의합니다~

Largopie · 2024-09-18T09:07:28Z

frontend/src/hooks/useCalendar/useCalendar.ts

+  const moveToPrevMonth = () => {
+    setCurrentFullDate(new Date(currentYear, currentMonth - 1));
+  };
+  const moveToNextMonth = () => {
+    setCurrentFullDate(new Date(currentYear, currentMonth + 1));
+  };


[P3]

개행 해주실 수 있나용? 부탁입니다.🥹

https://www.youtube.com/shorts/0CtRoi9-i24

개행개행~

Largopie · 2024-09-18T10:17:05Z

frontend/src/hooks/useCalendar/useCalendar.utils.ts

+
+export const getMonthlyStartIndex = (date: Date) => {
+  const firstOfMonth = setFirstDate(date);
+  return (firstOfMonth.getDay() + 7) % 7;


[Q]

(firstOfMonth.getDay() + 7) % 7 을 계산하면 firstOfMonth.getDay()와 똑같은 결과값이 나올 것으로 기대되는데 별도로 계산해주신 이유가 있나요?

로직 상 실수가 있었네요...ㅎ

Largopie · 2024-09-18T17:46:38Z

frontend/src/utils/date.ts

+export const getMonth = (date: Date) => date.getMonth();
+export const getYear = (date: Date) => date.getFullYear();
+export const getDay = (date: Date) => date.getDay();
+export const getDate = (date: Date) => date.getDate();


[P2]

같은 결과 값을 반환하는 것은 똑같아서 불필요한 연산 및 함수를 저장하고 있을 것 같다는 생각이 들어요!
date.getMonth()와 같은 Date 내장 메서드를 그대로 이용하는 것은 어떨까요?

저는 .을 사용해서 객체의 메서드를 호출하는 것 보다 위 구현처럼 함수로 만드는 것이 더 가독성 측면에서 괜찮다고 생각해서 저렇게 구현한 것인데... 나중에 확장성을 위해서라도 일단 저렇게 분리해서 구현하는 것이 어떨까요??!!

추후에 확장을 한다면 어떠한 형태로 확장할 수 있을까요? 특별하게 확장성이 있는 함수의 경우에만 추후에 별도로 분리하여 구현하는 방법도 괜찮다고 생각합니다 :)

실질적으로 1, 2번의 방법이 속도차이에서 큰 변화가 있지는 않으며, 가독성이 좋을 수는 있다고 생각합니다.

하지만 개인적으로 느끼기에는 1번의 경우보다 2번의 경우가 같은 결과를 반환하는 함수를 중복적으로 선언하지 않기 때문에 더 낫다는 생각이 드네요!

getMonth 함수 호출 -> Date 객체 내장 함수 호출 -> Date 내장 함수 결과 반환 -> getMonth 함수 결과 반환

Date 객체 내장 함수 호출 -> Date 내장 함수 결과 반환

그럼에도 해리가 생각하기에 확장 가능성이 있고, 가독성이 편리하다면 그대로 구현해도 괜찮을 것 같아요!

Largopie · 2024-09-18T17:49:59Z

frontend/src/components/MeetingCalendar/Date/Date.styles.ts

+export const s_baseDateButton = () => css`
+  cursor: pointer;
+
+  position: relative;
+
+  display: flex;
+  flex-direction: column;
+  align-items: center;
+  justify-content: center;
+
+  border: none;
+
+  &:disabled {
+    cursor: default;
+  }
+`;


[P3]

props가 별도로 필요하지 않으니 아래와 같이 작성해도 좋을 것 같습니다!

Suggested change

export const s_baseDateButton = () => css`

cursor: pointer;

position: relative;

display: flex;

flex-direction: column;

align-items: center;

justify-content: center;

border: none;

&:disabled {

cursor: default;

}

`;

export const s_baseDateButton = css`

cursor: pointer;

position: relative;

display: flex;

flex-direction: column;

align-items: center;

justify-content: center;

border: none;

&:disabled {

cursor: default;

}

`;

`;

엥 왜 저렇게 했을까요..ㅋㅋ

Largopie · 2024-09-18T17:57:43Z

frontend/src/components/MeetingCalendar/Date/Date.styles.ts

+const DAY_SLOT_TEXT_STYLES = {
+  selected: css`
+    color: ${theme.colors.calendar.color.selected};
+  `,
+  holiday: css`
+    color: ${theme.colors.calendar.color.holiday};
+  `,
+  today: css`
+    color: ${theme.colors.calendar.color.today};
+  `,
+  saturday: css`
+    color: #8c9eff;
+  `,
+  prevDay: css`
+    color: ${theme.colors.grey.primary};
+  `,
+  default: css`
+    color: ${theme.colors.black};
+  `,
+};


[P3]

코드의 흐름상 상수가 s_dateText보다 위에 위치하는 것이 가독성이 더 좋을 것 같다는 생각이 드는데 해리는 어떻게 생각하시나요?

그러네요~ 저도 선언을 위 라인에 하고 아래 라인에서 참조해서 사용하는 것에 동의합니다! 디테일적인 부분에 대한 피드백 감사합니다~

Largopie · 2024-09-18T18:12:35Z

frontend/src/components/MeetingCalendar/Date/RangeDate.tsx

+        }),
+      ]}
+    >
+      <span css={[s_baseDateText]}>{date}</span>


[P3]

하나의 css prop을 사용하니 없어도 괜찮을 것 같아요 :)

Suggested change

<span css={[s_baseDateText]}>{date}</span>

<span css={s_baseDateText}>{date}</span>

Largopie · 2024-09-18T18:27:34Z

frontend/src/components/MeetingCalendar/Date/DateExtraInfo.tsx

+  today: '오늘',
+  rangeStart: '시작',
+  rangeEnd: '끝',
+  default: '\u00A0',


[Q]

'\u00A0' 이 문자는 무슨뜻일까요?

[P3]

++ 해당 컴포넌트 내에서 한번만 사용되는 문자열이라면 상수로 분리하지 않아도 괜찮지 않을까 조심스레 의견 던져봅니다!

\u00A0는 줄바꿈을 하지 않는 빈칸을 의미하는 HMTL 특수 문자표(Entity)입니다. 해당 문자표를 사용한 이유는,

const DATE_INFO_TEXTS = { today: '오늘', rangeStart: '시작', rangeEnd: '끝', default: '', };

위와 같이 빈 문자열로 설정할 경우,

공휴일인 달과 그렇지 않은 달의 행간이 맞지 않는 문제가 있습니다. ' '와 같이 빈 칸을 추가해도 마찬가지였구요. 그래서 사용하게 되었습니다. HTML Entities Table에서도 확인할 수 있어요:)

Largopie · 2024-09-18T18:29:03Z

frontend/src/components/MeetingCalendar/Date/DateExtraInfo.tsx

+  return DATE_INFO_TEXTS.default;
+};
+
+export default function DateExtraInfo({


[Q]

DateExtraInfo라는 컴포넌트명을 선택하신 이유가 있을까요?
제 개인적인 견해로는 이름을 처음 보고 해당 컴포넌트의 역할이 무엇인지 파악하기 어려웠던 것 같아요!

저도 해당 컴포넌트의 네이밍에 대해서 굉장히 많은 고민을 했었는데요! 공휴일, 시작, 끝, 오늘, 빈 문자...등등 다향한 정보를 추가적으로 알려준다는 느낌을 표현하고자 했습니다. 기본적으로 날짜 정보(숫자)는 알려주니까 추가적이라는 느낌을 주고 싶었어요. 아직도 애매모호함이 남아있긴 하네요...

DateAdditionalText는 어떤가요~? 괜찮다구요? 알겠어요!

Yoonkyoungme

해리🍀 추석 동안 쉬지 않고 달력 UI 개선하느라 고생 많으셨어요👏👏
2024 추석은 모모와 함께 ~
몇 가지 간단한 피드백 남겨두었습니다 :) 확인해 주셔요!

Yoonkyoungme · 2024-09-19T00:42:01Z

frontend/package.json

@@ -45,6 +45,7 @@
    "@babel/preset-typescript": "7.24.7",
    "@chromatic-com/storybook": "1.6.1",
    "@emotion/babel-plugin": "11.11.0",
+    "@hyunbinseo/holidays-kr": "3.2025.1",


[Q]

한국 공휴일 라이브러리로 hyunbinseo/holidays-kr를 선택한 이유가 궁금합니다!

우선 공휴일을 직접 매년 업데이트 하는 것도 방법이지만, 배꼽이 더 커지는 구현이라고 생각해 라이브러리를 활용하기로 결정했습니다.

위 이미지에서 확인할 수 있듯, 세 라이브러리를 비교해 본 결과 현재 사용하고 있는 라이브러리의 다운로드 수가 가장 많은 것을 확인할 수 있었습니다. 그리고 가장 최근 커밋도 2주 전이고 계속해서 버젼 업데이트가 되는 것을 확인할 수 있었습니다. 기왕 라이브러리를 활용하기로 한 거, 다운로드 수도 많고 가장 활발히 업데이트 되고 있는 라이브러리를 사용하고자 했습니다.

Yoonkyoungme · 2024-09-19T02:15:10Z

frontend/src/components/MeetingCalendar/Date/Date.styles.ts

+  if (isHoliday) return DAY_SLOT_TEXT_STYLES.holiday;
+  if (isSunday) return DAY_SLOT_TEXT_STYLES.holiday;


[P3]

이 부분을 아래처럼 변경해도 괜찮을 것 같네요~

Suggested change

if (isHoliday) return DAY_SLOT_TEXT_STYLES.holiday;

if (isSunday) return DAY_SLOT_TEXT_STYLES.holiday;

if (status.isHoliday || status.isSunday) return DAY_SLOT_TEXT_STYLES.holiday;

혹은 코드 리팩터링 측면에서 아래와 같은 로직도 한 가지 방법일 것 같아요 :)

export const s_dateText = ({ isSelectedDate, isPrevDate, isSunday, isSaturday, isHoliday, isToday, }: FlagObject<DateStatus>) => { const statusMap = { isSelectedDate: DAY_SLOT_TEXT_STYLES.selected, isToday: DAY_SLOT_TEXT_STYLES.today, isPrevDate: DAY_SLOT_TEXT_STYLES.prevDay, isHoliday: DAY_SLOT_TEXT_STYLES.holiday, isSunday: DAY_SLOT_TEXT_STYLES.holiday, isSaturday: DAY_SLOT_TEXT_STYLES.saturday, }; const status = Object.keys(statusMap).find(key => statusMap[key]); return status ? statusMap[status] : DAY_SLOT_TEXT_STYLES.default; };

오..find 메서드를 활용해서 조건에 맞는 배열의 첫 번째 요소를 반환하면 우선순위와 관련된 문제도 자연스럽게 해결할 수 있겠네요.

Yoonkyoungme · 2024-09-19T02:34:08Z

frontend/src/components/MeetingCalendar/Date/Date.utils.ts

+  const holidayName = getHolidayNames(date);
+  /*
+  현재 사용하고 있는 라이브러리는 하루에 공휴일이 겹칠 수 있는 경우를 대비하기 위해서 string[] 형태로 공휴일들을 관리하고 있다.
+  -> 그래서, 공휴일이 있는 경우 첫 번째 공휴일을 반환하도록 했다. 
+  -> 네이버나 다른 서비스의 달력들을 참고했을 때 하루에 공휴일이 겹치는 경우 하나만 보여주는 것을 확인했다. 
+  -> 여러개를 모두 보여주는 경우, 레이아웃이 달라질 수 있기 때문에 하나만 보여주는 것으로 결정. (@해리)
+  */


[P3]

string[] 형태로 공휴일들을 관리하고 있으니 변수명에서도 그 의미를 파악하기 쉽게 holidayNames로 하는건 어떨까요?

Yoonkyoungme · 2024-09-19T02:44:48Z

frontend/src/components/MeetingCalendar/Date/Date.utils.ts

+export const getDateInfo = (date: Date, today: Date) => {
+  const currentDate = getDate(date);
+  const currentDay = getDay(date);
+
+  const currentFullDate = getFullDate(date);
+  const todayFullDate = getFullDate(today);


[P2]

OH.. Harry, 이번 코드에서는 변수명과 실제 값의 로직이 일치하지 않아 이해하는 데 시간이 조금 걸렸던 것 같아요 😭

콘솔 출력을 통해 확인해보니
date: Sun Sep 01 2024 11:17:56 GMT+0900 (한국 표준시)
today: Thu Sep 19 2024 11:17:55 GMT+0900 (한국 표준시)

일 때 결과값은 다음과 같았습니다.

currentDate: 1

currentDay: 0

currentFullDate: '2024-09-01'

todayFullDate: '2024-09-19'

currentDate와 currentDay에 날짜 값이 들어있을 것이라고 예상했는데 숫자가 출력되어서 당황스러웠어요.
또한 두 변수 모두 current, 즉 '현재'라는 의미를 가지고 있는데 서로 다른 날짜를 나타내고 있어서 혼란스러웠습니다.

변수명이 더욱 의미를 명확히 전달하도록 아래와 같이 변경해보는 것은 어떨까요?

const targetDateNumber = getDate(targetDate); const targetDayOfWeek = getDay(targetDate); const targetFullDate = getFullDate(targetDate); const todayFullDate = getFullDate(today);

좋습니다~ 기존에 날짜 도메인 용어를 맞췄던 것을 최대한 지킬 수 있도록 변수명을 수정하도록 할게요!

Yoonkyoungme · 2024-09-19T02:51:32Z

frontend/src/components/MeetingCalendar/Header/index.tsx

+        <TabButton
+          isActive={dateSelectMode === 'single'}
+          onClick={() => toggleDateSelectMode('single')}
+        >
+          하나씩
+        </TabButton>
+        <p>/</p>
+        <TabButton
+          isActive={dateSelectMode === 'range'}
+          onClick={() => toggleDateSelectMode('range')}
+        >
+          기간
+        </TabButton>


[P3]

VoiceOver로 테스트해보니, 각 버튼을 누를 때 '하나씩 버튼', '기간 버튼'이라고만 안내되는 것 같아요.

'하나씩 날짜 선택하기', '기간으로 날짜 선택하기'와 같은 더 구체적인 안내 메시지를 추가(aria-label 사용)하면 사용자가 버튼의 기능을 더 잘 이해할 수 있어 접근성이 향상될 것 같습니다.

이렇게 개선해보면 어떨까요? 😊

정말 디테일하게 피드백을 주시네요~ aria-label을 추가해서 더 접근성 높은 달력 컴포넌트를 만들어보도록 할게요!

Yoonkyoungme · 2024-09-19T04:28:32Z

frontend/src/hooks/useCalendar/useCalendar.ts

+    headers: {
+      currentYear,
+      currentMonth,
+      weekDays: ['일', '월', '화', '수', '목', '금', '토'],
+    },
+    body: {


[Q]

단순 궁금증인데 왜 headers는 복수형이고, body는 단수로 설정하셨나요?

피드백 반영을 위해서 둘 다 복수형으로 하려고 bodies로 변경하니 뭔가 이상하네요...굳이 headers가 복수형이 아니어도 충분히 의미를 전달할 수 있을 것 같으니, headers -> header로 변경해 볼게요.

Yoonkyoungme · 2024-09-19T04:38:26Z

frontend/src/hooks/useCalendar/useCalendar.utils.ts

+ * 반환 데이터 예시:
+ * [
+ *   {
+ *     key: 2023090, // 2023-09의 첫 번째 주
+ *     value: [
+ *       { key: '2023-08-27', value: Date, status: 'prevMonth' }, 이전 달 데이터인 8월 데이터가 포함되고, prevMonth 상태를 가짐.
+ *       { key: '2023-08-28', value: Date, status: 'prevMonth' },
+ *       ...
+ *       { key: '2023-09-03', value: Date, status: 'currentMonth' }
+ *     ]
+ *   },
+ *   ...
+ *   {
+ *     key: 2023094,
+ *     value: [
+ *       { key: '2023-09-25', value: Date, status: 'currentMonth' },
+ *       { key: '2023-09-26', value: Date, status: 'currentMonth' },
+ *       ...
+ *       { key: '2023-10-01', value: Date, status: 'nextMonth' } // 다음 달 데이터인 10월 데이터가 포함되고, nextMonth 상태를 가짐.
+ *     ]
+ *   }
+ * ]


[P3]

헉쓰 JSDoc이 정말 친절하군요! 하지만 엄청나게 기네요..! 반환 데이터 예시값은 필요시 콘솔에서 직접 확인할 수 있는 부분인 것 같아용! 해당 부분을 지워서 주석의 길이를 줄일 수 있을 것 같습니다 :)

깃허브에서 코드 리뷰를 할 때는, 콘솔에서 직접 확인을 하기 힘드니 코드를 읽을 때 반환값 예시가 있으면 이해하기 더 쉬울 것 같다고 생각해서 작성했었습니다. 저는 다른 사람의 코드를 리뷰할 때, 콘솔에서 직접 확인하지 않으니 반환값 예시를 그대로 두면 좋을 것 같다고 생각해요~ 주석의 경우는 프로덕션 빌드에서 제거가 되니 번들 사이즈 관련 문제도 없을 것 같아요

[P2]

해당 반환 함수를 편하게 확인할 수 있도록 테스트케이스를 구현해보는 것은 어떨까요? 🤓

Yoonkyoungme · 2024-09-19T05:00:24Z

frontend/src/hooks/useCalendar/useCalendar.utils.ts

+export const getNumberOfWeeks = (date: Date) => {
+  const firstOfMonth = setFirstDate(date);
+  const daysInMonth = new Date(getYear(date), getMonth(date) + 1, 0).getDate();
+  const dayOfWeek = firstOfMonth.getDay();


[P3]

7이라는 매직넘버가 많이 사용되는데, 상수로 바꿔보면 좋을 것 같아요~~
ex) const DAYS_IN_WEEK = 7;

Yoonkyoungme · 2024-09-19T05:11:30Z

frontend/src/hooks/useCalendar/useCalendar.utils.ts

+export const getWeeklyDate = (startDate: Date, currentMonth: number): DateInfo[] =>
+  Array.from({ length: CALENDAR_PROPERTIES.daysInOneWeek }, (_, i) => {
+    const date = new Date(startDate);
+    date.setDate(getDate(startDate) + i);
+
+    let status: MonthStatus;
+    if (getMonth(date) < currentMonth || (getMonth(date) === 11 && currentMonth === 0)) {
+      status = 'prevMonth';
+    } else if (getMonth(date) > currentMonth || (getMonth(date) === 0 && currentMonth === 11)) {
+      status = 'nextMonth';
+    } else {
+      status = 'currentMonth';
+    }
+
+    return {
+      key: `${date}`,
+      value: date,
+      status,
+    };
+  });


[P3]

getWeeklyDate 함수 내의 status 결정 로직을 별도의 함수로 분리하여 가독성과 유지보수성을 개선할 수 있을 것 같아요~

const getMonthStatus = (date: Date, currentMonth: number) => { const month = getMonth(date); if (month < currentMonth || (month === 11 && currentMonth === 0)) return 'prevMonth'; if (month > currentMonth || (month === 0 && currentMonth === 11)) return 'nextMonth'; return 'currentMonth'; };

export const getWeeklyDate = (startDate: Date, currentMonth: number): DateInfo[] => ... return { key: `${date}`, value: date, status: getMonthStatus(date, currentMonth), }; })

훨씬 보기 좋고 깔끔하네용~

Yoonkyoungme · 2024-09-19T06:17:21Z

frontend/src/types/calendar.ts

@@ -0,0 +1,14 @@
+export type DateSelectMode = 'single' | 'range';
+
+export type MonthStatus = 'prevMonth' | 'currentMonth' | 'nextMonth';


[P3]

type명에서 Month와 관련있다는 것이 유추 가능해서 -Month 접미사를 생략해도 괜찮을 것 같아요!

Suggested change

export type MonthStatus = 'prevMonth' | 'currentMonth' | 'nextMonth';

export type MonthStatus = 'prev' | 'current' | 'next';

좋은 피드백인 것 같아요~ month가 중복되고 있네요.

Largopie

핼리콥터~! 빠르게 RC를 들고 나타난 낙타🐫에요~
수정사항 반영 부탁드려요~!

Largopie · 2024-09-22T15:08:15Z

frontend/legacy/hooks/useCalendarInfo/useCalendarInfo.utils.ts

+  const daysInMonth = new Date(getYear(date), getMonth(date) + 1, 0).getDate();
+  const dayOfWeek = firstOfMonth.getDay();
+
+  return Math.ceil((daysInMonth + ((dayOfWeek + 7) % 7)) / 7);


[P2]

해당 로직도 마찬가지로 개선하면 좋을 것 같아요~! 헬리콥터~

Largopie · 2024-09-22T15:10:43Z

frontend/src/components/_common/Buttons/TabButton/index.tsx

  isActive: boolean;
  onClick: () => void;
  tabButtonVariants?: TabButtonVariants;
+  children: React.ReactNode;


[P2]

저희가 컴포넌트 내부 이벤트에 관련되어서만 타입을 정의했네요. 통일성을 위해 해당 타입도 ReactNode로 표현하는 것은 어떨까요?

동의합니다! 수정할게요~

Largopie · 2024-09-22T15:13:00Z

frontend/src/hooks/useCalendar/useCalendar.utils.ts

+ * 반환 데이터 예시:
+ * [
+ *   {
+ *     key: 2023090, // 2023-09의 첫 번째 주
+ *     value: [
+ *       { key: '2023-08-27', value: Date, status: 'prevMonth' }, 이전 달 데이터인 8월 데이터가 포함되고, prevMonth 상태를 가짐.
+ *       { key: '2023-08-28', value: Date, status: 'prevMonth' },
+ *       ...
+ *       { key: '2023-09-03', value: Date, status: 'currentMonth' }
+ *     ]
+ *   },
+ *   ...
+ *   {
+ *     key: 2023094,
+ *     value: [
+ *       { key: '2023-09-25', value: Date, status: 'currentMonth' },
+ *       { key: '2023-09-26', value: Date, status: 'currentMonth' },
+ *       ...
+ *       { key: '2023-10-01', value: Date, status: 'nextMonth' } // 다음 달 데이터인 10월 데이터가 포함되고, nextMonth 상태를 가짐.
+ *     ]
+ *   }
+ * ]


[P2]

해당 반환 함수를 편하게 확인할 수 있도록 테스트케이스를 구현해보는 것은 어떨까요? 🤓

Largopie · 2024-09-22T15:16:16Z

frontend/src/components/MeetingCalendar/Date/Date.styles.ts

+export const s_dateText = (dateStatusMap: FlagObject<DateStatus>) => {
+  // key가 dateStatusMap에 속하는 타입인지 확정할 수 없는 문제 발생 -> type guard 함수로 해결(@해리)
+  const dateStatusArray = Object.keys(dateStatusMap);
+  if (!isValidArrayType<string, DateStatus>(Object.keys(dateStatusMap), dateStatusArray)) return;


오 신기한 함수를 구현하셨네요! 활용도가 높을듯! 👍

[P2]

해당 반환 함수를 편하게 확인할 수 있도록 테스트케이스를 구현해보는 것은 어떨까요? 🤓

낙타가 빠르게 쓰셔야 한다고 하니,,,일단 머지를 한 후 테코톡이 끝난 후 테스트를 진행해 보도록 할게요,,,ㅎ

Largopie · 2024-09-22T15:19:26Z

frontend/src/components/MeetingCalendar/Date/DateAdditionalText.tsx

+  today: '오늘',
+  rangeStart: '시작',
+  rangeEnd: '끝',
+  default: '',


'\u00A0'

👀👀👀👀👀👀👀👀👀👀👀

Yoonkyoungme

헬리콥터~! 조금 느리지만 Comment를 들고 나타난 빙봉🔮이에요~ (낙타가 달리기가 빠르네요! 이랴 이랴)
가벼운 코멘트 1개 확인 부탁드려요~
앞서 낙타가 RC를 준 부분이 반영되면 다시 한번 멘션 주세요! 바로 Approve 하러 달려오겠습니다

Yoonkyoungme · 2024-09-23T14:16:33Z

frontend/src/components/MeetingCalendar/Date/DateAdditionalText.tsx

+interface DateInfoProps {
+  isToday?: boolean;
+  isRangeStart?: boolean;
+  isRangeEnd?: boolean;
+  holidayName: string | null;
+}
+
+const getDateInfoText = ({ isToday, isRangeStart, isRangeEnd, holidayName }: DateInfoProps) => {
+  if (isRangeStart) return DATE_INFO_TEXTS.rangeStart;
+  if (isRangeEnd) return DATE_INFO_TEXTS.rangeEnd;
+  if (isToday) return DATE_INFO_TEXTS.today;
+  if (holidayName) return holidayName;
+  return DATE_INFO_TEXTS.default;
+};


[P3]

DATE_INFO_TEXTS 객체에 as const를 추가하여 타입 안정성을 높여볼 수 있을 것 같네요!

Yoonkyoungme · 2024-09-23T14:22:26Z

frontend/src/components/MeetingCalendar/Header/index.tsx

@@ -51,13 +51,15 @@ export default function MeetingCalendarHeader({
        <TabButton
          isActive={dateSelectMode === 'single'}
          onClick={() => toggleDateSelectMode('single')}
+          aria-label="하나씩 선택하기"


👍👍👍

Largopie

시간 관계상 먼저 Approve를 드리도록 하겠습니다! 추가적으로 refactor 이슈를 파서 진행해주쎄요~!

- useCalendar 커스텀 훅이 반환하는 달력 데이터를 각 컴포넌트가 뽑아서 사용할 수 있도록 Context API 활용 - Header, Weekdays, Body 컴포넌트로 달력을 구성하는 컴포넌트들을 합성 컴포넌트 구조로 구현

- render props 패턴을 활용하여 useCalendar 컴포넌트가 반환하는 월, 일 데이터를 사용해서 UI를 그리는 컴포넌트를 호출할 수 있도록 구현

- render props 패턴을 활용하여 useCalendar 컴포넌트가 반환하는 일주일 날짜 데이터를 사용해서 UI를 그리는 컴포넌트를 호출할 수 있도록 구현

- render props 패턴을 활용하여 useCalendar 컴포넌트가 반환하는 날짜 데이터를 사용해서 UI를 그리는 컴포넌트를 호출할 수 있도록 구현

- 날짜를 선택할 수 있는 모드인 single, range를 상태로 관리 - 시작/끝 날짜가 모두 선택되면 그 사이 모든 날짜가 선택되도록 getDatesInRange 유틸 함수 구현 - 끝 날짜가 시작 날짜보다 이른 경우 해당 끝 날짜를 다시 시작 날짜로 변경하도록 예외 처리 - 모든 범위 날짜가 선택되었을 때, 다른 날짜를 선택한 경우 해당 다른 날짜를 시작 날짜로 변경하도록 예외 처리

- 기존에는 하나의 CalendarDate로 구현을 하려고 했으나, 하나씩 선택하는 것과 범위로 선택하는 것의 UI 책임이 너무 다르다고 판단해 따로 구현하는 것으로 결정

… 훅 구현

hwinkr added 🐈 프론트엔드 프론트엔드 관련 이슈에요 :) 🚀 기능 기능을 개발해요 :) 🎨 디자인 디자인을 해요 :) labels Sep 15, 2024

hwinkr self-assigned this Sep 15, 2024

hwinkr requested review from Yoonkyoungme and Largopie as code owners September 15, 2024 08:12

Largopie requested changes Sep 18, 2024

View reviewed changes

Yoonkyoungme requested changes Sep 19, 2024

View reviewed changes

Largopie requested changes Sep 22, 2024

View reviewed changes

Yoonkyoungme reviewed Sep 23, 2024

View reviewed changes

Largopie approved these changes Sep 24, 2024

View reviewed changes

hwinkr added 18 commits September 24, 2024 21:10

chore: 한국 공휴일을 계산해 주는 라이브러리 추가

25a92de

chore: 이전 달력 컴포넌트 & 로직 레거시 폴더로 이동

98acf0a

chore: 이전에 사용했던 달력 데이터 계산 로직 커스텀 훅 삭제

2d7d73a

feat: 공통 달력 컴포넌트, 헤더 컴포넌트 구현

e94ab43

- render props 패턴을 활용하여 useCalendar 컴포넌트가 반환하는 월, 일 데이터를 사용해서 UI를 그리는 컴포넌트를 호출할 수 있도록 구현

feat: 공통 달력 컴포넌트, 일주일 날짜를 표현하는 컴포넌트 구현

2d1e886

- render props 패턴을 활용하여 useCalendar 컴포넌트가 반환하는 일주일 날짜 데이터를 사용해서 UI를 그리는 컴포넌트를 호출할 수 있도록 구현

feat: 공통 달력 컴포넌트, 한 달을 구성하는 모든 날짜를 표현하는 컴포넌트 구현

4f790c8

- render props 패턴을 활용하여 useCalendar 컴포넌트가 반환하는 날짜 데이터를 사용해서 UI를 그리는 컴포넌트를 호출할 수 있도록 구현

feat: 약속을 생성할 때 필요한 달력 컴포넌트 - 헤더 컴포넌트 구현

c809b32

feat: 약속을 생성할 때 필요한 달력 컴포넌트 - 일주일 날짜 컴포넌트 구현

17de9a1

feat: 한 달의 날짜 데이터, 다음/이전 달로 이동시킬 수 있는 인터페이스를 제공하는 useCalendar 커스텀 훅 구현

6d5a6fb

chore: 폴더명 변경에 따른 import 경로 수정

0a46458

chore: 공통 달력 컴포넌트, useCalendar 커스텀 훅에서 필요한 타입 정의

868feaf

feat: 달, 년(year), 일, 날짜, 전체 날짜를 계산하는 유틸 함수 구현

3a2bf92

feat: 달력을 구성하는 각각의 컴포넌트들이 Context API로 공유되는 달력 데이터를 뽑아서 사용할 수 있도록 커스텀…

7055d8d

… 훅 구현

refactor: 변경된 달력 컴포넌트 반영

fea7bf2

feat: 날짜 추가 정보를 보여주는 컴포넌트 구현

7882c4e

hwinkr added 17 commits September 24, 2024 21:10

refactor: 날짜 추가 정보를 보여주는 컴포넌트를 사용하는 것으로 수정

71f0b79

chore: Fragment 제거, 리액트 모듈 import 구문 제거

dd62921

chore: 현재 날짜 정보를 구할 때, 더 의미있는 변수명을 사용하는 것으로 개선

6160f7c

design: 특정 날짜의 상태를 구하기 위해서 연속된 if문을 사용하는 것이 아닌 객체를 활용해서 구하는 것으로 수정

74295fa

feat: 유효한 배열의 타입인지 확인하는 타입 가드 함수 구현

271a614

chore: 날짜 아래에 위치하는 텍스트 컴포넌트명을 더 의미있게 수정

e4cd873

chore: 현재 달의 상태를 나타내는 변수명 수정 사항 반영, 날짜 상태 props 순서 수정

ed9d69e

chore: 달(Month) 상태 타입 네이밍 수정

72a4ccf

chore: 달력 정보를 Context API를 활용해서 내려주는 프로바이더 컴포넌트 위치 변경 반영

b979d8d

chore: 달력 정보를 Context API를 활용해서 내려주는 프로바이더 컴포넌트 위치 변경 반영

361db96

chore: 유효한 배열의 타입인지 확인하는 타입 가드 함수 위치 변경 반영

df46a82

refactor: 현재 달(Month)의 상태를 나타내는 로직을 함수로 분리

cabd2c6

chore: 함수 개행 추가

492b2d8

chore: aria-label을 추가하기 위한 TabButton 컴포넌트 props 타입 변경

b8c3823

chore: type import 구문 컨벤션에 맞게 수정

a9738c2

chore: 실수로 지웠던 HTML Entities 복구

b8a3795

chore: 필요없는 날짜 계산 로직 제거

45ab6ca

hwinkr force-pushed the refactor/338-calenadr-improvement branch from 0ee6a0b to 45ab6ca Compare September 24, 2024 12:10

Yoonkyoungme approved these changes Sep 24, 2024

View reviewed changes

hwinkr merged commit 8797d85 into develop Sep 24, 2024
7 checks passed

Yoonkyoungme deleted the refactor/338-calenadr-improvement branch September 24, 2024 12:13

	<span css={[s_baseDateText]}>{date}</span>
	<span css={s_baseDateText}>{date}</span>

		if (isHoliday) return DAY_SLOT_TEXT_STYLES.holiday;
		if (isSunday) return DAY_SLOT_TEXT_STYLES.holiday;

	if (isHoliday) return DAY_SLOT_TEXT_STYLES.holiday;
	if (isSunday) return DAY_SLOT_TEXT_STYLES.holiday;
	if (status.isHoliday \|\| status.isSunday) return DAY_SLOT_TEXT_STYLES.holiday;

		@@ -0,0 +1,14 @@
		export type DateSelectMode = 'single' \| 'range';

		export type MonthStatus = 'prevMonth' \| 'currentMonth' \| 'nextMonth';

	export type MonthStatus = 'prevMonth' \| 'currentMonth' \| 'nextMonth';
	export type MonthStatus = 'prev' \| 'current' \| 'next';

[FE] 런칭데이에서 받은 달력 UI 피드백 반영 #346

[FE] 런칭데이에서 받은 달력 UI 피드백 반영 #346

Conversation

hwinkr commented Sep 15, 2024

관련 이슈

작업 내용

오늘을 표시하는 방법을 오늘 텍스트로 변경

holidays-kr 라이브러리를 도입하여 실제 대한민국 공휴일을 표시하는 기능 추가

시작 날짜, 끝 날짜만 선택하여 연속된 날짜를 편하게 선택할 수 있도록 하는 기능 추가

특이 사항

달력 컴포넌트 구조 변경

스토리북 에러 임시 해결

리뷰 요구사항 (선택)

github-actions bot commented Sep 15, 2024 • edited Loading

Test Results

Largopie left a 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

Largopie Sep 22, 2024 • 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

Largopie Sep 18, 2024 • edited Loading

Choose a reason for hiding this comment

hwinkr Sep 21, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Yoonkyoungme left a 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Yoonkyoungme Sep 19, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Largopie left a 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

Yoonkyoungme left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Largopie left a comment

Choose a reason for hiding this comment

github-actions bot commented Sep 15, 2024 •

edited

Loading

Largopie Sep 22, 2024 •

edited

Loading

Largopie Sep 18, 2024 •

edited

Loading

hwinkr Sep 21, 2024 •

edited

Loading

Yoonkyoungme Sep 19, 2024 •

edited

Loading