refactor: next button 분기문 리팩토링 #104

2yunseong · 2024-03-02T09:11:28Z

주요 변경사항

리팩토링이라고 할 것 도 없네요 .. ㅋㅋ 아마 기능개선하면서 만지신 분들이 다 잘해주셔서 할게 별로 없었습니다

리뷰어에게...

PR Comment에 남겨두겠습니다 !

관련 이슈

[FE] refactor: ApplicationNextButton 컴포넌트 클린코드 및 구조 변경 #46
(closed를 하는게 나을까요?)

2yunseong

@JNU-econovation/econorecruitefe
아래 코멘트 달린 부분 위주로 리뷰 해주시기 바랍니다~

2yunseong · 2024-03-02T09:13:17Z

frontend/components/application/applicationNode/NextButton.component.tsx

@@ -13,34 +13,29 @@ interface ApplicationNextButtonProps {
  isLast?: boolean;
 }

+// etc. 단순히 boolean이 아닌 어느 곳에서 터지는 지와 그 이유를 담은 객체를 반환하면 어떨까?


이 논의를 발제한 이유는, 아래 줄에(53L~55L) alert에 넘겨주는 문구를 조금 더 구체화 하고 싶어서 그랬습니다.
포트폴리오 동의여부에서 "동의하지 않습니다"를 선택해도 필수 항목을 입력해달라고 요청하는 게 플로우상 어색해 보인다고 생각했습니다.
물론 포트폴리오를 제출해놓고 동의하지 않는 사람은 없겠죠?ㅋㅋ

음 이부분은 TF분들도 원하시는 것 같아요!

좋은 것 같아요~

좋을 것 같아요!

저도 이게 좋은 것 같다는 생각이 있었는데, 구조상 조금 복잡해서 일단 두었었거든요!! ㅠㅠㅠㅠ

일단은 그럼, 다른 이슈로 빼둘께요 !

2yunseong · 2024-03-02T09:13:49Z

frontend/components/application/applicationNode/NextButton.component.tsx

+    const EMPTY_STRING: string = "";
+    const localStorageValueFromName = localStorage.get(name, EMPTY_STRING);


아래 중복되서 사용되는 부분을 전부 삭제하고 상위로 끌어올렸어요.
리팩토링 과정에서는 별 문제 없어 보였습니다. 이 부분 확인 부탁드립니다.

magic string을 제거한건 너무 감사하네요

2yunseong · 2024-03-02T09:16:33Z

frontend/components/application/applicationNode/NextButton.component.tsx

+    return (
+      !(localStorageValueFromName.length === 0) ||
+      (name === "channel" && localStorage.get("channelEtc", "").length !== 0)
+    );


이 부분이 이렇게 변경된 이유는 중첩구조문이 파악이 힘들고, 불필요한 조건이 많이 들어간 것 같아 카르노 맵으로 단순화 하였습니다. 아래는 참고를 위한 카르노 맵입니다.

조건

localStorage.get(name, "").length === 0

name === "channel"

localStorage.get("channelEtc", "").length !== 0

우와...깔끔하고 좋네요

? 미쳤다... ㄷㄷㄷㄷㄷㄷㄷㄷㄷㄷㄷ ㅋㅋㅋㅋㅋㅋㅋㅋㅋㅋㅋㅋ 아니 이게 가능하다고?

2yunseong · 2024-03-02T09:17:04Z

frontend/components/application/applicationNode/NextButton.component.tsx

@@ -57,7 +52,7 @@ const ApplicationNextButton = ({
    );
    if (!canNext(Array.from(applicationName))) {
      alert("필수 항목을 입력해주세요.");
-      return false;
+      return;


이벤트 핸들러에서 boolean을 반환할 필요가 없다고 생각해 undefined으로 변경하였습니다.

혹시 몰라 더 찾아봤는데, 바닐라 자바스크립트에서는 이벤트핸들러가 false를 반환하면 기본 동작(이벤트 버블링)을 취소한다네요!
하지만 React에서는 지원하지 않는다고 합니다. (레거시 문서지만 현재버전까지 유지되는 내용 같아요 ! )

https://ko.legacy.reactjs.org/docs/events.html

저도 습관적으로 저걸 반환하긴 했는데, react는 (0.14라면 꾀나 예전이네요 ㅋㅋㅋ) 안했었군요 !! ㅋㅋㅋㅋ

bada308

좋습니다!!

bada308 · 2024-03-02T09:19:21Z

frontend/components/application/applicationNode/NextButton.component.tsx

    }
-    return true;
+    return (


깔끔해졌네요.. 👍

frontend/components/application/applicationNode/NextButton.component.tsx

baegyeong

수고하셨습니다!

baegyeong · 2024-03-02T09:26:11Z

frontend/components/application/applicationNode/NextButton.component.tsx

@@ -13,34 +13,29 @@ interface ApplicationNextButtonProps {
  isLast?: boolean;
 }

+// etc. 단순히 boolean이 아닌 어느 곳에서 터지는 지와 그 이유를 담은 객체를 반환하면 어떨까?


좋은 것 같아요~

baegyeong · 2024-03-02T09:26:40Z

frontend/components/application/applicationNode/NextButton.component.tsx

+    return (
+      !(localStorageValueFromName.length === 0) ||
+      (name === "channel" && localStorage.get("channelEtc", "").length !== 0)
+    );


우와...깔끔하고 좋네요

Co-authored-by: Ocean <[email protected]>

geongyu09 · 2024-03-02T09:31:12Z

frontend/components/application/applicationNode/NextButton.component.tsx

@@ -13,34 +13,29 @@ interface ApplicationNextButtonProps {
  isLast?: boolean;
 }

+// etc. 단순히 boolean이 아닌 어느 곳에서 터지는 지와 그 이유를 담은 객체를 반환하면 어떨까?


좋을 것 같아요!

loopy-lim

좋네요!

loopy-lim · 2024-03-02T13:12:50Z

frontend/components/application/applicationNode/NextButton.component.tsx

@@ -13,34 +13,29 @@ interface ApplicationNextButtonProps {
  isLast?: boolean;
 }

+// etc. 단순히 boolean이 아닌 어느 곳에서 터지는 지와 그 이유를 담은 객체를 반환하면 어떨까?


저도 이게 좋은 것 같다는 생각이 있었는데, 구조상 조금 복잡해서 일단 두었었거든요!! ㅠㅠㅠㅠ

loopy-lim · 2024-03-02T13:15:11Z

frontend/components/application/applicationNode/NextButton.component.tsx

@@ -57,7 +52,7 @@ const ApplicationNextButton = ({
    );
    if (!canNext(Array.from(applicationName))) {
      alert("필수 항목을 입력해주세요.");
-      return false;
+      return;


loopy-lim · 2024-03-02T13:16:29Z

frontend/components/application/applicationNode/NextButton.component.tsx

+    const EMPTY_STRING: string = "";
+    const localStorageValueFromName = localStorage.get(name, EMPTY_STRING);


magic string을 제거한건 너무 감사하네요

refactor: next button 분기문 리팩토링

b46e863

2yunseong requested review from geongyu09, bada308 and baegyeong March 2, 2024 09:11

2yunseong self-assigned this Mar 2, 2024

2yunseong requested review from BlackBean99 and loopy-lim as code owners March 2, 2024 09:11

2yunseong commented Mar 2, 2024

View reviewed changes

bada308 approved these changes Mar 2, 2024

View reviewed changes

frontend/components/application/applicationNode/NextButton.component.tsx

}

return true;

return (

Copy link

Collaborator

bada308 Mar 2, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

깔끔해졌네요.. 👍

bada308 reviewed Mar 2, 2024

View reviewed changes

frontend/components/application/applicationNode/NextButton.component.tsx Outdated Show resolved Hide resolved

baegyeong approved these changes Mar 2, 2024

View reviewed changes

refactor: 상수화가 적용안된 코드 수정

ee834ea

Co-authored-by: Ocean <[email protected]>

bada308 mentioned this pull request Mar 2, 2024

fix: 지원경로 기타 클릭 후 값 입력 시 제출할 수 있도록 수정한다. #106

Merged

geongyu09 approved these changes Mar 2, 2024

View reviewed changes

loopy-lim approved these changes Mar 2, 2024

View reviewed changes

2yunseong mentioned this pull request Mar 2, 2024

[FE] refactor: ApplicationNextButton 컴포넌트 클린코드 및 구조 변경 #46

Open

2 tasks

2yunseong merged commit d9fb3d8 into main Mar 2, 2024

2yunseong deleted the refactor/46-next-button branch September 3, 2024 17:04

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor: next button 분기문 리팩토링 #104

refactor: next button 분기문 리팩토링 #104

2yunseong commented Mar 2, 2024

2yunseong left a comment •

edited

Loading

2yunseong Mar 2, 2024

bada308 Mar 2, 2024

baegyeong Mar 2, 2024

geongyu09 Mar 2, 2024

loopy-lim Mar 2, 2024

2yunseong Mar 2, 2024

2yunseong Mar 2, 2024

loopy-lim Mar 2, 2024

2yunseong Mar 2, 2024

baegyeong Mar 2, 2024

loopy-lim Mar 2, 2024

2yunseong Mar 2, 2024

loopy-lim Mar 2, 2024

2yunseong Mar 2, 2024 •

edited

Loading

loopy-lim Mar 2, 2024

bada308 left a comment

bada308 Mar 2, 2024

baegyeong left a comment

baegyeong Mar 2, 2024

baegyeong Mar 2, 2024

geongyu09 Mar 2, 2024

loopy-lim left a comment

loopy-lim Mar 2, 2024

loopy-lim Mar 2, 2024

loopy-lim Mar 2, 2024

		const EMPTY_STRING: string = "";
		const localStorageValueFromName = localStorage.get(name, EMPTY_STRING);

refactor: next button 분기문 리팩토링 #104

refactor: next button 분기문 리팩토링 #104

Conversation

2yunseong commented Mar 2, 2024

주요 변경사항

리뷰어에게...

관련 이슈

2yunseong 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

2yunseong Mar 2, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bada308 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

baegyeong 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

loopy-lim 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

2yunseong left a comment •

edited

Loading

2yunseong Mar 2, 2024 •

edited

Loading