[FE] 해커톤 구현 내용 리팩터링 #49

Largopie · 2024-07-22T09:44:02Z

작업 내용

.vscode 내부 setting.json → settings.json 파일로 수정
TimePicker와 Button 분리 -> TimePicker & TimeViewer 컴포넌트 나누기
TimeSlot 컴포넌트 분리
query(서버 상태 관리) 레이어 분리 ⇒ stores/servers 하위

특이 사항

invalidateQueries 사용으로 인한 화면 깜빡임

2024-07-22.6.38.56.mov

기능 구현 하면서 정리했던 자료 첨부

https://paper-mass-5ff.notion.site/2d94c34cbb87470193e7e824c0114afc?pvs=4

리뷰 요구사항 (선택)

최대한 회의한 내용 기반으로 파일 수정했습니다! 아무래도 리팩터링을 진행한 만큼 코드에 잘못된 부분이 없는지 확인 한 번 부탁드립니다요 :)

추가적으로 화면 깜빡임을 어떻게 해결할 건지에 대해서도 얘기를 나누어보는게 좋겠어요!
(참고) MSW는 현재 서버 측에서 정상 작동되는지 확인하기 위해서 별도로 구현하지 않았습니다

Yoonkyoungme

낙타! 리팩터링 작업 쉽지 않았을 것 같은게 느껴졌어요🥹 정말 고생하셨습니다
빠른 시간 내에 깔끔하게 리팩터링 하셨다니 !! 대단합니다👍👍👏👏👏👏
코멘트 확인해보시고, 논의하고 싶은 부분이 있다면 편하게 멘션 주세요!

Yoonkyoungme · 2024-07-23T05:56:07Z

frontend/src/components/_common/TimeSlot/TimeSlot.styles.ts

+import theme from '@styles/theme';
+
+export const styledTd = (isSelected: boolean, isUpdate: boolean) => css`
+  cursor: pointer;


[p2]
현재 보기 모드(수정 버튼 안 누른 상태)에서도 cursor: pointer;가 적용되어 있어서, 사용자 입장에서는 드래그 가능한 것처럼 느껴질 것 같아요. not-allowed 속성을 적용해 보는건 어떨까요?
참고: https://developer.mozilla.org/en-US/docs/Web/CSS/cursor

P2

🤚 빙봉 의견에 대한 답 + 의견 제시

저희 2차 스프린트 기능에 테이블 셀 한칸을 클릭하면 해당 시간을 클릭한, 약속 참여자들의 이름 목록을 툴팁 형태로 보여준다.라는 기능이 있어서, 보기 모드에서도 td가 버튼의 역할을 해야하니 cursor : pointer;로 유지하는게 좋을 것 같습니다.

추후에 보기모드일 때 드래그를 하려고 하면, 토스트로 경고 메시지를 보여주면 좋을 것 같네요!

테이블 셀 한칸을 클릭하면 해당 시간을 클릭한, 약속 참여자들의 이름 목록을 툴팁 형태로 보여준다.

해리의 의견과 정확히 일치합니다! 추후에 확장성을 고려해서 이렇게 pointer로 유지하게 되었습니다 :)

Yoonkyoungme · 2024-07-23T06:12:21Z

frontend/src/apis/schedule.ts

[Q]
해당 파일명이 schedule인데, schedule 관련 get과 post api 요청함수를 포괄하는 의미인가요?

[P3]
의도가 제가 작성한 것과 동일하다면 schedule 혹은 schedules 라는 디렉터리를 두고, 그 내부에 get-, post- 파일을 만드는건 어떨까요?

해당 파일명이 schedule인데, schedule 관련 get과 post api 요청함수를 포괄하는 의미인가요?

맞습니다!

의도가 제가 작성한 것과 동일하다면 schedule 혹은 schedules 라는 디렉터리를 두고, 그 내부에 get-, post- 파일을 만드는건 어떨까요?

저는 보통 파일단위로 분리하기보다는 비슷한 로직은 한꺼번에 묶어서 표현하는 편이 좋다고 생각했습니다! 나중에 get, post를 함께 보고 싶은 경우가 있으면 두 개의 파일을 클릭해서 봐야할 것 같다는 생각이 들었어요! 물론 여기서 api가 너무 많아지면 분리해야 할 필요성을 느끼겠지만 schedule이라는 컨텍스트에 묶여있는 api가 가독성에 큰 영향을 줄 것 같다는 생각은 들지 않았던 것 같아요!

빙봉은 파일을 분리하도록 제안하신 이유가 있을까요?

만약 수정한다면 schedules.ts로 복수형으로 표현해주는 편이 더 좋긴 하겠네요!

Yoonkyoungme · 2024-07-23T06:14:18Z

frontend/src/pages/MeetingTimePickPage/index.tsx


 import responseData from '@mocks/data.json';

 import { buttonContainer, title } from './MeetingTimePickPage.styles';
 import { convertToSchedule, generateScheduleMatrix } from './MeetingTimePickPage.utils';

+// import { useGetMeetingQuery } from 'stores/servers/meeting/queries';
+
 export default function MeetingTimePickPage() {
  // TODO: 임시 데이터 설정, 추후에 msw로 연결 수정


TODO 주석 좋네요~👍

Yoonkyoungme · 2024-07-23T06:39:00Z

frontend/src/stores/servers/meeting/mutations.ts

+
+import { QUERY_KEY } from '@constants/queryKeys';
+
+export const usePostScheduleMutation = () => {


[P3]
상위 디렉터리명이 mutations이고, 함수명에서도 post가 mutation을 사용한다는 것을 충분히 전달하고 있다고 생각합니다. 함수명에서 Mutation을 제거하는 것은 어떨까요? (usePostScheduleMutation → usePostSchedule)

Yoonkyoungme · 2024-07-23T06:42:28Z

frontend/src/stores/servers/meeting/queries.ts

+
+import { QUERY_KEY } from '@constants/queryKeys';
+
+export const useGetMeetingQuery = () =>


[P3]
useGetMeetingQuery 함수명도 위의 usePostScheduleMutation 코멘트와 동일한 맥락입니다!
(useGetMeetingQuery → useGetMeeting)

해당 코멘트는 가벼운 제안이니 낙타의 생각을 공유해 주시면 좋을 것 같아요 ~~🌟

저도 네이밍에 대해서 약간이나마 고민을 했었는데, 이는 Mutation, Query임을 더욱 잘 명시하기 위해서 사용하게 되었습니다.

만약 useGetMeeting이라고 명시되어 있는 경우에는 프로젝트를 처음 접하는 사람이라면 해당 로직을 hook이라고 이해할 것 같아 이를 찾기 위해서 hook 폴더를 찾아볼 수도 있겠다는 생각이 들었어요! 따라서 더욱 명확하게 구분하기 위해서 사용한 네이밍인데 빙봉은 어떻게 생각하시나요?!

[C]

@hwinkr @Largopie
흠 낙타의 의견도 고려해 봐야겠네요.
해당 코멘트에 관한건 이번 스프린트 끝나고 다같이 한 번 이야기해서 컨벤션을 정해봅시다!

Yoonkyoungme · 2024-07-23T06:59:28Z

frontend/src/components/Time/Picker/TimePicker.util.ts

@@ -1,11 +1,11 @@
-import { TimePickerProps } from '.';
+import { getMeetingResponse } from '@apis/getMeeting';


[P1]
타입 import 할 때 type 붙이는게 컨벤션이었던 것 같아요!

Suggested change

import { getMeetingResponse } from '@apis/getMeeting';

import type { getMeetingResponse } from '@apis/getMeeting';

[P3]
그리고 해당 타입은 여러 곳에서 사용되니까 따로 파일로 분리하는 것도 고민해봐도 좋을 것 같아요!

앗차,, 이 부분도 ESLint 규칙 설정 후 formatOnSave로 적용하기로 했었는데, 해커톤 하면서 규칙을 삭제하게 되어버렸네요..!
해당 규칙 추가하고 import type 수정하도록 하겠습니다 :) 👍

꼼꼼한 리뷰 감사해요!

Yoonkyoungme · 2024-07-23T07:28:46Z

frontend/src/pages/MeetingTimePickPage/index.tsx

+import TimePicker from '@components/Time/Picker';
+import TimeViewer from '@components/Time/Viewer';
+
+import { useGetMeetingQuery } from '@stores/servers/meeting/queries';

 import { title } from './MeetingTimePickPage.styles';


[P1]
emotion css prop 네이밍 컨벤션을 <name>Style으로 정했던 것 같습니다!

해당 부분 모두 수정했습니다!

추가적으로 질문이 있는데, ~Container와 같은 네이밍을 사용한 경우에도 ~ContainerStyle로 명시를 해주는 것이 좋을까요?
제 개인적인 의견으로는 너무 길어질 것 같아서 <name>Style postfix를 붙이지 않아도 괜찮을 것 같다는 생각이 들었습니다!

[기록]
07/24 FE 회의 때 css prop 네이밍은 s_ prefix를 붙이기로 컨벤션을 정함 (wiki에 기록 완료)

Yoonkyoungme · 2024-07-23T07:35:59Z

frontend/src/pages/MeetingTimePickPage/index.tsx

  return (
    <div>
      <h1 css={title}>momo TimePicker</h1>
-      {data && <TimePicker {...data} />}
+      {data && getUpdateState() && <TimePicker data={data} />}
+      {data && !getUpdateState() && <TimeViewer data={data} />}
    </div>
  );


[P3]
return 문을 좀 더 간결하게 가져가고 싶으면 아래 코드처럼 바꿔볼 수도 있을 것 같아요!

const renderContent = () => { if (!data) return null; return getUpdateState() ? <TimePicker data={data} /> : <TimeViewer data={data} />; }; return ( <div> <h1 css={title}>momo TimePicker</h1> {renderContent()} </div> );

컴포넌트 내부에 컴포넌트를 선언하면 안티패턴이락고 하더라구요..! 참고 자료도 함께 첨부합니다!

https://ko.react.dev/learn/your-first-component#nesting-and-organizing-components

저는 개인적으로 함수형 컴포넌트가 return문에 JSX를 반환하니, 어떤 JSX를 반환할지 결정하는 함수를 따로 구현하기 보다는 지금처럼 return문 내에 조건부 렌더링 로직을 가지고 있는 방법이 더 보기 편한 것 같아요!

빙봉과 의견이 반대라서 의견을 나눠봐야할 것 같네요..

@Yoonkyoungme

Yoonkyoungme · 2024-07-23T07:39:13Z

frontend/src/contexts/updateStateProvider.tsx

+  {} as UpdateStateContextProps,
+);
+
+export const UpdateStateProvider = ({ children }: PropsWithChildren) => {


[P2]
파일명이랑 함수명 대소문자 일치시켜 주세요!

위에 해리가 남긴 네이밍 수정 이슈로 함께 수정 완료했습니다 리뷰 고마워요! :) 👍

github-actions · 2024-07-23T10:43:44Z

Test Results

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

Results for commit 996832c.

♻️ This comment has been updated with latest results.

hwinkr

리팩터링 고생했습니다!

오늘 오후에 낙관적 업데이트와 관련해서 같이 리팩터링 했던 내용도 반영해서 올려주시면 좋을 것 같아요!

hwinkr · 2024-07-23T10:40:50Z

frontend/src/contexts/updateStateProvider.tsx

@@ -0,0 +1,26 @@
+import { PropsWithChildren, createContext, useCallback, useState } from 'react';
+
+interface UpdateStateContextProps {


P2

Suggested change

interface UpdateStateContextProps {

interface UpdateStateContextProps {

isUpdate: boolean;

handleToggleIsUpdate: () => void;

}

상태값을 반환하는 함수를 Provider를 사용해서 내려주기 보다, 불리언 타입의 값 자체를 내려주는 것이 어떨까요~?

저도 왜 이게 안될까를 수 없이 고민하다가 이렇게 적용을 했었는데, 되네요... ㅠ
반영해주셔서 감사합니다 해리 :)

hwinkr · 2024-07-23T10:41:33Z

frontend/src/contexts/updateStateProvider.tsx

+  const handleToggleIsUpdate = useCallback(() => {
+    setIsUpdate((prevState) => !prevState);
+  }, []);


토글 기능이 있는 함수를 만들어서 Provider를 내려주는 것은 좋네요 👏

그렇죠!! 🐫

hwinkr · 2024-07-23T10:42:31Z

frontend/src/apis/getMeeting.ts

  schedules: Schedules[];
 }


P1

Suggested change

schedules: Schedules[];

}

export interface GetMeetingResponse {

인터페이스 네이밍을 파스칼 케이스로 통일하면 좋을 것 같아요!

아이고.. 이 부분을 놓쳤네요..! 수정하겠습니다 :)

꼼꼼한 해리 감사해요!

hwinkr · 2024-07-23T10:46:37Z

frontend/src/components/_common/TimeSlot/TimeSlot.styles.ts

+import theme from '@styles/theme';
+
+export const styledTd = (isSelected: boolean, isUpdate: boolean) => css`
+  cursor: pointer;


P2

🤚 빙봉 의견에 대한 답 + 의견 제시

저희 2차 스프린트 기능에 테이블 셀 한칸을 클릭하면 해당 시간을 클릭한, 약속 참여자들의 이름 목록을 툴팁 형태로 보여준다.라는 기능이 있어서, 보기 모드에서도 td가 버튼의 역할을 해야하니 cursor : pointer;로 유지하는게 좋을 것 같습니다.

추후에 보기모드일 때 드래그를 하려고 하면, 토스트로 경고 메시지를 보여주면 좋을 것 같네요!

hwinkr · 2024-07-23T10:48:14Z

frontend/src/contexts/updateStateProvider.tsx

+export const UpdateStateContext = createContext<UpdateStateContextProps>(
+  {} as UpdateStateContextProps,
+);


P2

어떤 상태를 업데이트 하는지 이름이 길더라도 자세하게 알려주는 것은 어떨까요? 컨텍스트 이름만 봤을 때, 어떤 역할을 수행하기 위해서 존재하는 컨텍스트인지 파악하기 힘든 것 같아요!

좋은 의견 감사합니다. 확실히 어떤 상태를 업데이트 해야하는 지 파악하기 어려울 수 있을 것 같아요.

굉장히 고심 끝에 결정한 네이밍은 TimePickerUpdateState입니다.

TimePicker, TimeViewer를 모두 포괄하고 있는 단어인 Time을 사용하기엔 '시간 선택 상태'라는 것을 모두 표현해주기에는 모호하다고 생각했으며, 그렇다고 Timetable이란 단어를 사용하기에는 용어 정리가 되지 않는 듯 하여 가장 처음 구현했던 이름인 TimePicker를 사용하는 편이 좋다고 판단이 들어 위와 같은 결정을 하게 되었습니다!

해리가 느끼기에 명확한가요?

hwinkr · 2024-07-23T10:50:43Z

frontend/src/pages/MeetingTimePickPage/index.tsx

  return (
    <div>
      <h1 css={title}>momo TimePicker</h1>
-      {data && <TimePicker {...data} />}
+      {data && getUpdateState() && <TimePicker data={data} />}
+      {data && !getUpdateState() && <TimeViewer data={data} />}
    </div>
  );


저는 개인적으로 함수형 컴포넌트가 return문에 JSX를 반환하니, 어떤 JSX를 반환할지 결정하는 함수를 따로 구현하기 보다는 지금처럼 return문 내에 조건부 렌더링 로직을 가지고 있는 방법이 더 보기 편한 것 같아요!

빙봉과 의견이 반대라서 의견을 나눠봐야할 것 같네요..

@Yoonkyoungme

Largopie

@Yoonkyoungme @hwinkr

리뷰 모두 반영 했습니다! 꼼꼼한 리뷰 감사해요 :)

확인 부탁드리겠습니다~! 🐫

낙관적 업데이트에 도움을 주신 해리 감사합니다 🙇‍♂️

Largopie · 2024-07-23T08:04:51Z

frontend/src/apis/schedule.ts

해당 파일명이 schedule인데, schedule 관련 get과 post api 요청함수를 포괄하는 의미인가요?

맞습니다!

의도가 제가 작성한 것과 동일하다면 schedule 혹은 schedules 라는 디렉터리를 두고, 그 내부에 get-, post- 파일을 만드는건 어떨까요?

저는 보통 파일단위로 분리하기보다는 비슷한 로직은 한꺼번에 묶어서 표현하는 편이 좋다고 생각했습니다! 나중에 get, post를 함께 보고 싶은 경우가 있으면 두 개의 파일을 클릭해서 봐야할 것 같다는 생각이 들었어요! 물론 여기서 api가 너무 많아지면 분리해야 할 필요성을 느끼겠지만 schedule이라는 컨텍스트에 묶여있는 api가 가독성에 큰 영향을 줄 것 같다는 생각은 들지 않았던 것 같아요!

빙봉은 파일을 분리하도록 제안하신 이유가 있을까요?

만약 수정한다면 schedules.ts로 복수형으로 표현해주는 편이 더 좋긴 하겠네요!

Largopie · 2024-07-23T08:13:52Z

frontend/src/pages/MeetingTimePickPage/index.tsx

  return (
    <div>
      <h1 css={title}>momo TimePicker</h1>
-      {data && <TimePicker {...data} />}
+      {data && getUpdateState() && <TimePicker data={data} />}
+      {data && !getUpdateState() && <TimeViewer data={data} />}
    </div>
  );


컴포넌트 내부에 컴포넌트를 선언하면 안티패턴이락고 하더라구요..! 참고 자료도 함께 첨부합니다!

https://ko.react.dev/learn/your-first-component#nesting-and-organizing-components

Largopie · 2024-07-23T13:31:44Z

frontend/src/apis/getMeeting.ts

  schedules: Schedules[];
 }


아이고.. 이 부분을 놓쳤네요..! 수정하겠습니다 :)

꼼꼼한 해리 감사해요!

Largopie · 2024-07-23T13:40:13Z

frontend/src/components/Time/Picker/TimePicker.util.ts

@@ -1,11 +1,11 @@
-import { TimePickerProps } from '.';
+import { getMeetingResponse } from '@apis/getMeeting';


앗차,, 이 부분도 ESLint 규칙 설정 후 formatOnSave로 적용하기로 했었는데, 해커톤 하면서 규칙을 삭제하게 되어버렸네요..!
해당 규칙 추가하고 import type 수정하도록 하겠습니다 :) 👍

꼼꼼한 리뷰 감사해요!

Largopie · 2024-07-23T13:41:32Z

frontend/src/components/_common/TimeSlot/TimeSlot.styles.ts

+import theme from '@styles/theme';
+
+export const styledTd = (isSelected: boolean, isUpdate: boolean) => css`
+  cursor: pointer;


테이블 셀 한칸을 클릭하면 해당 시간을 클릭한, 약속 참여자들의 이름 목록을 툴팁 형태로 보여준다.

해리의 의견과 정확히 일치합니다! 추후에 확장성을 고려해서 이렇게 pointer로 유지하게 되었습니다 :)

Largopie · 2024-07-23T14:00:24Z

frontend/src/contexts/updateStateProvider.tsx

+export const UpdateStateContext = createContext<UpdateStateContextProps>(
+  {} as UpdateStateContextProps,
+);


좋은 의견 감사합니다. 확실히 어떤 상태를 업데이트 해야하는 지 파악하기 어려울 수 있을 것 같아요.

굉장히 고심 끝에 결정한 네이밍은 TimePickerUpdateState입니다.

TimePicker, TimeViewer를 모두 포괄하고 있는 단어인 Time을 사용하기엔 '시간 선택 상태'라는 것을 모두 표현해주기에는 모호하다고 생각했으며, 그렇다고 Timetable이란 단어를 사용하기에는 용어 정리가 되지 않는 듯 하여 가장 처음 구현했던 이름인 TimePicker를 사용하는 편이 좋다고 판단이 들어 위와 같은 결정을 하게 되었습니다!

해리가 느끼기에 명확한가요?

Largopie · 2024-07-23T14:04:54Z

frontend/src/contexts/updateStateProvider.tsx

+  {} as UpdateStateContextProps,
+);
+
+export const UpdateStateProvider = ({ children }: PropsWithChildren) => {


위에 해리가 남긴 네이밍 수정 이슈로 함께 수정 완료했습니다 리뷰 고마워요! :) 👍

Largopie · 2024-07-23T14:05:16Z

frontend/src/contexts/updateStateProvider.tsx

+  const handleToggleIsUpdate = useCallback(() => {
+    setIsUpdate((prevState) => !prevState);
+  }, []);


그렇죠!! 🐫

Largopie · 2024-07-23T14:12:14Z

frontend/src/pages/MeetingTimePickPage/index.tsx

+import TimePicker from '@components/Time/Picker';
+import TimeViewer from '@components/Time/Viewer';
+
+import { useGetMeetingQuery } from '@stores/servers/meeting/queries';

 import { title } from './MeetingTimePickPage.styles';


해당 부분 모두 수정했습니다!

추가적으로 질문이 있는데, ~Container와 같은 네이밍을 사용한 경우에도 ~ContainerStyle로 명시를 해주는 것이 좋을까요?
제 개인적인 의견으로는 너무 길어질 것 같아서 <name>Style postfix를 붙이지 않아도 괜찮을 것 같다는 생각이 들었습니다!

Largopie · 2024-07-23T14:14:44Z

frontend/src/stores/servers/meeting/queries.ts

+
+import { QUERY_KEY } from '@constants/queryKeys';
+
+export const useGetMeetingQuery = () =>


저도 네이밍에 대해서 약간이나마 고민을 했었는데, 이는 Mutation, Query임을 더욱 잘 명시하기 위해서 사용하게 되었습니다.

만약 useGetMeeting이라고 명시되어 있는 경우에는 프로젝트를 처음 접하는 사람이라면 해당 로직을 hook이라고 이해할 것 같아 이를 찾기 위해서 hook 폴더를 찾아볼 수도 있겠다는 생각이 들었어요! 따라서 더욱 명확하게 구분하기 위해서 사용한 네이밍인데 빙봉은 어떻게 생각하시나요?!

hwinkr

s_ 컨벤션 추가만 될 것 같네요!

고생하셨ㅅ습니다~

hwinkr · 2024-07-24T05:36:44Z

frontend/src/pages/MeetingTimePickPage/index.tsx

  return (
    <div>
      <h1 css={titleStyle}>momo TimePicker</h1>
-      {data && isTimePickerUpdate && <TimePicker data={data} />}
-      {data && !isTimePickerUpdate && <TimeViewer data={data} />}
+      {isTimePickerUpdate ? <TimePicker data={data} /> : <TimeViewer data={data} />}


이 방법도 보기 좋네요, 이 방법대로 가시죠!

너무 보기 좋습니다👍🏻👍🏻

hwinkr · 2024-07-24T05:42:32Z

frontend/.eslintrc.json

@@ -26,6 +26,7 @@
    "plugin:storybook/recommended"
  ],
  "rules": {
+    "@typescript-eslint/consistent-type-imports": "error",


오 이 설정 좋은데?

hwinkr · 2024-07-24T05:49:38Z

frontend/src/contexts/TimePickerUpdateStateProvider.tsx

+  const handleToggleIsTimePickerUpdate = useCallback(() => {
+    setIsTimePickerUpdate((prevState) => !prevState);
+  }, []);


사소하지만, useCallback useMemo useEffect 훅과 같이 두 번째 인자로 의존성 배열을 받는 훅들은 훅 내부 함수에서 어떤 것들에 의존하고 있는지를 명시적으로 표현하는 것이 좋습니다. 의존하고 있는 요소들이 변할 때마다 함수가 새롭게 저장되거나 값이 새롭게 계산되어야 하기 때문입니다.

"react-hooks/exhaustive-deps": "warn" 과 같은 린트 설정으로, 미리 경고를 받아보는 건 어떨까요!

이미 eslint-plugin-react-hooks 의 recommended에 설정되어있는 속성인데, 에러가 안잡힌 것 같은데, 별도로 넣어주어야 하는 deps가 있나요?

https://github.com/facebook/react/blob/main/packages/eslint-plugin-react-hooks/src/index.js

Yoonkyoungme

낙타 🐫 고생하셨습니다👍🏻
ㅋㅋㅋㅋㅋㅋㅋ 대화로 코멘트 하고, 그 부분도 반영 완료하 주서서 특별히 더 수정해야 할 부분은 없어보입니다!
요구 사항 척척 반영해주기고, 제가 제안한 의견보다 훨씬 더 좋은 로직으로 반영해 주셔서 기쁘네요😆

Yoonkyoungme · 2024-07-24T09:02:41Z

frontend/src/stores/servers/meeting/queries.ts

+
+import { QUERY_KEY } from '@constants/queryKeys';
+
+export const useGetMeetingQuery = () =>


[C]

@hwinkr @Largopie
흠 낙타의 의견도 고려해 봐야겠네요.
해당 코멘트에 관한건 이번 스프린트 끝나고 다같이 한 번 이야기해서 컨벤션을 정해봅시다!

Yoonkyoungme · 2024-07-24T11:18:59Z

frontend/src/pages/MeetingTimePickPage/index.tsx

  return (
    <div>
      <h1 css={titleStyle}>momo TimePicker</h1>
-      {data && isTimePickerUpdate && <TimePicker data={data} />}
-      {data && !isTimePickerUpdate && <TimeViewer data={data} />}
+      {isTimePickerUpdate ? <TimePicker data={data} /> : <TimeViewer data={data} />}


너무 보기 좋습니다👍🏻👍🏻

Yoonkyoungme · 2024-07-24T11:20:25Z

frontend/src/components/Time/Picker/index.tsx

@@ -54,7 +58,7 @@ export default function TimePicker({ data }: TimePickerProps) {
                <TimeSlot
                  key={rowIndex + columnIndex}
                  isSelected={value[rowIndex][columnIndex]}
-                  isUpdate={isUpdate}
+                  isUpdate={isTimePickerUpdate}


오 훨씬 명확한 네이밍으로 바꼈네요👍🏻

- TimePicker내부 로직을 페이지 최상단으로 끌어 올림 - TimePicker util함수 TimePickerPage로 이동 - 가능 시간, 시작 시간, ref, value를 TimePicker props로 받도록 수정

…더 분리

- 네트워크가 느린 사용자를 고려하여 낙관적 업데이트 로직 추가 - onSettled가 실행된 후, 낙관적 업데이트가 끝난 후 수정, 보기 모드가 변경되도록 하여 깜빡임 현상 개선

- 타입 정의 import 시 import type 명시되도록 수정

Largopie added 🐈 프론트엔드 프론트엔드 관련 이슈에요 :) ♻️ 리팩터링 코드를 깎아요 :) labels Jul 22, 2024

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

Largopie requested review from hwinkr and Yoonkyoungme July 22, 2024 09:44

Largopie assigned hwinkr, Yoonkyoungme and Largopie Jul 22, 2024

Largopie requested review from ehBeak, ikjo39, hw0603, seokmyungham and seunghye218 as code owners July 22, 2024 09:44

Yoonkyoungme requested changes Jul 23, 2024

View reviewed changes

hwinkr requested changes Jul 23, 2024

View reviewed changes

Largopie commented Jul 23, 2024

View reviewed changes

Largopie force-pushed the feat/44-refactor-feature branch from fa46d4c to 1b0d155 Compare July 23, 2024 14:16

Largopie removed request for hw0603, ikjo39, ehBeak, seunghye218 and seokmyungham July 23, 2024 14:22

hwinkr approved these changes Jul 24, 2024

View reviewed changes

hwinkr reviewed Jul 24, 2024

View reviewed changes

Yoonkyoungme approved these changes Jul 24, 2024

View reviewed changes

Largopie added 4 commits July 24, 2024 20:39

chore: .vscode 세팅파일 수정

6aabe24

refactor: TimePicker 컴포넌트에서 Button 컴포넌트 분리

707d41b

- TimePicker내부 로직을 페이지 최상단으로 끌어 올림 - TimePicker util함수 TimePickerPage로 이동 - 가능 시간, 시작 시간, ref, value를 TimePicker props로 받도록 수정

design: button 기본 색상 'red' 및 border 제거

a5c58ac

chore: eslint jsx-a11y 일부 설정 "off"로 설정

f405e1b

Largopie and others added 24 commits July 24, 2024 20:39

refactor(TimeSlot): 시간 선택 박스 컴포넌트 분리

7b5ccc4

refactor: schedule api 컨텍스트 통합

50cdb94

refactor: 서버 상태 관리 로직 레이어 분리

1f9b183

comment: uuid 관련 주석 추가

958bb63

design(TimeSlot): 스타일 로직 통합

dec3d79

chore: stores, contexts 경로 추가

0c33c29

refactor: isUpdate 상태 context로 관리

decd119

refactor(TimePicker): UpdateProvider 로직 연결 및 Picker, Viewer과 나누기 위한 폴…

360ba72

…더 분리

style: css 선언 순서 수정

518117a

fix: post 요청 시 return 값 없도록 수정

7b733d9

feat: queryKey 상수 정의

cb8635f

refactor: mutation에 invalidateQueries 추가 및 queryKey 상수로 변경

681bc1c

refactor: update 상태 provider 연결 및 TimePicker, TimeViewer 분리 후 구현

96cc195

refactor: 기존 테이블을 구성할 때 사용했던 서버 상태의 '초' 단위 제거

ab0f629

refactor: context에서 관리하는 isUpdate 상태 값을 Provider 에서 내려주도록 변경

7fe63fa

chore: 인터페이스명 PascalCase로 수정

a520dda

chore: consistent-type-imports 규칙 추가 및 규칙 반영

eb386bc

- 타입 정의 import 시 import type 명시되도록 수정

refactor: 상수 정의 as const 사용

7639820

rename: update -> TimePickerUpdate로 네이밍 수정

a444887

rename: css prop 네이밍 <name>Style로 수정

e287ee4

refactor: 조건부 렌더링 코드 수정

ba54102

refactor: 페이지명 변경사항 router파일에 적용

5725b66

rename: css prop 네이밍 규칙에 따른 변수명 수정

996832c

Largopie force-pushed the feat/44-refactor-feature branch from da36c39 to 996832c Compare July 24, 2024 11:39

Largopie merged commit c92c9ae into develop Jul 24, 2024
5 checks passed

Largopie deleted the feat/44-refactor-feature branch July 24, 2024 11:40


		import { QUERY_KEY } from '@constants/queryKeys';

		export const usePostScheduleMutation = () => {


		import { QUERY_KEY } from '@constants/queryKeys';

		export const useGetMeetingQuery = () =>

		@@ -1,11 +1,11 @@
		import { TimePickerProps } from '.';
		import { getMeetingResponse } from '@apis/getMeeting';

	import { getMeetingResponse } from '@apis/getMeeting';
	import type { getMeetingResponse } from '@apis/getMeeting';

		@@ -0,0 +1,26 @@
		import { PropsWithChildren, createContext, useCallback, useState } from 'react';

		interface UpdateStateContextProps {

	schedules: Schedules[];
	}
	export interface GetMeetingResponse {

[FE] 해커톤 구현 내용 리팩터링 #49

[FE] 해커톤 구현 내용 리팩터링 #49

Conversation

Largopie commented Jul 22, 2024

관련 이슈

작업 내용

특이 사항

invalidateQueries 사용으로 인한 화면 깜빡임

기능 구현 하면서 정리했던 자료 첨부

리뷰 요구사항 (선택)

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

P2

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 Jul 23, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

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

Test Results

hwinkr left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

P2

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

P1

Choose a reason for hiding this comment

Choose a reason for hiding this comment

P2

Choose a reason for hiding this comment

P2

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Largopie 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

hwinkr 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

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

Yoonkyoungme Jul 23, 2024 •

edited

Loading

github-actions bot commented Jul 23, 2024 •

edited

Loading

Largopie left a comment •

edited

Loading