Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FE/#215 메인 페이지, ProjectCard, Banner 컴포넌트 마이그레이션 #227

Merged
merged 13 commits into from
Sep 29, 2023

Conversation

Yujin-Baek
Copy link
Member

@Yujin-Baek Yujin-Baek commented Sep 28, 2023

🛠️ 변경사항

  1. 메인 페이지 마이그레이션
  • API 호출 방식 변경 : axios -> fetch
  • API 호출 쿼리 파라미터 부분 수정
  • useNavigate -> useRouter
  • react-query 사용을 위해 utils 폴더에 queryProvider.tsx 파일을 만들고 최상단 layout.tsx에 import 해서 사용함
    • layout.tsx에 직접 사용하지 못하는 이유 : recoil과 동일한 이유
  • map 함수에서 리턴받은 index 값을 ProjectCard의 props로 추가(첫 번째로 렌더링 되는 이미지는 loading=’lazy’ 속성 사용하지 않기 위함)
  1. ProjectCard 컴포넌트 마이그레이션
  • img 태그 일부 'next/image'의 Image 컴포넌트로 변경
  • useNavigate → useRouter
  1. Banner 컴포넌트 마이그레이션
  • Swiper 컴포넌트에서 사용하지 않는 리스너 함수 제거
  • Swiper 컴포넌트에 Autoplay 기능 추가


☝️ 유의사항

  • NavBar 컴포넌트 마이그레이션 후, import 해야함


👀 참고자료



❗체크리스트

  • 하나의 메소드는 최소의 기능만 하도록 설정했나요?
  • 수정 가능하도록 유연하게 작성했나요?
  • 필요 없는 import문이나 setter 등을 삭제했나요?
  • 기존의 코드에 영향이 없는 것을 확인하였나요?

kimhalin and others added 9 commits September 24, 2023 15:32
BE/#209 controller mock mvc 테스트를 위한 userdetails 설정
- Swiper 컴포넌트에서 사용하지 않는 리스너 함수 제거
- Swiper 컴포넌트에 Autoplay 기능 추가
- API 호출 방식 변경 : axios -> fetch
- useNavigate -> useRouter
- react-query 사용을 위한 queryProvider 컴포넌트 생성
- img 태그 일부 'next/image'의 Image 컴포넌트로 변경
- API 호출 쿼리 파라미터 부분 수정
Copy link
Collaborator

@Mayreeel Mayreeel left a comment

Choose a reason for hiding this comment

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

수고하셨습니다!! 리뷰 한번 확인 부탁드려용
근데 왜 백엔드에서 작업한 파일까지 딸려온 걸까요?..

"next": "13.5.1",
"postcss": "8.4.30",
"prettier": "^3.0.3",
"react": "18.2.0",
"react-dom": "18.2.0",
"react-router-dom": "^6.16.0",
Copy link
Collaborator

Choose a reason for hiding this comment

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

혹시 react-router-dom를 설치하신 이유가 있을까요??

Copy link
Member Author

Choose a reason for hiding this comment

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

마이그레이션 하기 전에 에러가 떠서 설치했는데 지워야겠네요..! 확인 감사합니다😊

<RecoilRootProvider>{children}</RecoilRootProvider>
<QueryProvider>
<RecoilRootProvider>{children}</RecoilRootProvider>
</QueryProvider>
Copy link
Collaborator

Choose a reason for hiding this comment

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

어차피 react-query든 Recoil이든 Component를 한번 거쳐서 세팅해야되면 파일 하나로 합치는게 좋아보이네요!!

Copy link
Member Author

Choose a reason for hiding this comment

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

헉...좋은 의견 감사합니다! 😄 그렇게 수정하도록 할게요

- recoilRootProvider, queryProvider 컴포넌트 -> Provider 컴포넌트 하나로 합치기
- react-router-dom 라이브러리 삭제
Copy link
Collaborator

@Mayreeel Mayreeel left a comment

Choose a reason for hiding this comment

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

수고하셨습니다~~!!

@github-actions
Copy link

github-actions bot commented Sep 29, 2023

Unit Test Results

86 tests  +5   86 ✔️ +5   5s ⏱️ -1s
17 suites ±0     0 💤 ±0 
17 files   ±0     0 ±0 

Results for commit 3135764. ± Comparison against base commit c69984c.

♻️ This comment has been updated with latest results.

@Yujin-Baek Yujin-Baek merged commit 1f747c6 into FE/#207 Sep 29, 2023
3 checks passed
@Yujin-Baek Yujin-Baek changed the title FE/#215 메인 페이지, ProjectCard 마이그레이션 FE/#215 메인 페이지, ProjectCard, Banner 컴포넌트 마이그레이션 Sep 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants