글 작성/수정 폼 리팩토링해서 폼 훅 분리하기 #838

chsua · 2023-11-08T05:13:30Z

🔥 연관 이슈

close: #837

📝 작업 요약

글 작성/수정 폼 리팩토링
마감시간 오류 수정
리다이렉트 오류 수정

⏰ 소요 시간

1일

🔎 작업 상세 설명

글 작성/수정 폼 리팩토링
- 게시글 정보를 form-data로 바꾸는 과정 유틸함수로 분리
- 마감시간 관리하는 로직을 useDeadline 훅으로 분리
마감시간 오류 수정
- 모달 초기화를 누르면 게시글이 제출되는 로직 수정
- 모달 확인 누르면 유효시간 확인 안하고 바로 설정되는 오류 수정
리다이렉트 오류 수정
- 마감시간 여부를 검증하지 않아 진입되는 오류 수정
- 참여자가 있는지 검증하지 않아 진입되는 오류 수정

🌟 논의 사항

글 작성/수정 폼 리팩토링을 하면 많이 비워질 줄 알았는데, 생각보다 비워지지 않더라구요.

useText, useContentImage 등 훅을 사용하는 경우 이걸 다시 모아 리턴하는 것이 유의미할지 잘 모르겠어서 그대로 두었습니다. 분리를 하는 것도 의미가 있겠지만 파악하기 위한 뎁스가 깊어져 오히려 가독성을 떨어트릴 수 있다고 생각했습니다.

결과적으로 폼 안에는

훅(useText, useContentImage 등)
리다리엑트 검증 로직
이벤트 핸들러
모달용 button

위 로직들만 존재하고 있습니다. 혹시 더 뺄 수 있거나 리팩토링 가능해보인다면 말씀해주세요!

ps. 함수명 중 ---Formdata가 있는데, 이는 문법상 이상하지만 FormData라고 하면 오히려 폼데이터 형식처럼 와닿지 않을 것 같아서 붙여서 작성했습니다. 이에 대해 좋은 의견이 있다면 말씀해주세요!(변수명 너무 어렵네요😢)

- 수정 페이지 url로 접근 시 마감시간이 지난 글을 수정할 수 있는 오류 수정 - 타임피커 모달을 확인을 누르면 시간 검증없이 시간이 설정되는 오류 수정

- 폼데이터를 만드는 정보객체 재활용 - 마감시간 정보가 stringData 타입으로 수정되며 0일 0시간 0분을 검열하기 어려워짐 -> 코드 삭제

- 모달에 넣는 buttonInfo가 ButtonHTMLAttributes 상속하게 만들어 type을 지정할 수 있도록 함 - 모달과 폼에서 모두 사용되어 타입을 전역 타입폴더로 분리

github-actions · 2023-11-08T05:15:28Z

⚡️ Lighthouse report!

Category	Score
🟠 Performance	50
🟠 Accessibilty	89
🟢 SEO	100
🟠 PWA	89

Category	Score
🟢 First Contentful Paint	0.7 s
🔴 Largest Contentful Paint	5.7 s
🔴 Total Blocking Time	1,990 ms
🟢 Cumulative Layout Shift	0.046
🟠 Speed Index	3.6 s

Gilpop8663

복잡했던 PostForm이 훨씬 읽기 좋아졌다고 생각해요 👍

그리고 저도 수아의 의견처럼 useContent와 useText를 모아서 반환하는 함수는 과하다고 생각이 들어요

Gilpop8663 · 2023-11-08T05:58:00Z

frontend/src/components/PostForm/index.tsx

+      const errorMessage = checkValidationPost(writingPostInfo);
+      if (errorMessage) return addMessage(errorMessage);
+
+      const formData = convertToFormdata(writingPostInfo);


const formData = convertToFormData(writingPostInfo);

카멜케이스로 변경 가능할까요?

이 부분이 일부로 Formdata로 하신 부분이군요!

저는 FormData와 Formdata의 미묘한 차이를 잘 모르겠어요. 변수명 짓는것은 정말 어렵네요

convertToPostFormData 은 어떠세요?

저도 이 함수가 글 작성 폼에 한정되어 있다고 생각해서 우스의 의견에 동의합니다!

Gilpop8663 · 2023-11-08T06:04:43Z

frontend/src/components/PostForm/index.tsx

  };

-  const secondaryButton = {
+  const secondaryButton: ButtonInfo = {


모달의 Button 관련 타입이 types로 나오게 된 이유가 궁금했는데 사용하는 곳에서 타입을 선언해줘야 했군요!

ButtonInfo도 좋지만 Modal 관련 버튼 타입이니까 ModalButton 은 어떠세요?

Gilpop8663 · 2023-11-08T06:27:41Z

frontend/src/utils/post/convertToFormdata.ts

+
+import { deleteOverlappingNewLine } from '@utils/deleteOverlappingNewLine';
+
+export const convertToFormdata = ({


유틸 함수로 분리되니 코드 가독성이 올라갔어요 😀

Gilpop8663 · 2023-11-08T06:28:52Z

frontend/src/components/PostForm/index.tsx

+  if (deadline && Number(new Date(deadline)) < Date.now()) {
+    addMessage('마감완료된 게시물은 수정할 수 없습니다.');
+    return <Navigate to={PATH.HOME} />;
+  }
+
+  if (postId && writer && !checkWriter(writer.id)) {
+    addMessage('사용자가 작성한 글만 수정할 수 있습니다.');
+    return <Navigate to={PATH.HOME} />;
+  }
+
+  if (serverVoteInfo && serverVoteInfo.allPeopleCount !== 0) {
+    addMessage('투표한 사용자가 있어 글 수정이 불가합니다.');
+    return <Navigate to={PATH.HOME} />;
+  }


예외 처리로 서비스 완성도가 올라갔네요 👍👍👍

Gilpop8663 · 2023-11-08T06:38:55Z

fe-리뷰완

inyeong-kang

와우 수아~~ 리팩터링 달인이시네요!
가장 리팩터링이 시급한 컴포넌트라고 생각했는데 나서주셔서 감사합니다👍

추가로 수정했으면 좋을 부분 코멘트로 남겼습니다~~

inyeong-kang · 2023-11-09T01:54:12Z

frontend/src/components/PostForm/index.tsx

+  if (deadline && Number(new Date(deadline)) < Date.now()) {
+    addMessage('마감완료된 게시물은 수정할 수 없습니다.');
+    return <Navigate to={PATH.HOME} />;
+  }
+
+  if (postId && writer && !checkWriter(writer.id)) {
+    addMessage('사용자가 작성한 글만 수정할 수 있습니다.');
+    return <Navigate to={PATH.HOME} />;
+  }
+
+  if (serverVoteInfo && serverVoteInfo.allPeopleCount !== 0) {
+    addMessage('투표한 사용자가 있어 글 수정이 불가합니다.');
+    return <Navigate to={PATH.HOME} />;
+  }


음 이 3가지 validation도 함수로 분리해보는거 어떨까요?
3가지 유효성 검사(수정 가능한지 아닌지 검사)를 통과하면 isValid 가 true가 되고,

if(!isPostFormEditValid) { addMessage('유효하지 않은 이유'); return <Navigate to={PATH.HOME} />; }

요런 식으로 리팩터링할 여지가 보이네요!!

inyeong-kang · 2023-11-09T01:54:26Z

frontend/src/components/PostForm/index.tsx

+    resetDeadline,
+    getFinalDeadline,
+    getLimitDeadline,
+  } = useDeadline(createTime, deadline);


👍👍👍

궁금한 점! 혹시 파일명과 함수명을 다르게 하신 이유가 있나요??

오 중간에 수정했는데 신경쓰지 못했습니다 수정하겠습니다!

inyeong-kang · 2023-11-09T01:55:42Z

frontend/src/components/PostForm/index.tsx

+  if (serverVoteInfo && serverVoteInfo.allPeopleCount !== 0) {
+    addMessage('투표한 사용자가 있어 글 수정이 불가합니다.');
+    return <Navigate to={PATH.HOME} />;
+  }

  // 마감시간 관련 핸들러
  const handleDeadlineButtonClick = (option: DeadlineOptionInfo) => {


이 함수 로직도 useDeadline hook에 포함시키는건 어떠신가요?
hook 하나 분리한 김에 관심사 같은 것은 최대한 하나로 빼내는 것도 좋을거 같아서요!

저도 고민했는데, 핸들러 로직은 버튼이나 모달 등 다른 정보와 관련있는데 훅 안으로 옮기는 것이 관심사가 섞이는 것이라고 생각했습니다. 그래서 마감시간을 수정하는 로직만 훅으로 두고 핸들러에서 훅에서 return 함수/변수를 조작-이용하는 것이 좋다고 생각했습니다!

inyeong-kang · 2023-11-09T01:58:05Z

frontend/src/components/PostForm/index.tsx

-    setSelectTimeOption(option.name);
-    setUserSelectTime(targetTime);
+
+    changeDeadlineOption(option);
  };

  const handleResetButton = () => {


이 함수도 useDeadline hook에 포함시키는게 좋을거 같아요!
(handleResetDeadline 이라는 함수명으로)

inyeong-kang · 2023-11-09T01:59:32Z

frontend/src/components/PostForm/index.tsx

+      const errorMessage = checkValidationPost(writingPostInfo);
+      if (errorMessage) return addMessage(errorMessage);
+
+      const formData = convertToFormdata(writingPostInfo);


저도 이 함수가 글 작성 폼에 한정되어 있다고 생각해서 우스의 의견에 동의합니다!

inyeong-kang · 2023-11-09T02:02:21Z

frontend/src/components/PostForm/index.tsx

-                  hour: userSelectTime.hour,
-                  day: userSelectTime.day,
-                  minute: userSelectTime.minute,
+                  hour: userSelectedDHMTime.hour,


👍👍구체적인 변수명으로 의미가 더 명확해졌네요!!

inyeong-kang · 2023-11-09T02:03:14Z

frontend/src/components/PostForm/type.ts

+
+import { Option } from '@components/common/MultiSelect/types';
+
+interface OptionWithIsServerId extends WritingVoteOptionType {


혹시 OptionWithIsServerId 에서 Is 는 어떤 의미인지 궁금합니다!

타입 확장 좋아요!!!👍

여기서 isServerId는 클라이언트(프론트엔드)에서만 가지는 필드입니다
서버에서 준 선택지와 사용자가 입력한 선택지를 구분하기 위함으로 해당 필드가 포함된 옵션 타입이라고 명시하기 위함입니다.

inyeong-kang · 2023-11-09T02:04:31Z

frontend/src/components/common/Modal/Modal.stories.tsx

@@ -217,7 +230,7 @@ export const WithTimePicker = () => {
        >
          <S.Body>
            <S.Description>최대 {MAX_DEADLINE}일을 넘을 수 없습니다.</S.Description>
-            <TimePickerOptionList time={time} setTime={setTime} />
+            <TimePickerOptionList time={time} setTime={changeDeadlinePicker} />


오 구체적인 상태가 되었네요!👍

어떤 스토리인지 이름도 같이 구체화하면 좋을거 같아서, 혹시 WithTimePicker 라는 이름 대신 WithDeadlineTimePicker 로 바꿔주실 수 있나요?!

inyeong-kang · 2023-11-09T02:07:25Z

frontend/src/components/common/TimePickerOptionList/TimePickerOptionList.stories.tsx

  return (
    <>
-      <TimePickerOptionList time={time} setTime={setTime} />
+      <TimePickerOptionList time={time} setTime={changeDeadlinePicker} />


함수명 바꾼 김에 Default 라는 스토리 이름 대신 Deadline 으로 바꿔주실 수 있나요?

inyeong-kang · 2023-11-09T02:22:17Z

fe-리뷰완

chsua added 6 commits November 7, 2023 21:27

refactor: (#837) 변수명 수정 (deadLine -> deadline)

fe65dde

refactor: (#837) 마감시간 관련 훅 분리

904cb2c

fix: (#837) 마감시간 설정 오류 및 리다이렉트 오류 해결

e0c0190

- 수정 페이지 url로 접근 시 마감시간이 지난 글을 수정할 수 있는 오류 수정 - 타임피커 모달을 확인을 누르면 시간 검증없이 시간이 설정되는 오류 수정

refactor: (#837) 게시물 정보를 폼데이터형식으로 변환함수 분리

16bf812

refactor: (#837) 게시물 정보 유효성 검사 인수 형식 객체로 수정 및 기존 타입 활용

3e49d0d

- 폼데이터를 만드는 정보객체 재활용 - 마감시간 정보가 stringData 타입으로 수정되며 0일 0시간 0분을 검열하기 어려워짐 -> 코드 삭제

chsua added FE 🐾 개발 🐛 버그 labels Nov 8, 2023

chsua requested review from Gilpop8663 and inyeong-kang November 8, 2023 05:13

chsua self-assigned this Nov 8, 2023

Gilpop8663 approved these changes Nov 8, 2023

View reviewed changes

inyeong-kang approved these changes Nov 9, 2023

View reviewed changes

chsua added 4 commits November 14, 2023 12:34

refactor: (#837) formdata -> formData로 수정

67ecea2

refactor: (#837) type buttonInfo -> ModalButton 수정

f966d60

refactor: (#837) useDeadline 함수 파일명 수정

08c4f8f

refactor: (#837) 모달 테스트 스토리명 변경

24b6053

chsua merged commit fdca208 into dev Nov 14, 2023
1 check failed

woo-chang deleted the feat/#837 branch December 6, 2023 07:07

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

글 작성/수정 폼 리팩토링해서 폼 훅 분리하기 #838

글 작성/수정 폼 리팩토링해서 폼 훅 분리하기 #838

chsua commented Nov 8, 2023

github-actions bot commented Nov 8, 2023

Gilpop8663 left a comment

Gilpop8663 Nov 8, 2023

Gilpop8663 Nov 8, 2023

inyeong-kang Nov 9, 2023

Gilpop8663 Nov 8, 2023

Gilpop8663 Nov 8, 2023

Gilpop8663 Nov 8, 2023

Gilpop8663 commented Nov 8, 2023

inyeong-kang left a comment •

edited

Loading

inyeong-kang Nov 9, 2023

inyeong-kang Nov 9, 2023

inyeong-kang Nov 9, 2023

chsua Nov 14, 2023

inyeong-kang Nov 9, 2023

chsua Nov 14, 2023

inyeong-kang Nov 9, 2023

inyeong-kang Nov 9, 2023

inyeong-kang Nov 9, 2023

inyeong-kang Nov 9, 2023

chsua Nov 14, 2023

inyeong-kang Nov 9, 2023

inyeong-kang Nov 9, 2023

inyeong-kang Nov 9, 2023

inyeong-kang commented Nov 9, 2023


		import { deleteOverlappingNewLine } from '@utils/deleteOverlappingNewLine';

		export const convertToFormdata = ({


		import { Option } from '@components/common/MultiSelect/types';

		interface OptionWithIsServerId extends WritingVoteOptionType {

글 작성/수정 폼 리팩토링해서 폼 훅 분리하기 #838

글 작성/수정 폼 리팩토링해서 폼 훅 분리하기 #838

Conversation

chsua commented Nov 8, 2023

🔥 연관 이슈

📝 작업 요약

⏰ 소요 시간

🔎 작업 상세 설명

🌟 논의 사항

github-actions bot commented Nov 8, 2023

Gilpop8663 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

Gilpop8663 commented Nov 8, 2023

inyeong-kang 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

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

inyeong-kang commented Nov 9, 2023

inyeong-kang left a comment •

edited

Loading