7, 8주차 코드리뷰 #73

kang-kibong · 2024-10-25T13:00:19Z

안녕하세요 멘토님,

7, 8주차 궁금한점을 README.md에 작성하였습니다.

Storybook 배포링크는 다음과 같습니다!

https://66e528a32564a3669b75354b-brbizuczms.chromatic.com/

이번주차도 잘 부탁드립니다, 감사합니다!

* Update README.md * Refactor/#47 6주차 코드리뷰 리팩토링 (#48) * refactor: separate style and prop-related constants * refactor: remove lambda function * refactor: restructure SignUp and RecruitmentHeader components * feat: separate Button and Text component of SignIn * feat: add SignUpText component * refactor: remove auth page's barrel file * Feat/#42, #43 지원자 목록 페이지 및 팝업 구현 (#51) * feat: 지원자 목록 페이지 구현 * feat: 지원자 목록 페이지 스토리북 생성 * refactor: ApplicantList 테이블 컴포넌트 분리 * feat: Applicants path 설정 * feat: 지원자 목록 페이지에 계약 관련 팝업 추가 * fix: 이미지 경로 수정 및 불필요한 태그 제거 * refactor: MyAccount 페이지 구조 변경 및 CompanyRecruitments로 파일명 변경 - 기존 MyAccount 페이지를 CompanyRecruitments로 이름 변경 - 새로운 MyAccount 페이지 구현을 위해 기존 페이지의 역할 변경 * refactor: visaRegistration 관련 파일 구조 변경 - visaRegistration 페이지를 src/pages/employee에서 src/pages로 이동 - 관련 기능을 src/features/employee/visaRegistration에서 src/features/visaRegistration으로 이동 * feat: Table 컴포넌트 구현 * feat: 변경된 고용주 마이페이지 구현 * refactor: 회사 관련 공통 기능을 features/companies로 이동 및 CompanyInfo 수정 * refactor: CompanyRecruitments 페이지 이름을 MyCompany로 변경 * feat: EmployerMyAccount path 설정 * refactor: 불필요한 코드 삭제 및 폴더명 일관성 있게 변경 * Feat/#50 Select 컴포넌트 구현 (#53) * feat: add Select component * feat: add useGlobalSelect and useSelect * refactor: move directory * refactor: EmployerMyAccount 페이지에서 mock 데이터 분리 및 코드 정리 * refactor: visaRegistration 및 applicants 페이지의 mock, style 파일 분리 * refactor: RecruitmentList 컴포넌트 리팩토링 및 RecruitmentsTable 분리 * refactor: CompanyInfo 반응형 디자인 수정 * refactor: 외국인 번호 및 비자 발급 일자 등록 페이지 스타일 수정 * refactor: Header 컴포넌트의 닉네임 버튼을 사용자 프로필 이미지로 변경 * Refactor/#54 Modal 컴포넌트 재설계 (#55) * chore: add loadable component package * feat: implement modal management system with context and dynamic loading * refactor: 코드 리뷰 반영 - formValid를 useMemo로 관리 - validateForeignerNumber 함수를 별도 파일로 분리 * Feat/#56 메인 페이지 API 연동 (#57) * chore: setting mockServiceWorker * feat: add useFetchRecruitments hooks and recruitmentsMockHandler * feat: add useFetchSlides hooks and slidesMockHandler * feat: add Spinner component * feat: add AsyncBoundary component * chore: add msw-storybook-addon * Feat/#58 OAuth 구글 로그인 구현 (#59) * feat: add useGoogleOAuth hook * feat: add Loading page * chore: add MemoryRouter to decorators * Feat/#60 가입자 정보 선택 API 연동 (#61) * feat: add useRegister hook * fix: change role prop value * style: Button 컴포넌트 Props 이름변경 theme->design * feat: 근로자마이페이지 아이콘 설정 * feat: 근로자 마이페이지 구현 * feat: 근로자마이페이지 라우터 설정 * style: Button props 이름 변경 * feat: msw 세팅 및 API path 작성 * feat: 구인글 등록 API 연결 및 msw 세팅 * fix: 구인글 업로드 mock 핸들러 수정 * feat: 근로자 마이페이지 mock 핸들러 추가 * feat: 근로자 마이페이지 API 연결 및 msw 설정 * feat: 이력서 페이지 구현 (#63) - react-hook-form 을 사용했습니다. - api 명세서에 맞게 이름,주소,번호,경력,자기소개,한국어실력을 필수값으로 받게 했습니다. Co-authored-by: kangkibong <[email protected]> * fix: change button prop * feat: add GitHub Actions workflow for linting and type checking --------- Co-authored-by: yimsebin <[email protected]> Co-authored-by: Kim Jian <[email protected]> Co-authored-by: KimJi-An <[email protected]> Co-authored-by: LEE YONGJIN <[email protected]>

7, 8주차 최종 산출물

netlify · 2024-10-25T13:00:37Z

✅ Deploy Preview for hire-higher ready!

Name	Link
🔨 Latest commit	`caf2e34`
🔍 Latest deploy log	https://app.netlify.com/sites/hire-higher/deploys/671ba5e93905640008d7b64e
😎 Deploy Preview	https://deploy-preview-73--hire-higher.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

JunilHwang · 2024-10-28T01:58:48Z

.github/workflows/lint.yaml

@@ -0,0 +1,43 @@
+name: Lint and Type Check


보통 이런걸 ci 라고 한답니다!

JunilHwang · 2024-10-28T01:59:33Z

.github/workflows/lint.yaml

+  push:
+    # 모든 브랜치에 대해 푸시 시 실행
+    branches:
+      - '**'


push에 대해서 실행하면... 비용이 너무 많이 들 것 같아요 ㅎㅎ
pr의 sync에 대해서 실행해주면 어떨까요?

on: pull_request_target: types: - synchronize - opened

JunilHwang · 2024-10-28T05:04:02Z

.github/workflows/lint.yaml

+      # Run ESLint to check for linting errors
+      - name: Run ESLint
+        run: npm run lint
+
+      # Run Prettier to check formatting
+      - name: Run Prettier Check
+        run: npm run format -- --check
+
+      # Run TypeScript compiler to check for type errors
+      - name: Run TypeScript Check
+        run: npm run tsc


이렇게 세 개는 husky 같은걸 통해서 git에 push하기 전에 로컬에서 스테이지된 파일에 대해 검사하는 방법도 있답니다!

git hook 이라는 키워드로 한 번 찾아보세요~

JunilHwang · 2024-10-28T05:05:51Z

package.json

@@ -27,10 +28,14 @@
    "@emotion/css": "^11.13.0",
    "@emotion/react": "^11.13.3",
    "@emotion/styled": "^11.13.0",
+    "@loadable/component": "^5.16.4",


그냥 react의 Lazy 기능을 사용하면 되지 않을까요~?

JunilHwang · 2024-10-28T05:08:35Z

package.json

@@ -18,6 +18,7 @@
  },
  "lint-staged": {
    "**/*.{tsx,ts,jsx,js}": [
+      "bash -c tsc -p tsconfig.json --noEmit",


이미 lint-staged 에서 검사하고 있었군요 ㅎㅎ 좋습니다!

JunilHwang · 2024-10-28T05:36:42Z

src/components/common/Select/index.context.tsx

+export const SelectProvider = ({ children }: PropsWithChildren) => {
+  const [isOpen, toggle] = useToggle();
+  const [selectedValue, setSelectedValue] = useState<string | undefined>(undefined);
+
+  const handleItemSelect = (value: string) => {
+    setSelectedValue(value);
+    toggle();
+  };
+
+  return (
+    <SelectContext.Provider value={{ isOpen, toggle, selectedValue, onItemSelect: handleItemSelect }}>
+      {children}
+    </SelectContext.Provider>
+  );
+};


이제보니 컴파운드 패턴을 사용하기 위함이었군요..!
다만 모든 컴포넌트를 다 index로 표현하게 되면 컴포넌트를 탐색할 때 무척 어렵답니다 ㅠㅠ
파일 탐색으로는 컴포넌트 탐색이 불가능하고, 코드 탐색을 통해 선언하는 곳과 쓰이는 곳을 모두 찾아 돌아다니게 되어요..

JunilHwang · 2024-10-28T05:38:14Z

src/components/common/Modal/Modals.tsx

+      handleClose();
+    };
+
+    return <Component {...restProps} key={index} onClose={handleClose} onSubmit={handleSubmit} />;


지금 닫기를 Modal이 아니라 Modals 에서 주입해주고 있어요.
Modal이 닫기 기능을 가지고 있고, Modals에게 닫힐 때 알려주는 방식으로 만들어지면 좋겠네요!

JunilHwang · 2024-10-28T05:39:01Z

src/features/applicants/ApplicantList/ApplicantsTable/index.tsx

+          />
+        </tbody>
+      </Table>
+      <ContractModal isOpen={isModalOpen} onClose={handleCloseModal} />


모달 컴포넌트를 미리 렌더링 해놓는게 좋은 방법일까요?

JunilHwang · 2024-10-28T05:39:50Z

src/features/applicants/ApplicantList/ContractModal/index.tsx

+      {isOpen && (
+        <Modal
+          textChildren={
+            <ModalText foreignerIdNumber={foreigner.foreignerIdNumber} visaGenerateDate={foreigner.visaGenerateDate} />
+          }
+          buttonChildren={
+            <Flex justifyContent="space-between">
+              <Button onClick={onClose}>취소</Button>
+              <Button onClick={onClose} css={customButtonStyle}>
+                <Flex gap={{ x: '15px' }}>
+                  <Typo size="16px" style={buttonTextStyle}>
+                    확인하였습니다.
+                  </Typo>
+                  <Icon.Arrow.RightWhite />
+                </Flex>
+              </Button>
+            </Flex>
+          }
+          /* onClose 부분 추후 수정 예정 */
+          onClose={onClose}
+          style={modalStyle}
+        />
+      )}


어차피 isOpen일 때 Modal이 렌더링 되고, 랩핑만 해주고 있군요 ㅎㅎ 이것도 좋은 방법 같아요.

JunilHwang · 2024-10-28T05:41:41Z

src/features/auth/SignUp/components/common/RoleSelector/index.tsx

+import { Card, Flex, Typo } from '@components/common';
+import { roleConfig } from './index.config';
+import { bounceAnimation } from '@assets/styles/animations';
+import { responsiveStyle } from '@utils/responsive';
+import useModals from '@components/common/Modal/hooks/useModals';
+import { modals } from '@/components/common/Modal/Modals';
+import { useRegister } from '@/apis/auth/mutations/useRegister';
+import { useNavigate } from 'react-router-dom';
+import ROUTE_PATH from '@/routes/path';


각 폴더의 의존 관계를 한 번 그려보세요.
그 다음에 "이렇게 서로 의존하고 있는 모습이 자연스러운걸까?" 라는 판단을 해보시면 좋겠습니다.

앞선 리뷰에서 제가 common이 feature를 의존하는게 맞을까요? 라고 이야기 했는데
그런 것 처럼 어떤 영역이 어떤 영역을 가져다 사용해야 하는지, 반대로, 어떤 영역은 어떤 영역을 가져다 사용하면 안 되는지 고민이 필요해보입니다!

kang-kibong and others added 3 commits October 24, 2024 21:55

Merge pull request #72 from kakao-tech-campus-2nd-step3/Develop

8c32073

7, 8주차 최종 산출물

docs: update README.md

26668f2

kang-kibong self-assigned this Oct 25, 2024

kang-kibong added 2 commits October 25, 2024 23:05

docs: update README.md

7d9c6dd

docs: update README.md

caf2e34

kang-kibong changed the title ~~7, 8주차 코드리뷰 PR7, 8주차 코드리뷰 PR~~ 7, 8주차 코드리뷰 Oct 25, 2024

JunilHwang self-requested a review October 28, 2024 05:42

JunilHwang reviewed Oct 28, 2024

View reviewed changes

JunilHwang merged commit 56a0161 into Review Oct 28, 2024
8 of 9 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

7, 8주차 코드리뷰 #73

7, 8주차 코드리뷰 #73

kang-kibong commented Oct 25, 2024 •

edited

Loading

netlify bot commented Oct 25, 2024 •

edited

Loading

JunilHwang Oct 28, 2024

JunilHwang Oct 28, 2024

JunilHwang Oct 28, 2024

JunilHwang Oct 28, 2024

JunilHwang Oct 28, 2024

JunilHwang Oct 28, 2024

JunilHwang Oct 28, 2024

JunilHwang Oct 28, 2024

JunilHwang Oct 28, 2024

JunilHwang Oct 28, 2024

7, 8주차 코드리뷰 #73

7, 8주차 코드리뷰 #73

Conversation

kang-kibong commented Oct 25, 2024 • edited Loading

netlify bot commented Oct 25, 2024 • edited Loading

✅ Deploy Preview for hire-higher ready!

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

kang-kibong commented Oct 25, 2024 •

edited

Loading

netlify bot commented Oct 25, 2024 •

edited

Loading