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

Feat/carousel: Carousel 컴포넌트 구현 #64

Merged
merged 39 commits into from
Mar 27, 2023
Merged

Conversation

leesunmin1231
Copy link
Contributor

@leesunmin1231 leesunmin1231 commented Mar 20, 2023

🤠 개요

추가된 사항

  • props가 바뀌었습니다.
  • slide 형태가 추가되었습니다.

최종 수정

  • 슬라이딩 방식 변경: useEffect로 스크롤 옮기는 방식 -> 스크롤 옮기는 함수 호출
  • scrollIntoView 메소드 behavior: 'smooth'옵션이 크롬 브라우저에서 제대로 동작 안하는 오류 -> setTimeout으로 감쌌습니다. 자세한 설명은 Feat: Carousel 컴포넌트 구현 #60 이슈에 가면 확인하실 수 있습니다.
  • 슬라이드 이동 버튼 빠르게 클릭시 progress 상태가 왔다갔다 하는 오류가 있어서, throttle을 걸어주었습니다.
  • useContext 커스텀 훅 분리

이외에 추가적으로 궁금하신 사항이나 수정할 사항있으면 댓글 남겨주세요~~

💫 설명

  • 디자인을 어떻게 해야할까 고민을 많이 했는데 모바일 퍼스트라는 점을 가장 크게 고려했어요.
  • 처음에 넣어줄 내용들을 배열 타입 props로 받았다가, 너무 유연함이 없는 컴포넌트인 거 같아서 Compound component 패턴으로 리팩토링했습니다.
  • Carousel 컴포넌트 안에 자식 컴포넌트로 여러개의 Card 컴포넌트가 있는 형태이며, Card 컴포넌트 내에 있는 자식 요소들은 하나의 카드 안에 들어가게 됩니다.
  • 자식 컴포넌트는 Card, Slide 컴포넌트가 있으며 Slide 컴포넌트를 사용하게 될 경우, width가 100%인 캐러셀을 사용할 수 있습니다. (자세한 건 스토리북을 참고해주세요)
  • 가로 스크롤과 버튼으로 slide를 이동할 수 있습니다.

components

Carousel: 감싸는 부모 컴포넌트
Card: 카드 형태 자식 컴포넌트
Slide: slide 형태 자식 컴포넌트: 적용시 width는 100vw로 들어가는 layout입니다. Carousel props로 width나 line을 넘겨줘도 무시됩니다. height는 정상적으로 반영됩니다.

props

line: number => 캐러셀 카드형태일 때, 가로줄 개수
width: number => 캐러셀에 들어갈 카드 하나의 너비
height: number => 캐러셀에 들어갈 카드 하나의 높이

line 제외하고 optional 이고, line의 경우 default를 1로 설정해두었습니다.
width, height는 카드 하나의 규격을 커스텀 할 수 있도록 넣어주었습니다.

📷 스크린샷 (Optional)

Default inline layout

image

Custom inline layout

image

Default Two Line layout

image

Custom Two Line layout

image

Multi Line layout

image

Slide layout

image

@leesunmin1231 leesunmin1231 added the ✨ feat 기능 구현 label Mar 20, 2023
@leesunmin1231 leesunmin1231 self-assigned this Mar 20, 2023
@prayinforrain
Copy link
Contributor

멋있어요~! 너무 이뻐요
리뷰에 자잘한 생각을 적어뒀고 하나만 더 추가할게요. 지금 구현된 캐러셀에서 선택된 페이지가 오는 위치를 지정할 수 있을까요?
그러니까 음.. 애플의 캐러셀을 예로 들면
image
선택된 카드가 이 체크한 위치에 오도록 되어 있고, usecase에 따라 선택된 카드가 캐러셀의 중앙에 오도록 하고싶을 수도 있을 것 같아요. 이 부분을 어떻게 제어할 수 있는지 궁금해요.

src/components/Carousel/index.tsx Outdated Show resolved Hide resolved
src/components/Carousel/index.tsx Outdated Show resolved Hide resolved
src/components/Carousel/index.tsx Outdated Show resolved Hide resolved
src/components/Carousel/index.tsx Outdated Show resolved Hide resolved
Copy link
Member

@iyu88 iyu88 left a comment

Choose a reason for hiding this comment

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

수고 많으셨어요! 👍👍👍
변경 사항이 많을 것 같은데 천천히 해주세요~ 😊

src/components/Carousel/index.tsx Show resolved Hide resolved
src/components/Carousel/index.tsx Outdated Show resolved Hide resolved
src/components/Carousel/index.tsx Outdated Show resolved Hide resolved
src/components/Carousel/index.tsx Outdated Show resolved Hide resolved
src/components/Carousel/index.tsx Show resolved Hide resolved
src/components/Carousel/index.tsx Outdated Show resolved Hide resolved
src/components/Carousel/Carousel.stories.tsx Outdated Show resolved Hide resolved
Copy link
Contributor

@se030 se030 left a comment

Choose a reason for hiding this comment

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

고생하셨습니다 !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
리뷰할게 자잘한 것밖에 없어요 !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!

src/components/Carousel/index.tsx Show resolved Hide resolved
src/components/Carousel/index.tsx Outdated Show resolved Hide resolved
src/components/Carousel/index.tsx Outdated Show resolved Hide resolved
src/components/Carousel/index.tsx Show resolved Hide resolved
src/components/Carousel/index.tsx Outdated Show resolved Hide resolved
Copy link
Contributor

@prayinforrain prayinforrain left a comment

Choose a reason for hiding this comment

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

고생하셨어요!

작은 버그가 있는데..ㅎㅎ..
NavigationButton으로 페이지 이동 -> scrollEventHandler 디바운스 타이머 시작 -> 타이머가 끝나기 전에 한번 더 눌러 페이지 이동
이런 상황에서 자꾸 이전 카드로 돌아오는 것 같아요. 간단히 말하면 네비게이션 버튼을 막 광클하면 덜컹덜컹해요.

이것만 해결되면 다른 문제는 없는 것 같아요! 너무 좋아요

Copy link
Member

@iyu88 iyu88 left a comment

Choose a reason for hiding this comment

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

수고많으셨어요~~
무수히 많은 Conversation을 처리하느라 고생하셨습니다.
👍👍👍

src/components/Carousel/NavigationButton.tsx Outdated Show resolved Hide resolved
src/components/Carousel/index.tsx Outdated Show resolved Hide resolved
src/components/Carousel/index.tsx Outdated Show resolved Hide resolved
src/components/Carousel/index.tsx Outdated Show resolved Hide resolved
Copy link
Contributor

@prayinforrain prayinforrain left a comment

Choose a reason for hiding this comment

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

너무 고생하셨어요 ㅎㅎ..... 면목없네요
두 가지만 반영해 주시구.. 혹시 주말에 제가 늦게 확인하면 곤란하니 approve는 미리 꽂아 둘게요..

src/components/Carousel/index.tsx Show resolved Hide resolved
src/components/Carousel/index.tsx Outdated Show resolved Hide resolved
src/components/Carousel/index.tsx Show resolved Hide resolved
src/components/Carousel/index.tsx Outdated Show resolved Hide resolved
Copy link
Member

@iyu88 iyu88 left a comment

Choose a reason for hiding this comment

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

수정사항만 반영하고 합치면 될 것 같아요~
고생 많으셨습니다!

src/components/Carousel/NavigationButton.tsx Show resolved Hide resolved
src/components/Carousel/index.tsx Show resolved Hide resolved
src/components/Carousel/index.tsx Outdated Show resolved Hide resolved
@leesunmin1231 leesunmin1231 linked an issue Mar 27, 2023 that may be closed by this pull request
1 task
@leesunmin1231 leesunmin1231 merged commit 9c33405 into main Mar 27, 2023
@leesunmin1231 leesunmin1231 deleted the feat/carousel branch March 27, 2023 13:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
✨ feat 기능 구현
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feat: Carousel 컴포넌트 구현
4 participants