[FE] Dropdown 컴포넌트 제작 및 시간 선택 hook 구현 #71

Largopie · 2024-07-24T16:35:12Z

작업 내용

드롭다운 컴포넌트 구현
시간 선택 hook 구현
- 시간 선택 시, startTime이 endTime을 느리거나 endTime이 startTime보다 빠르면 선택되지 않도록 구현
시간 선택 hook이 정상적으로 동작하는지 테스트케이스 작성

특이 사항

시간 선택 시, startTime이 endTime을 느리거나 endTime이 startTime보다 빠르면 선택되지 않도록 구현

이 부분은 추후에 변경사항이 생길 수도 있지만 일단은 구현해두었습니다!

리뷰 요구사항 (선택)

github-actions · 2024-07-24T16:36:05Z

Test Results

0 tests 0 ✅ 0s ⏱️
0 suites 0 💤
0 files 0 ❌

Results for commit eca2f5d.

♻️ This comment has been updated with latest results.

hwinkr

구현 빠르네요! 고생하셨습니다~🐪 사소한 코멘트 몇개 남겼습니다!

hwinkr · 2024-07-24T17:10:19Z

frontend/src/hooks/useTimeRangeDropdown/useTimeRangeDropdown.utils.ts

+  if (endHours < startHours) return false;
+  else if (endHours === startHours && endMinutes < startMinutes) return false;


[P2]

Suggested change

if (endHours < startHours) return false;

else if (endHours === startHours && endMinutes < startMinutes) return false;

if (endHours < startHours) return false;

if (endHours === startHours && endMinutes < startMinutes) return false;

early return을 해주기 때문에, else if문을 사용할 필요는 없을 것 같아요 :)

그렇네요! 수정하도록 하겠습니다 🐫👍

hwinkr · 2024-07-24T17:22:34Z

frontend/src/hooks/useTimeRangeDropdown/useTimeRangeDropdown.utils.ts

+}
+
+// 만약 시작 시간보다 끝 시간이 빠르다면 false를 반환하는 함수(@낙타)
+export function compareTimes(startTime: string, endTime: string) {


[P2]

함수명이 시간을 비교한다는 의미를 내포하고 있는 것 같은데, 보다 확실하게 어떤 동작을 하고 무엇을 반환하는지를 더 잘 표현하도록 함수명을 변경해보는 것은 어떨까요?

예를들어, checkStartTimeEarlier 와 같은...?

오 굉장히 좋은데요! 👍 반영하겠습니다 감사해요 :)

hwinkr · 2024-07-24T17:23:35Z

frontend/src/hooks/useTimeRangeDropdown/useTimeRangeDropdown.test.ts

+    expect(result.current.startTime).toBe('0:00');
+    expect(result.current.endTime).toBe('23:00');


[P3]

시간이 된다면 해당 매직 넘버에 대한 상수화를 진행해보는 것이 괜찮을 것 같네요!!

저도 이번 스프린트가 끝나면 엄청난 상수화를 해야 하기에....매직 넘버가 눈에 띄네요 ㅋㅋ

[C]

++ hook, util 파일에서 정의하신 상수를 사용해봐도 좋을 것 같아요~!

일단 가장 먼저 필요에 의한 상수 정의는 해당 폴더의 constant파일에 별도로 정의해두도록 하겠습니다! 의견 고마워요! ☺️

hwinkr · 2024-07-24T17:26:01Z

frontend/src/hooks/useTimeRangeDropdown/useTimeRangeDropdown.test.ts

+      result.current.onStartTimeChange(CHANGE_TIME);
+    });
+
+    expect(result.current.startTime).not.toBe(CHANGE_TIME);


[P2]

not을 사용해서 검증하기 보다는, 변경되지 않을 때의 값과 비교해보는 것은 어떨까요? 현재는 CHANGE_TIME만 아니면 통과하기 때문에, 더 정확한 테스트를 위해서 특정 값 자체와 동등 연산자로 비교하는 것이 괜찮을 것 같다는 생각이 들어요~

이 부분은 제가 고민하고 수정한 부분인데요!
특정 값 자체와 동등 연산자로 비교하기보다는 클릭했을 때 '선택되지 않는다'에 초점을 더 맞추어서 구현한 테스트케이스 입니다!

제 생각으로는 "1시를 선택했는데 1시로 변경되지 않는다." 라는 컨텍스트의 입장에서는 맞는 테스트케이스라고 생각했습니다!

해리는 다른 의견이실까요?!

++ 추가적으로 해당 내용에 대해서 아예 선택하지 않게끔 options 값을 유동적으로 변화하게 코드를 수정해보겠습니다!

Yoonkyoungme

낙타 고생했습니다!! 작업 속도가 엄청 빠르네요~! 간단한 코멘트 남겼는데, 천천히 확인해 보세요!

Yoonkyoungme · 2024-07-25T00:37:12Z

frontend/src/hooks/useTimeRangeDropdown/useTimeRangeDropdown.utils.ts

+}
+
+// 만약 시작 시간보다 끝 시간이 빠르다면 false를 반환하는 함수(@낙타)
+export function compareTimes(startTime: string, endTime: string) {


[C]

boolean 값을 리턴하니까
isTimeRangeValid 이런 느낌의 네이밍은 어떤가요?

이게 좋네요! 👍👍👍

Yoonkyoungme · 2024-07-25T00:51:23Z

frontend/src/hooks/useTimeRangeDropdown/useTimeRangeDropdown.test.ts

+    expect(result.current.startTime).toBe('0:00');
+    expect(result.current.endTime).toBe('23:00');


[C]

++ hook, util 파일에서 정의하신 상수를 사용해봐도 좋을 것 같아요~!

Largopie

리뷰 반영 완료했습니다 :)

현재는 시작시간은 끝 시간을 초과할 수 없고, 끝 시간은 시작 시간 이전으로 설정할 수 없게끔 옵션값이 변경되도록 수정했습니다.

하지만 추후에 아래의 동영상과 hover되었을 때 끝 시간이 변경되도록 수정하면 좋을 것 같네요!

2024-07-25.2.11.18.mov

Largopie · 2024-07-25T04:56:09Z

frontend/src/hooks/useTimeRangeDropdown/useTimeRangeDropdown.test.ts

+    expect(result.current.startTime).toBe('0:00');
+    expect(result.current.endTime).toBe('23:00');


일단 가장 먼저 필요에 의한 상수 정의는 해당 폴더의 constant파일에 별도로 정의해두도록 하겠습니다! 의견 고마워요! ☺️

Largopie · 2024-07-25T04:59:02Z

frontend/src/hooks/useTimeRangeDropdown/useTimeRangeDropdown.test.ts

+      result.current.onStartTimeChange(CHANGE_TIME);
+    });
+
+    expect(result.current.startTime).not.toBe(CHANGE_TIME);


이 부분은 제가 고민하고 수정한 부분인데요!
특정 값 자체와 동등 연산자로 비교하기보다는 클릭했을 때 '선택되지 않는다'에 초점을 더 맞추어서 구현한 테스트케이스 입니다!

제 생각으로는 "1시를 선택했는데 1시로 변경되지 않는다." 라는 컨텍스트의 입장에서는 맞는 테스트케이스라고 생각했습니다!

해리는 다른 의견이실까요?!

Largopie · 2024-07-25T04:59:28Z

frontend/src/hooks/useTimeRangeDropdown/useTimeRangeDropdown.test.ts

+      result.current.onStartTimeChange(CHANGE_TIME);
+    });
+
+    expect(result.current.startTime).not.toBe(CHANGE_TIME);


++ 추가적으로 해당 내용에 대해서 아예 선택하지 않게끔 options 값을 유동적으로 변화하게 코드를 수정해보겠습니다!

Largopie · 2024-07-25T04:59:59Z

frontend/src/hooks/useTimeRangeDropdown/useTimeRangeDropdown.utils.ts

+}
+
+// 만약 시작 시간보다 끝 시간이 빠르다면 false를 반환하는 함수(@낙타)
+export function compareTimes(startTime: string, endTime: string) {


오 굉장히 좋은데요! 👍 반영하겠습니다 감사해요 :)

Largopie · 2024-07-25T05:03:58Z

frontend/src/hooks/useTimeRangeDropdown/useTimeRangeDropdown.utils.ts

+}
+
+// 만약 시작 시간보다 끝 시간이 빠르다면 false를 반환하는 함수(@낙타)
+export function compareTimes(startTime: string, endTime: string) {


이게 좋네요! 👍👍👍

Largopie · 2024-07-25T05:06:30Z

frontend/src/hooks/useTimeRangeDropdown/useTimeRangeDropdown.utils.ts

+  if (endHours < startHours) return false;
+  else if (endHours === startHours && endMinutes < startMinutes) return false;


그렇네요! 수정하도록 하겠습니다 🐫👍

hwinkr

낙타 좋은데요~??

Yoonkyoungme

낙타👍👍 열심히 고민하고, 리뷰 반영해 주셨네요!

Yoonkyoungme · 2024-07-25T08:56:59Z

frontend/src/hooks/useTimeRangeDropdown/constants.ts

hooks, components 내에서만 사용하는 상수파일을 어떻게 분리할지 (ex: constants.ts / hook명.constants.ts, ... 등)에 대해서도 스프린트 끝나고 이야기 나누면 좋을 것 같네요!
스프린트 이후 TODO 작업으로 남겨두겠습니다~

Largopie requested review from hwinkr and Yoonkyoungme as code owners July 24, 2024 16:35

Largopie assigned hwinkr and Yoonkyoungme Jul 24, 2024

Largopie added 🐈 프론트엔드 프론트엔드 관련 이슈에요 :) 🚀 기능 기능을 개발해요 :) labels Jul 24, 2024

Largopie added this to the 2차 데모데이 milestone Jul 24, 2024

hwinkr requested changes Jul 24, 2024

View reviewed changes

Yoonkyoungme requested changes Jul 25, 2024

View reviewed changes

Largopie assigned Largopie and unassigned hwinkr and Yoonkyoungme Jul 25, 2024

Largopie commented Jul 25, 2024

View reviewed changes

hwinkr approved these changes Jul 25, 2024

View reviewed changes

Yoonkyoungme approved these changes Jul 25, 2024

View reviewed changes

Largopie added 9 commits July 25, 2024 19:30

feat(Dropdown): 컴포넌트 구현

49f0eac

test(Dropdown): 컴포넌트 UI 테스트

c369898

feat(useTimeRangeDropdown): 시간 선택 hook 구현

8ff81f4

test(useTimeRangeDropdown): 시작시간과 끝시간을 선택하는 로직에 대한 테스트 케이스 작성

418b277

test(Dropdown): 시간 선택 UI 테스트 추가

137953b

feat(useTimeRangeDropdown): 상수 정의

98d2a00

refactor: 12시, 24시에 관련된 오전, 오후 구분 로직 수정

e96e472

refactor(useTimeRangeDropdown): 선택한 시간에 따라 선택할 수 있는 옵션 값이 변경되도록 구현

0c37f8a

refactor: 정의된 상수 사용 및 함수 네이밍 수정

eca2f5d

Largopie force-pushed the feat/70-select branch from af51744 to eca2f5d Compare July 25, 2024 10:30

Largopie merged commit 8a83bd9 into develop Jul 25, 2024
5 checks passed

Largopie deleted the feat/70-select branch July 25, 2024 10:31

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[FE] Dropdown 컴포넌트 제작 및 시간 선택 hook 구현 #71

[FE] Dropdown 컴포넌트 제작 및 시간 선택 hook 구현 #71

Largopie commented Jul 24, 2024

github-actions bot commented Jul 24, 2024 •

edited

Loading

hwinkr left a comment

hwinkr Jul 24, 2024 •

edited

Loading

Largopie Jul 25, 2024

hwinkr Jul 24, 2024 •

edited

Loading

Largopie Jul 25, 2024

hwinkr Jul 24, 2024 •

edited

Loading

Yoonkyoungme Jul 25, 2024

Largopie Jul 25, 2024

hwinkr Jul 24, 2024 •

edited

Loading

Largopie Jul 25, 2024

Largopie Jul 25, 2024

Yoonkyoungme left a comment

Yoonkyoungme Jul 25, 2024

Largopie Jul 25, 2024

Yoonkyoungme Jul 25, 2024

Largopie left a comment

Largopie Jul 25, 2024

Largopie Jul 25, 2024

Largopie Jul 25, 2024

Largopie Jul 25, 2024

Largopie Jul 25, 2024

Largopie Jul 25, 2024

hwinkr left a comment

Yoonkyoungme left a comment

Yoonkyoungme Jul 25, 2024 •

edited

Loading

		if (endHours < startHours) return false;
		else if (endHours === startHours && endMinutes < startMinutes) return false;

		expect(result.current.startTime).toBe('0:00');
		expect(result.current.endTime).toBe('23:00');

[FE] Dropdown 컴포넌트 제작 및 시간 선택 hook 구현 #71

[FE] Dropdown 컴포넌트 제작 및 시간 선택 hook 구현 #71

Conversation

Largopie commented Jul 24, 2024

관련 이슈

작업 내용

특이 사항

리뷰 요구사항 (선택)

github-actions bot commented Jul 24, 2024 • edited Loading

Test Results

hwinkr left a comment

Choose a reason for hiding this comment

hwinkr Jul 24, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hwinkr Jul 24, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hwinkr Jul 24, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hwinkr Jul 24, 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

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

hwinkr left a comment

Choose a reason for hiding this comment

Yoonkyoungme left a comment

Choose a reason for hiding this comment

Yoonkyoungme Jul 25, 2024 • edited Loading

Choose a reason for hiding this comment

github-actions bot commented Jul 24, 2024 •

edited

Loading

hwinkr Jul 24, 2024 •

edited

Loading

hwinkr Jul 24, 2024 •

edited

Loading

hwinkr Jul 24, 2024 •

edited

Loading

hwinkr Jul 24, 2024 •

edited

Loading

Yoonkyoungme Jul 25, 2024 •

edited

Loading