[feat] 로그인 온보딩 디자인 구현 #39
Conversation
JaeIn1
requested review from
semnil5202,
jangwonyoon,
youngju6143 and
evenway2025
as code owners
📝 WalkthroughWalkthrough이번 PR은 swiper 라이브러리를 의존성에 추가하고, 로그인 페이지와 온보딩 기능에 대한 전반적인 업데이트를 포함합니다. LoginPage 컴포넌트가 상태 관리(useState)를 도입하여 온보딩과 로그인 화면 간 전환 기능을 구현하며, 이에 따라 관련 UI 컴포넌트(LoginFeature, KakaoLoginButton 등)도 업데이트되었습니다. 또한 Swiper를 활용한 온보딩 슬라이더(OnboardingCarousel) 및 온보딩 기능(OnboardingFeature, OnboardingHeader, OnboardingSlide)과 슬라이드 구성을 위한 설정 파일이 추가되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant U as 사용자
participant LP as LoginPage
participant OF as OnboardingFeature
participant LF as LoginFeature
U->>LP: 페이지 접근
Note over LP: 초기 상태 showOnboarding: true
LP->>OF: 온보딩 컴포넌트 렌더링
U->>OF: 온보딩 완료 클릭
OF->>LP: onComplete callback 호출
LP->>LF: 상태 업데이트 (showOnboarding: false)
LP->>U: LoginFeature 렌더링
sequenceDiagram
participant U as 사용자
participant OC as OnboardingCarousel
participant OS as OnboardingSlide
U->>OC: 슬라이드 전환 요청
OC->>OS: 해당 슬라이드 렌더링
U->>OC: 마지막 슬라이드에서 '다음' 버튼 클릭
OC->>OC: onComplete 콜백 호출
Possibly related PRs
Suggested reviewers
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (17)
src/features/auth/login/ui/LoginFeature.tsx (2)
6-6: 이미지 경로를 상수로 분리하는 것이 좋습니다.이미지 URL을 상수 파일이나 설정 파일로 분리하는 것이 코드 유지보수에 더 좋습니다.
17-18: 중복된 KakaoLoginButton 사용현재 동일한 KakaoLoginButton을 두 번 사용하고 있습니다. 주석에는 이를 나중에 Google과 Apple 로그인 버튼으로 변경할 예정이라고 되어 있습니다.
좀 더 명확하게 표현하기 위해 다음과 같이 수정하는 것이 좋습니다:
<div className="absolute bottom-0 w-full space-y-4 px-6 pb-10"> <KakaoLoginButton /> - <KakaoLoginButton /> {/* 구글 , 애플 로그인 버튼으로 바꿀 예정 */} + {/* TODO: 아래 버튼을 구글, 애플 로그인 버튼으로 구현 예정 */} + <KakaoLoginButton buttonText="다른 계정으로 로그인" /> </div>src/features/onboarding/ui/OnboardingHeader.tsx (2)
9-10: 빈 div 요소 사용 개선 필요레이아웃 목적으로 빈
<div></div>를 사용하고 있습니다. 이는 시맨틱하지 않은 방식입니다. 대신에justify-end를 사용하거나, 필요한 경우 더 명시적인 방식으로 레이아웃을 조정하는 것이 좋습니다.- <header className="flex items-center justify-between p-4"> - <div></div> + <header className="flex items-center justify-end p-4">
11-13: 접근성 개선 필요건너뛰기 버튼에
aria-label이 없습니다. 스크린 리더 사용자를 위해 접근성을 개선해주세요. 또한 국제화(i18n)를 위해 "건너뛰기" 텍스트를 상수로 분리하는 것이 좋습니다.- <button onClick={onSkip} className="font-medium text-gray-500"> + <button onClick={onSkip} className="font-medium text-gray-500" aria-label="건너뛰기"> <Typography variant="body-2">건너뛰기</Typography> </button>src/features/onboarding/ui/OnboardingFeature.tsx (1)
1-18: 컴포넌트 구조가 깔끔합니다.OnboardingFeature 컴포넌트의 구조가 깔끔하며, 콜백 함수를 적절히 전달하고 있습니다. 하지만 OnboardingCarousel을 import 할 때 default import를 사용하고 OnboardingHeader는 named import를 사용하는 등 import 스타일이 일관되지 않습니다.
- import OnboardingCarousel from './OnboardingCarousel'; - import { OnboardingHeader } from './OnboardingHeader'; + import { OnboardingCarousel } from './OnboardingCarousel'; + import { OnboardingHeader } from './OnboardingHeader';또는
- import OnboardingCarousel from './OnboardingCarousel'; - import { OnboardingHeader } from './OnboardingHeader'; + import OnboardingCarousel from './OnboardingCarousel'; + import OnboardingHeader from './OnboardingHeader';src/features/onboarding/ui/OnboardingSlide.tsx (2)
16-16: alt 텍스트 보강을 고려해보세요.
현재 alt 값이"온보딩 이미지"로 하드코딩되어 있어, 접근성 면에서 충분하지 않을 수 있습니다.- <Image src={imageUrl} alt="온보딩 이미지" fill style={{ objectFit: 'contain' }} priority /> + <Image src={imageUrl} alt={typeof description === 'string' ? description : '온보딩 이미지'} fill style={{ objectFit: 'contain' }} priority />
25-33: 배열과 문자열 처리 방식이 간결합니다.
현재 배열일 경우 map을 통해 제목 형태로, 문자열일 경우 본문 형태로 깔끔하게 처리하고 있어 유지보수에 용이합니다. 다만, 온보딩 설명이 길어질 경우 가독성 유지에 대한 고민도 필요합니다.src/features/auth/ui/KakaoLoginButton.tsx (2)
3-3: 임시 주석에 대한 추가 설명이 필요할 수 있습니다.
추후 디자인 완성 본이 나오면 어떤 부분을 수정해야 하는지 구체적으로 기술해두면 협업에 도움이 됩니다.
20-26: 버튼 구조와 스타일 수정이 적절합니다.
카카오 로고와 텍스트를 분리해 좀 더 시각적으로 명확해졌습니다. 다만, 마지막에 들어간<div></div>는 공백 확보 의도 외에는 의미가 없으므로, 도우미 주석이나 적절한 className을 설정해 목적을 분명히 하면 좋겠습니다.src/app/login/page.tsx (2)
20-29: 백그라운드 이미지 설정이 유연하며 유지보수에 유리합니다.
style속성으로 이미지 경로와 반복 여부 등을 제어하는 방식은 단순합니다. 다만, 추후 theme나 설정 파일로 분리해 사용하면 재사용 및 일관성 유지에 도움이 될 수 있습니다.
30-34: 온보딩 완료 후 데이터 저장 로직 검토가 필요합니다.
현재는 상태만으로 온보딩을 제어하는데, 사용자가 새로고침 시 다시 온보딩이 노출될 우려가 있습니다. Local Storage나 서버 저장을 통해 실제 온보딩 완료 정보를 반영할 수 있게 고려해보세요.src/features/onboarding/ui/OnboardingCarousel.tsx (6)
58-65: 접근성(a11y) 개선이 필요합니다.pagination 버튼에 접근성 속성이 부족합니다. 스크린 리더 사용자를 위해 aria-label 속성을 추가하는 것이 좋습니다.
다음과 같이 수정해보세요:
<button key={index} className={`h-2 rounded-full transition-all ${ index === activeIndex ? 'w-2 bg-white' : 'w-2 bg-white opacity-50' }`} onClick={() => swiperRef.current?.slideToLoop(index)} + aria-label={`슬라이드 ${index + 1}로 이동`} />
39-102: 중복 export 문제가 있습니다.컴포넌트가 named export와 default export 두 가지 방식으로 모두 내보내지고 있습니다. 일반적으로 한 가지 방식만 선택하는 것이 좋습니다.
프로젝트의 일관성에 따라 한 가지 방식만 선택하세요. 일반적으로 페이지 컴포넌트는 default export를 사용하고, 재사용 가능한 컴포넌트는 named export를 사용하는 경우가 많습니다.
- export function OnboardingCarousel({ onComplete }: OnboardingCarouselProps): React.ReactElement { + function OnboardingCarousel({ onComplete }: OnboardingCarouselProps): React.ReactElement { // component code } export default OnboardingCarousel;또는
export function OnboardingCarousel({ onComplete }: OnboardingCarouselProps): React.ReactElement { // component code } - export default OnboardingCarousel;
81-81: 자동 재생 설정에 접근성 고려가 필요합니다.자동 재생 기능은 접근성 문제를 일으킬 수 있습니다. 사용자가 콘텐츠를 읽기 전에 다음 슬라이드로 넘어갈 수 있기 때문입니다. 또한 일부 사용자에게는 움직이는 콘텐츠가 집중을 방해할 수 있습니다.
사용자가 자동 재생을 중지할 수 있는 옵션을 제공하거나, 사용자 상호작용이 있을 때 자동 재생을 일시 중지하는 것을 고려해보세요.
- autoplay={{ delay: 5000, disableOnInteraction: false }} + autoplay={{ delay: 5000, disableOnInteraction: true }}또는 자동 재생을 켜고 끌 수 있는 제어 버튼을 추가하는 것도 좋은 방법입니다.
17-33: 하드코딩된 데이터에 대한 관리 필요슬라이드 데이터가 컴포넌트 내부에 하드코딩되어 있습니다. 이 데이터가 변경될 가능성이 있거나 다른 곳에서도 사용될 수 있다면, 별도의 상수 파일이나 설정 파일로 분리하는 것이 좋습니다.
슬라이드 데이터를 별도의 파일(예:
onboardingSlideData.ts)로 분리하고 import해서 사용하는 것을 고려해보세요.
19-21: 이미지 최적화 고려 필요이미지 경로가 직접 참조되고 있습니다. Next.js를 사용 중이라면, 내장된 Image 컴포넌트를 사용하여 이미지 최적화와 지연 로딩을 구현하는 것이 좋습니다.
이미지 표시에 Next.js의 Image 컴포넌트를 사용하여 성능을 개선하는 것을 고려해보세요.
import Image from 'next/image'; // OnboardingSlide 컴포넌트 내부에서 이미지 사용 시 <Image src={imageUrl} alt="온보딩 이미지" width={500} height={300} priority={activeIndex === slide.id - 1} quality={90} />
95-95: 하드코딩된 문자열에 대한 국제화(i18n) 고려"다음"과 같은 문자열이 하드코딩되어 있습니다. 향후 다국어 지원이 필요할 경우를 대비하여 국제화(i18n) 라이브러리를 사용하거나 상수로 분리하는 것이 좋습니다.
프로젝트에서 사용하는 국제화 방식에 따라 문자열을 관리하세요. 예를 들어 react-i18next를 사용하는 경우:
import { useTranslation } from 'react-i18next'; // 컴포넌트 내부 const { t } = useTranslation(); // 버튼 텍스트 사용 시 <Typography variant="body-2">{t('onboarding.next')}</Typography>
📜 Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (6)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yamlpublic/images/login/onboarding/onboarding-1.pngis excluded by!**/*.pngpublic/images/login/onboarding/onboarding-2.pngis excluded by!**/*.pngpublic/images/login/onboarding/onboarding-3.pngis excluded by!**/*.pngpublic/images/login/onboarding/onboarding-bg.pngis excluded by!**/*.pngpublic/images/login/onboarding/onboarding-login.pngis excluded by!**/*.png
📒 Files selected for processing (8)
package.json(1 hunks)src/app/login/page.tsx(1 hunks)src/features/auth/login/ui/LoginFeature.tsx(1 hunks)src/features/auth/ui/KakaoLoginButton.tsx(2 hunks)src/features/onboarding/ui/OnboardingCarousel.tsx(1 hunks)src/features/onboarding/ui/OnboardingFeature.tsx(1 hunks)src/features/onboarding/ui/OnboardingHeader.tsx(1 hunks)src/features/onboarding/ui/OnboardingSlide.tsx(1 hunks)
🔇 Additional comments (4)
package.json (1)
24-24: Swiper 라이브러리가 추가되었습니다.캐러셀 기능을 위한 Swiper 라이브러리가 추가되었습니다. 현재 사용 중인 버전은 최신 버전에 가깝습니다.
src/features/auth/login/ui/LoginFeature.tsx (1)
10-14: 임시 스타일링 사용에 주의해주세요.주석에 언급된 대로
top-[45%]와pl-[1rem]과 같은 임시 스타일링을 사용하고 있습니다. 이는 나중에 레이아웃 문제를 일으킬 수 있으므로, 완성된 디자인에서는 더 견고한 솔루션으로 변경해주세요.- {/* 이미지 배치가 안되서 임시로 top-[45%]사용했습니다. */} - <div className="absolute left-1/2 top-[45%] h-[23%] w-full -translate-x-1/2 -translate-y-1/2 transform"> - {/* 이미지 가운데 배치가 안되서 임시로 pl-[1rem]사용했습니다. */} - <Image src={imgUrl} alt="온보딩 이미지" fill className="object-contain pl-[1rem]" priority /> + <div className="absolute left-1/2 top-1/2 h-[23%] w-full -translate-x-1/2 -translate-y-1/2 transform"> + <Image src={imgUrl} alt="온보딩 이미지" fill className="object-contain" priority />src/features/onboarding/ui/OnboardingSlide.tsx (1)
5-9: 인터페이스 구조가 명확하고 사용하기 편리합니다.
description과imageUrl등의 prop명에 일관성이 있어, 컴포넌트를 사용하는 측에서도 쉽게 이해할 수 있습니다.src/app/login/page.tsx (1)
9-17: 온보딩 진행 여부 관리가 간결합니다.
showOnboarding상태를 통해 온보딩을 보이거나 숨기는 로직이 명확하여, 유저 경험 관리가 수월합니다.
| <div className="flex h-full w-full flex-col items-center"> | ||
| <div className="w-full flex-1"> | ||
| <Swiper | ||
| modules={[Pagination, Navigation, Autoplay]} |
There was a problem hiding this comment.
modules 배열에 잘못된 컴포넌트가 포함되어 있습니다.
modules 배열에 로컬에서 정의한 Pagination 컴포넌트가 포함되어 있습니다. 이는 swiper의 Pagination 모듈과 다른 것입니다. swiper에서 가져온 모듈만 사용해야 합니다.
다음과 같이 수정해주세요:
- modules={[Pagination, Navigation, Autoplay]}
+ modules={[Navigation, Autoplay]}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| modules={[Pagination, Navigation, Autoplay]} | |
| modules={[Navigation, Autoplay]} |
There was a problem hiding this comment.
There was a problem hiding this comment.
🧩 Analysis chain
@semnil5202 감사합니다! 제가 제안한 이전 리뷰가 잘못되었네요. 스와이퍼의 modules 속성은 로컬 컴포넌트를 참조하는 것이 아니라 import 해온 모듈을 참조하는 것이 맞습니다.
현재 코드를 보니 import { Navigation, Autoplay } from 'swiper/modules';는 있지만, Swiper의 Pagination 모듈은 import하지 않았습니다. 그러나 modules 배열에 로컬 Pagination 컴포넌트를 포함시키는 것은 Swiper에서 지원하는 방식이 아닙니다.
제 리뷰 제안을 취소합니다. 올바른 수정은 아래와 같을 것입니다:
// Pagination 모듈 추가로 import
import { Navigation, Autoplay, Pagination } from 'swiper/modules';
// 그리고 modules 배열에 Pagination 추가
modules={[Pagination, Navigation, Autoplay]}또는 현재 커스텀 Pagination을 사용하고 있다면, modules 배열에서 제거하는 것이 맞습니다:
modules={[Navigation, Autoplay]}제공해주신 레퍼런스 링크를 확인하겠습니다. 혼란을 드려 죄송합니다.
🏁 Script executed:
#!/bin/bash
# Explore the swiper imports in the codebase to understand the correct usage
head -n 10 src/features/onboarding/ui/OnboardingCarousel.tsx
# Also search for how Pagination is imported and used in the file
grep -A 5 "import.*Pagination" src/features/onboarding/ui/OnboardingCarousel.tsx || echo "No direct Pagination import found"
# Search for any other file that might use Swiper Pagination correctly
rg "import.*Pagination.*from 'swiper" || echo "No Swiper Pagination imports found in the codebase"
# Check available modules in package
npm list swiper || echo "Swiper package details not available"
Length of output: 827
🌐 Web query:
How to use Pagination module in Swiper React correctly
💡 Result:
To implement pagination in Swiper React correctly, follow these key steps:
Core Implementation
- Install Required Packages:
npm install swiper- Basic Component Setup:
import { Pagination } from 'swiper/modules';
import { Swiper, SwiperSlide } from 'swiper/react';
import 'swiper/css';
import 'swiper/css/pagination';
export default () => {
return (
<Swiper
modules={[Pagination]}
pagination={{ clickable: true }}
>
<SwiperSlide>Slide 1</SwiperSlide>
<SwiperSlide>Slide 2</SwiperSlide>
<SwiperSlide>Slide 3</SwiperSlide>
</Swiper>
);
};Key Configuration Options
| Parameter | Type | Description | Default |
|---|---|---|---|
clickable |
boolean | Enables bullet click navigation | false |
dynamicBullets |
boolean | Animates bullet size for active slide | false |
type |
string | Pagination style (bullets, fraction, progressbar) |
bullets |
Advanced Usage Example:
<Swiper
modules={[Pagination]}
pagination={{
type: 'progressbar',
clickable: true,
dynamicBullets: true,
renderBullet: (index, className) => {
return `<span class="${className}">${index + 1}</span>`;
}
}}
>Common Issues & Solutions
-
Missing Pagination Elements:
- Ensure CSS imports are included:
import 'swiper/css/pagination';
- Verify module registration in
modulesarray
-
Custom Styling:
.swiper-pagination-bullet {
background: #000;
opacity: 0.5;
width: 12px;
height: 12px;
}
.swiper-pagination-bullet-active {
background: #ff0000;
opacity: 1;
}- Position Adjustment:
<Swiper
pagination={{
el: '.custom-pagination',
...
}}
className="my-swiper"
>
<div className="custom-pagination"></div>Pro Tip: For TypeScript users, utilize the SwiperOptions type from swiper/types for proper type checking[4].
The pagination module works with Swiper's responsive breakpoints system and supports touch/swipe navigation out of the box. For complex implementations, combine with other modules like Navigation and Scrollbar using the same pattern[7][9].
Citations:
- 1: https://refine.dev/blog/swiper-js/
- 2: https://www.youtube.com/watch?v=yCzBObw3ia8
- 3: https://www.freecodecamp.org/news/how-to-set-up-swiper-element-in-a-react-application/
- 4: https://swiperjs.com/types/interfaces/types_swiper_options.SwiperOptions
- 5: https://v8.swiperjs.com/react
- 6: [swiper/react] Custom navigation/pagination components using React refs not working/possible? nolimits4web/swiper#3855
- 7: https://swiperjs.com/react
- 8: https://www.youtube.com/watch?v=zPyACEvZvOA
- 9: Using Swiper in React in 2024 nolimits4web/swiper#7354
- 10: https://codesandbox.io/s/react-swiper-pagination-3is7j
@semnil5202 죄송합니다! 제 리뷰가 부정확했습니다. 코드를 다시 검토해보니 현재 코드에서는 커스텀 Pagination 컴포넌트를 정의하고 있는데, 이것을 Swiper의 modules 배열에 포함시키고 있습니다.
제가 살펴본 결과:
- 현재 파일에서는
import { Navigation, Autoplay } from 'swiper/modules';로 Swiper 모듈을 가져오고 있지만, Pagination 모듈은 import하지 않았습니다. - 대신 컴포넌트 내부에 로컬
Pagination함수형 컴포넌트를 정의했습니다. - 이 로컬 컴포넌트는 Swiper의
modules배열에 포함되면 안 됩니다.
올바른 접근 방식은 다음 두 가지 중 하나입니다:
- Swiper의 내장 Pagination을 사용하려면:
import { Navigation, Autoplay, Pagination } from 'swiper/modules';
// 그리고 modules={[Pagination, Navigation, Autoplay]}- 현재처럼 커스텀 pagination UI를 사용하려면:
modules={[Navigation, Autoplay]} // Pagination 제외
// 커스텀 Pagination 컴포넌트는 JSX에서 별도로 렌더링제공해주신 레퍼런스를 통해 올바른 사용법을 확인할 수 있었습니다. 잘못된 리뷰로 혼란을 드려 죄송합니다.
✏️ Learnings added
Learnt from: semnil5202
PR: depromeet/Took-FE#39
File: src/features/onboarding/ui/OnboardingCarousel.tsx:56-56
Timestamp: 2025-03-01T15:22:18.564Z
Learning: Swiper 라이브러리 사용 시, `modules` 속성에는 'swiper/modules'에서 import한 모듈만 포함되어야 하며, 로컬에서 정의한 React 컴포넌트는 포함하지 않아야 합니다. 커스텀 pagination UI는 별도의 JSX로 구현하는 것이 적절합니다.
Learnt from: semnil5202
PR: depromeet/Took-FE#39
File: src/features/onboarding/ui/OnboardingCarousel.tsx:56-56
Timestamp: 2025-03-01T15:22:18.564Z
Learning: Swiper 라이브러리 사용 시, `modules` 속성에는 'swiper/modules'에서 import한 모듈만 포함되어야 하며, 로컬에서 정의한 React 컴포넌트는 포함하지 않아야 합니다. 커스텀 pagination UI는 별도의 JSX로 구현하는 것이 적절합니다.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/features/onboarding/ui/OnboardingSlide.tsx (3)
16-16: 이미지 최적화 처리.
priority속성을 사용하여 중요한 이미지로 표시한 것은 좋습니다. 다만, 모든 온보딩 이미지에priority를 적용하는 것은 초기 로딩 성능에 영향을 줄 수 있습니다. 첫 번째 슬라이드만priority를 적용하고 나머지는 적용하지 않는 것이 좋을 수 있습니다.- <Image src={imageUrl} alt="온보딩 이미지" fill className="object-contain" priority /> + <Image + src={imageUrl} + alt="온보딩 이미지" + fill + className="object-contain" + priority={isPriority} + />
isPriority속성을 추가하여 첫 번째 슬라이드에만priority를 적용하는 방식을 고려해보세요.
24-33: 코드 주석 및 조건부 렌더링 로직.코드에 "이게 좋은 방법인지는 잘 모르겠습니다.."라는 주석이 있습니다. 이 방식은 실제로 유효하지만, 다음과 같이 개선할 수 있습니다:
- 불필요한 주석은 제거하고 코드를 명확하게 작성하세요.
- 타입에 따른 조건부 렌더링을 별도 함수로 추출하는 것이 가독성을 높일 수 있습니다.
- {/* 이게 좋은 방법인지는 잘 모르겠습니다.. */} - {Array.isArray(description) ? ( - description.map((line, index) => ( - <Typography key={index} variant="title-2"> - {line} - </Typography> - )) - ) : ( - <Typography variant="body-1">{description}</Typography> - )} + {renderDescription(description)} // 컴포넌트 위에 다음 함수 추가 + const renderDescription = (description: string | string[]) => { + if (Array.isArray(description)) { + return description.map((line, index) => ( + <Typography key={index} variant="title-2"> + {line} + </Typography> + )); + } + return <Typography variant="body-1">{description}</Typography>; + };
27-29: key 속성 개선.배열 렌더링 시
index를 key로 사용하고 있습니다. 가능하다면 고유한 ID나 문자열 자체를 사용하는 것이 React 성능에 더 좋습니다. 현재 상황에서는 수정이 필수는 아니지만, 배열 항목이 동적으로 변경될 경우 고려해보세요.- <Typography key={index} variant="title-2"> + <Typography key={`description-${index}`} variant="title-2">
📜 Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/features/onboarding/ui/OnboardingSlide.tsx(1 hunks)
🔇 Additional comments (5)
src/features/onboarding/ui/OnboardingSlide.tsx (5)
1-4: 적절한 임포트 구조 사용.필요한 모듈만 가져오고 있으며, 그룹화와 간격도 적절합니다.
5-9: 명확한 인터페이스 정의.
OnboardingSlideProps인터페이스가 잘 정의되어 있습니다.description이 문자열 또는 문자열 배열을 받을 수 있어 유연한 구현이 가능합니다.
11-19: 레이아웃 구조와 이미지 배치.레이아웃 구조가 잘 설계되어 있습니다. 다만 이미지 크기를 상대적 값으로 하드코딩(h-[75%], w-[65%])한 것이 있는데, 다양한 화면 크기에 따라 어떻게 반응하는지 테스트해보세요.
다양한 모바일 디바이스에서 이미지 크기가 적절하게 보이는지 확인해 주세요.
20-22: 그라데이션 배경 및 페이지네이션 처리.검은색에서 투명으로 변화하는 그라데이션 배경이 텍스트 가독성을 높이는 좋은 접근법입니다. 페이지네이션을 조건부로 렌더링하는 것도 적절합니다.
40-41: 간결한 내보내기.컴포넌트 내보내기가 명확합니다. 필요한 경우 나중에 명명된 내보내기로 변경할 수 있습니다.
| onComplete: () => void; | ||
| } | ||
|
|
||
| export function OnboardingCarousel({ onComplete }: OnboardingCarouselProps): React.ReactElement { |
There was a problem hiding this comment.
명시적으로 React.ReactElement로 선언한 이유가 궁급합니다. 타입스크립트에서 추론이 될거 같아서요.
There was a problem hiding this comment.
엇 실수였던 것 같습니다. 수정하겠습니다 🥲
| const slides = [ | ||
| { | ||
| id: 1, | ||
| description: ['과한 시작정보는 빼고', '내가 원하는 정보만'], |
There was a problem hiding this comment.
descripttion을 하드코딩으로 관리하기 보다는 config 파일을 만들어서 관리하는것이 유지보수성에 더 좋을거 같아요.
There was a problem hiding this comment.
아아 넵. 따로 분리해서 다시 올리겠습니다!!
|
|
||
| <div className="mt-auto w-full bg-black p-6"> | ||
| <Button onClick={handleNext} className="w-full"> | ||
| <Typography variant="body-2">다음</Typography> |
There was a problem hiding this comment.
만든 타이포컴포넌트 사용하신부분 좋습니다!!
| @@ -0,0 +1,23 @@ | |||
| import Image from 'next/image'; | |||
|
|
|||
| import KakaoLoginButton from '../../ui/KakaoLoginButton'; | |||
There was a problem hiding this comment.
../ 이상의 폴더 접근은 @shared/ui로 절대경로로 접근해주시면 더 좋을거 같습니다.
|
|
||
| import OnboardingSlide from './OnboardingSlide'; | ||
|
|
||
| const slides = [ |
There was a problem hiding this comment.
slide는 config 파일로 빼서 관리하면 추후에 이미지 url이나, decription이 바뀌었을 때 관리하기 좋을거 같아요.
| interface OnboardingFeatureProps { | ||
| onComplete: () => void; | ||
| onSkip: () => void; | ||
| } | ||
|
|
||
| export function OnboardingFeature({ onComplete, onSkip }: OnboardingFeatureProps) { |
There was a problem hiding this comment.
타입에 대한 설명을 주석으로 문서화하면 좋겠습니다.
| {Array.isArray(description) ? ( | ||
| description.map((line, index) => ( | ||
| <Typography key={index} variant="title-2"> | ||
| {line} | ||
| </Typography> | ||
| )) | ||
| ) : ( |
There was a problem hiding this comment.
Array.isArray 를 통해서, 분리하기 보다는 utils 함수를 만들어서 함수인지 관리해주는것이 더 좋은 방안 같아 보입니다. es-toolkit을 써서 관리를 해줘도 좋은 부분 같네요!
There was a problem hiding this comment.
보통 저렇게 줄바꿈을 할 때 구현을 어떻게 하는지 궁금합니다.
맞는 방법인지 몰라서 ..
utils함수로 분리하겠습니다!
There was a problem hiding this comment.
저렇게 줄바꿈이라는 이야기가 이해가 되지 않는데 좀더 설명 부탁드려요~
| const [activeIndex, setActiveIndex] = useState<number>(0); | ||
| const swiperRef = useRef<SwiperType | null>(null); | ||
|
|
||
| const handleNext = (): void => { |
There was a problem hiding this comment.
타입스크립트로 유추가 충분히 되는 타입은 명시적으로 선언하지 않아도 될거 같습니다.
There was a problem hiding this comment.
SwiperType은 사용중이라 number타입 제거 하겠습니다!
| <div className="relative flex h-full w-full flex-col items-center"> | ||
| <div className="absolute inset-0 flex w-full items-center justify-center"> | ||
| <div className="relative h-[75%] w-[65%]"> | ||
| <Image src={imageUrl} alt="온보딩 이미지" fill style={{ objectFit: 'contain' }} priority /> |
There was a problem hiding this comment.
인라인 스타일로 표현하신 이유가 있을까요?? tailwindcss 에서 지원해주는 프로퍼티같아서요.
There was a problem hiding this comment.
엇 이 부분 수정해서 다시 올렸습니다!
PR에서 JSON 파싱 오류가 생겨서 발견하고 수정했습니다😊
| 카카오 로그인 | ||
| <button | ||
| onClick={kakaoLogin} | ||
| className="flex w-full items-center justify-between rounded-[12px] bg-yellow-300 px-4 py-[15px] text-gray-800" |
There was a problem hiding this comment.
rounded-[12px]는 디자인 토큰(--radius-m: 12px;)으로 저장되어 있습니다! 변수를 사용하는 방법으로 수정하는 게 좋을 것 같습니다 :)
There was a problem hiding this comment.
엇 이걸 확인을 못했내요.. 수정하겠습니다 😊
| export function LoginFeature() { | ||
| const imgUrl = '/images/login/onboarding/onboarding-login.png'; | ||
| return ( | ||
| <div className="flex h-full w-full max-w-[600px] flex-col"> |
There was a problem hiding this comment.
해당 스타일(flex h-full w-full max-w-[600px] flex-col)이 루트 div에서 전체적으로 쓰이는 스타일인 것 같아요. 이를 변수로 저장해서 사용하면 좋을 것 같은데 팀원들과 논의해보고 싶어요!
There was a problem hiding this comment.
엇 이 부분은 max-w-[600px]를 중복적으로 사용한거같아서 제거하고 루트에서만 사용하게 수정했습니다.
루트 layout.tsx에 규격을 정하고 개발하는게 좋다고 생각드네요!
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/features/onboarding/ui/OnboardingCarousel.tsx (1)
56-56:⚠️ Potential issue잘못된 모듈이 포함되어 있습니다.
modules배열에 로컬에서 정의한Pagination컴포넌트가 포함되어 있습니다. 이는 swiper의Pagination모듈과 다른 것입니다. swiper에서 가져온 모듈만 사용해야 합니다.다음과 같이 수정해주세요:
- modules={[Pagination, Navigation, Autoplay]} + modules={[Navigation, Autoplay]}
🧹 Nitpick comments (6)
src/features/auth/ui/KakaoLoginButton.tsx (2)
26-26: 빈 div 요소 개선 필요빈
<div>요소는justify-between레이아웃을 위해 사용된 것으로 보이지만, 의미론적으로 맞지 않습니다.다음과 같은 대안을 고려해보세요:
- <div></div> + <span className="w-[18px]" aria-hidden="true"></span>또는 Flexbox 레이아웃을 다음과 같이 수정할 수도 있습니다:
- className="flex w-full items-center justify-between rounded-md bg-yellow-300 px-4 py-[15px] text-gray-800" + className="flex w-full items-center rounded-md bg-yellow-300 px-4 py-[15px] text-gray-800" + <KakaoIcon className="mr-auto" /> + <Typography variant="body-2" className="absolute left-1/2 -translate-x-1/2">카카오톡으로 시작하기</Typography>
24-25: UI 요소 접근성 개선 필요KakaoIcon에 적절한 접근성 속성이 없습니다. 스크린 리더 사용자를 위해 아이콘의 목적을 명확히 해야 합니다.
다음과 같이 수정하는 것이 좋겠습니다:
- <KakaoIcon /> + <KakaoIcon aria-hidden="true" />또한 Typography 컴포넌트에 시맨틱한 의미를 추가할 수 있습니다:
- <Typography variant="body-2">카카오톡으로 시작하기</Typography> + <Typography variant="body-2" as="span">카카오톡으로 시작하기</Typography>src/features/onboarding/utils/text.ts (1)
1-7: 더 간결한 유틸리티 함수 구현을 제안합니다.이 함수는 더 간결하게 구현할 수 있습니다. 불필요한 if 문과 공백을 제거하여 가독성을 높일 수 있습니다.
다음과 같이 수정해보세요:
-export const isTextArray = (content: string | string[]) => { - if (Array.isArray(content)) { - return true; - } - - return false; -}; +export const isTextArray = (content: string | string[]): boolean => Array.isArray(content);src/features/onboarding/ui/OnboardingFeature.tsx (1)
3-4: 일관된 import 스타일을 사용하는 것이 좋습니다.
OnboardingCarousel은 기본 import를,OnboardingHeader는 named import를, 같은 디렉토리의 파일임에도 다르게 사용하고 있습니다. 일관된 스타일을 유지하는 것이 코드 가독성에 도움이 됩니다.둘 중 하나의 스타일로 통일하는 것을 권장합니다:
-import OnboardingCarousel from './OnboardingCarousel'; -import { OnboardingHeader } from './OnboardingHeader'; +// 모두 기본 import 사용 +import OnboardingCarousel from './OnboardingCarousel'; +import OnboardingHeader from './OnboardingHeader';또는
-import OnboardingCarousel from './OnboardingCarousel'; -import { OnboardingHeader } from './OnboardingHeader'; +// 모두 named import 사용 +import { OnboardingCarousel } from './OnboardingCarousel'; +import { OnboardingHeader } from './OnboardingHeader';src/features/onboarding/ui/OnboardingSlide.tsx (1)
26-34: 불필요한 타입 단언을 제거하세요.
isTextArray함수로 이미 타입을 확인한 후에는 타입 단언(as string[],as string)이 필요하지 않습니다. TypeScript는 조건문 내에서 타입을 정확히 추론할 수 있습니다.다음과 같이 수정해보세요:
{isTextArray(description) ? ( - (description as string[]).map((line, index) => ( + description.map((line, index) => ( <Typography key={index} variant="title-2"> {line} </Typography> )) ) : ( - <Typography variant="body-1">{description as string}</Typography> + <Typography variant="body-1">{description}</Typography> )}TypeScript는
isTextArray(description)이 true일 때description이string[]타입이고, false일 때string타입임을 알 수 있습니다.src/features/onboarding/ui/OnboardingCarousel.tsx (1)
26-36: 타입 선언 없이도 충분합니다.TypeScript는 대부분의 경우 타입을 명시적으로 선언하지 않아도 충분히 유추할 수 있습니다. 이전 리뷰 의견처럼 불필요한 타입 선언은 제거하는 것이 좋습니다.
다음과 같이 수정해보세요:
-const handleNext = (): void => { +const handleNext = () => { if (activeIndex === slides.length - 1) { onComplete(); } else if (swiperRef.current) { swiperRef.current.slideNext(); } }; -const handleSlideChange = (swiper: SwiperType): void => { +const handleSlideChange = (swiper: SwiperType) => { setActiveIndex(swiper.realIndex); };
📜 Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
src/features/auth/login/ui/LoginFeature.tsx(1 hunks)src/features/auth/ui/KakaoLoginButton.tsx(2 hunks)src/features/onboarding/config/slides.ts(1 hunks)src/features/onboarding/ui/OnboardingCarousel.tsx(1 hunks)src/features/onboarding/ui/OnboardingFeature.tsx(1 hunks)src/features/onboarding/ui/OnboardingSlide.tsx(1 hunks)src/features/onboarding/utils/text.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/features/onboarding/config/slides.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/features/auth/login/ui/LoginFeature.tsx
🔇 Additional comments (5)
src/features/auth/ui/KakaoLoginButton.tsx (2)
3-4: 주석에 디자인 변경 가능성을 명시한 것은 좋습니다임시 구현임을 알려주는 주석은 팀 간 소통에 도움이 됩니다. 그러나 가능하다면 어떤 부분이 변경될 수 있는지 좀 더 구체적으로 명시하면 더 좋을 것 같습니다.
22-22: 디자인 토큰 사용 권장
rounded-md와py-[15px]에서 하드코딩된 값을 사용하고 있습니다. 이전 리뷰 내용에 따르면 디자인 토큰(CSS 변수)을 사용하는 것이 권장되고 있습니다.디자인 토큰을 활용하여 다음과 같이 수정하는 것이 좋겠습니다:
- className="flex w-full items-center justify-between rounded-md bg-yellow-300 px-4 py-[15px] text-gray-800" + className="flex w-full items-center justify-between rounded-[var(--radius-m)] bg-yellow-300 px-4 py-[var(--spacing-m)] text-gray-800"혹은 Tailwind 설정에 디자인 토큰이 정의되어 있다면 해당 클래스를 사용하세요.
src/features/onboarding/ui/OnboardingFeature.tsx (1)
6-12: 문서화가 잘 되어있습니다!이전 리뷰 요청대로 타입에 대한 설명을 주석으로 잘 문서화하셨습니다. 사용 목적과 각 prop의 역할이 명확하게 설명되어 있어 좋습니다.
src/features/onboarding/ui/OnboardingSlide.tsx (1)
5-5: 이전 리뷰 제안을 잘 반영하셨습니다.이전 리뷰에서 제안된 대로 타입 확인 로직을 유틸리티 함수로 분리하여 구현하셨네요. 이렇게 하면 코드의 재사용성과 가독성이 향상됩니다.
src/features/onboarding/ui/OnboardingCarousel.tsx (1)
12-12: 설정 파일로 분리한 것이 좋습니다.이전 리뷰에서 제안된 대로 슬라이드 데이터를 config 파일로 분리하여 관리하고 있네요. 이렇게 하면 추후에 이미지 URL이나 설명이 변경되었을 때 더 쉽게 관리할 수 있습니다.
| const Pagination = () => ( | ||
| <div className="mb-8 flex space-x-2"> | ||
| {slides.map((_, index) => ( | ||
| <button | ||
| key={index} | ||
| className={`h-2 rounded-full transition-all ${ | ||
| index === activeIndex ? 'w-2 bg-white' : 'w-2 bg-white opacity-50' | ||
| }`} | ||
| onClick={() => swiperRef.current?.slideToLoop(index)} | ||
| /> | ||
| ))} | ||
| </div> | ||
| ); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Pagination 컴포넌트 이름 수정을 고려하세요.
현재 정의한 Pagination 컴포넌트는 Swiper의 Pagination 모듈과 이름이 동일하여 혼란을 줄 수 있습니다. 이는 56번 줄의 modules 배열에서 발생한 오류의 원인이기도 합니다.
컴포넌트 이름을 더 명확하게 변경하는 것이 좋습니다:
-const Pagination = () => (
+const SlidePagination = () => (
<div className="mb-8 flex space-x-2">
{slides.map((_, index) => (
<button
key={index}
className={`h-2 rounded-full transition-all ${
index === activeIndex ? 'w-2 bg-white' : 'w-2 bg-white opacity-50'
}`}
onClick={() => swiperRef.current?.slideToLoop(index)}
/>
))}
</div>
);그리고 70번 줄도 함께 수정해야 합니다:
-<OnboardingSlide description={slide.description} imageUrl={slide.imageUrl} pagination={<Pagination />} />
+<OnboardingSlide description={slide.description} imageUrl={slide.imageUrl} pagination={<SlidePagination />} />📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const Pagination = () => ( | |
| <div className="mb-8 flex space-x-2"> | |
| {slides.map((_, index) => ( | |
| <button | |
| key={index} | |
| className={`h-2 rounded-full transition-all ${ | |
| index === activeIndex ? 'w-2 bg-white' : 'w-2 bg-white opacity-50' | |
| }`} | |
| onClick={() => swiperRef.current?.slideToLoop(index)} | |
| /> | |
| ))} | |
| </div> | |
| ); | |
| const SlidePagination = () => ( | |
| <div className="mb-8 flex space-x-2"> | |
| {slides.map((_, index) => ( | |
| <button | |
| key={index} | |
| className={`h-2 rounded-full transition-all ${ | |
| index === activeIndex ? 'w-2 bg-white' : 'w-2 bg-white opacity-50' | |
| }`} | |
| onClick={() => swiperRef.current?.slideToLoop(index)} | |
| /> | |
| ))} | |
| </div> | |
| ); | |
| // ... | |
| <OnboardingSlide | |
| description={slide.description} | |
| imageUrl={slide.imageUrl} | |
| pagination={<SlidePagination />} | |
| /> |
src/app/login/page.tsx
Outdated
| const handleOnboardingComplete = () => { | ||
| setShowOnboarding(false); | ||
| }; | ||
|
|
||
| const handleSkipOnboarding = () => { | ||
| setShowOnboarding(false); | ||
| }; |
There was a problem hiding this comment.
[Q]
같은 역할의 함수를 다른 명칭의 이벤트 핸들러로 감싼 이유가 있으실까요? 보통 흔한 케이스는 아닌 것 같아서요~
추후 로직이 변경될 여지가 있다면 TODO 주석등으로 표기해두면 조금 더 맥락을 이해하기 쉬울 것 같아요~
There was a problem hiding this comment.
맞아요 하면서 고민했던 부분이였는데..
혹시 나중에 액션이 달라질 경우가 있을까? 하고 작성했습니다.
같은 기능이라 하나로 합치는게 더 좋을거같네요~
| {/* 이미지 배치가 안되서 임시로 top-[45%]사용했습니다. */} | ||
| <div className="absolute left-1/2 top-[45%] h-[23%] w-full -translate-x-1/2 -translate-y-1/2 transform"> | ||
| {/* 이미지 가운데 배치가 안되서 임시로 pl-[1rem]사용했습니다. */} | ||
| <Image src={imgUrl} alt="온보딩 이미지" fill className="object-contain pl-[1rem]" priority /> |
There was a problem hiding this comment.
넵넵 확인했습니다. 추후 온보딩 디자인이 변경될 가능성이 높아서, 우선은 이와같이 진행해도 될 것 같습니다.
| const handleSlideChange = (swiper: SwiperType): void => { | ||
| setActiveIndex(swiper.realIndex); | ||
| }; |
There was a problem hiding this comment.
[P3]
위에서도 장원이형이 말했지만, 이런 함수 반환타입은 굳이 명시하지 않아도 될 것 같습니다. 타입 추론에 맡기고 추후 props로 넘기거나 할 때 interface 단에서 타입 검사가 가능하므로 생략해도 좋을 것 같아요.
| <div className="flex h-full w-full flex-col items-center"> | ||
| <div className="w-full flex-1"> | ||
| <Swiper | ||
| modules={[Pagination, Navigation, Autoplay]} |
There was a problem hiding this comment.
There was a problem hiding this comment.
[Q]
혹시 이 로직이 유틸 함수로까지 분리된 이유가 있을까요??
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (8)
src/features/onboarding/ui/OnboardingFeature.tsx (1)
6-11: 문서화가 개선되었지만 더 명확하게 수정해 주세요.JSDoc 주석이 추가되어 좋습니다만,
showOnboarding상태에 대한 설명이 있는데 이 컴포넌트에서는 해당 prop을 직접 받지 않습니다. 부모 컴포넌트에서 사용되는 상태일 수 있지만, 이 컴포넌트의 문서화는 실제로 받는 props에 집중하는 것이 좋습니다.아래와 같이 수정하는 것을 권장합니다:
/** * OnboardingFeatureProps 설명 - * onComplete , showOnboarding 상태를 false로 바구는 함수입니다. - * 헤더의 건너뛰기 , 모든 온보딩 페이지를 다 확인했을 때 사용합니다. - * onComplete = 온보딩 마지막 페이지에서 사용 + * @param onComplete - 온보딩을 완료하거나 건너뛸 때 호출되는 콜백 함수 + * 헤더의 '건너뛰기' 버튼이나 마지막 온보딩 페이지에서 호출됩니다. */src/features/onboarding/ui/OnboardingHeader.tsx (2)
3-5: 인터페이스에 JSDoc 주석 추가가 필요합니다.
OnboardingHeaderProps인터페이스에 대한 설명 주석이 없습니다. 코드의 일관성과 문서화를 위해 JSDoc 주석을 추가해 주세요.+/** + * OnboardingHeader 컴포넌트의 Props 타입 정의 + * @param onComplete - 온보딩 건너뛰기 버튼 클릭 시 호출되는 콜백 함수 + */ interface OnboardingHeaderProps { onComplete: () => void; }
9-14: 불필요한 빈 div 요소가 있습니다.헤더 내부에 내용이 없는 빈
<div>요소가 있습니다. 이는 레이아웃 조정을 위한 것으로 보이지만, 의미 없는 요소는 제거하거나 목적을 명확히 하는 것이 좋습니다.아래 방법 중 하나를 사용해 보세요:
- 빈 div를 제거하고 레이아웃 조정:
<header className="flex items-center justify-between p-4"> - <div></div> <button onClick={onComplete} className="font-medium text-gray-500 ml-auto"> <Typography variant="body-2">건너뛰기</Typography> </button> </header>
- 또는 빈 div에 의미 부여:
<header className="flex items-center justify-between p-4"> - <div></div> + <div aria-hidden="true" className="w-16"> + {/* 레이아웃 밸런스를 위한 빈 공간 */} + </div> <button onClick={onComplete} className="font-medium text-gray-500"> <Typography variant="body-2">건너뛰기</Typography> </button> </header>src/features/onboarding/ui/OnboardingSlide.tsx (2)
15-15: 이미지의 alt 텍스트가 더 구체적일 필요가 있습니다.현재 alt 텍스트 "온보딩 이미지"는 너무 일반적입니다. 스크린 리더 사용자에게 더 의미 있는 정보를 제공하기 위해 각 슬라이드의 내용을 설명하는 구체적인 alt 텍스트를 사용하는 것이 좋습니다.
- <Image src={imageUrl} alt="온보딩 이미지" fill className="object-contain" priority /> + <Image src={imageUrl} alt={`온보딩 이미지: ${description[0]}`} fill className="object-contain" priority />또는 props에 별도의 alt 텍스트를 추가하는 것도 고려해 보세요:
interface OnboardingSlideProps { description: string[]; imageUrl: string; + imageAlt: string; }
12-28: 시맨틱 HTML 요소 사용을 고려해 보세요.현재 슬라이드는
div요소들로만 구성되어 있습니다. 접근성과 SEO를 향상시키기 위해 더 의미 있는 시맨틱 HTML 요소를 사용하는 것이 좋습니다.- <div className="relative flex h-full w-full flex-col items-center"> + <article className="relative flex h-full w-full flex-col items-center"> <div className="absolute inset-0 flex w-full items-center justify-center"> <div className="relative h-[75%] w-[65%]"> <Image src={imageUrl} alt="온보딩 이미지" fill className="object-contain" priority /> </div> </div> <div className="absolute bottom-0 left-0 right-0 h-2/5 bg-gradient-to-t from-black via-black to-transparent pt-32"> - <div className="flex flex-col items-center px-6 pt-16"> + <footer className="flex flex-col items-center px-6 pt-16"> {description.map((line, index) => ( <Typography key={index} variant="title-2"> {line} </Typography> ))} - </div> + </footer> </div> - </div> + </article>src/app/login/page.tsx (1)
17-25: 인라인 스타일 대신 Tailwind CSS 클래스 사용을 고려해 보세요.현재 배경 이미지를 설정하기 위해 인라인 스타일을 사용하고 있습니다. Tailwind CSS를 사용 중이므로, 가능한 한 인라인 스타일 대신 Tailwind 클래스를 사용하는 것이 일관성 있는 스타일링에 도움이 됩니다.
<div - className="h-full w-full max-w-[600px] flex-col items-center justify-center bg-black" - style={{ - backgroundImage: `url(${'/images/login/onboarding/onboarding-bg.png'})`, - backgroundSize: 'contain', - backgroundPosition: 'bottom', - backgroundRepeat: 'no-repeat', - }} + className="h-full w-full max-w-[600px] flex-col items-center justify-center bg-black bg-[url('/images/login/onboarding/onboarding-bg.png')] bg-contain bg-bottom bg-no-repeat" >또는 이미지 경로를 상수로 분리하여 관리하는 것도 좋은 방법입니다:
+ const ONBOARDING_BG_IMAGE = '/images/login/onboarding/onboarding-bg.png'; ... <div className="h-full w-full max-w-[600px] flex-col items-center justify-center bg-black" style={{ - backgroundImage: `url(${'/images/login/onboarding/onboarding-bg.png'})`, + backgroundImage: `url(${ONBOARDING_BG_IMAGE})`, backgroundSize: 'contain', backgroundPosition: 'bottom', backgroundRepeat: 'no-repeat', }} >src/features/onboarding/ui/OnboardingCarousel.tsx (2)
3-6: Swiper 관련 불필요한 import가 있을 수 있습니다.현재 코드에서는
Pagination,Navigation,Autoplay모듈을 모두 가져오고 있지만,Navigation은 56번 줄에서navigation={false}로 설정되어 실제로 사용되지 않는 것으로 보입니다. 사용하지 않는 모듈은 제거하는 것이 번들 크기를 줄이는 데 도움이 될 수 있습니다.-import { Navigation, Autoplay, Pagination } from 'swiper/modules'; +import { Autoplay, Pagination } from 'swiper/modules'; // 그리고 43번 줄도 변경 -modules={[Pagination, Navigation, Autoplay]} +modules={[Pagination, Autoplay]}
24-24: useRef 타입 정의 간소화가 가능합니다.
useRef<SwiperType | null>(null)에서null유니온 타입은 불필요할 수 있습니다. TypeScript의useRef제네릭은 이미null초기값을 이해하고 올바르게 타입 추론을 합니다.-const swiperRef = useRef<SwiperType | null>(null); +const swiperRef = useRef<SwiperType>(null);
📜 Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/app/login/page.tsx(1 hunks)src/features/onboarding/ui/OnboardingCarousel.tsx(1 hunks)src/features/onboarding/ui/OnboardingFeature.tsx(1 hunks)src/features/onboarding/ui/OnboardingHeader.tsx(1 hunks)src/features/onboarding/ui/OnboardingSlide.tsx(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
src/features/onboarding/ui/OnboardingCarousel.tsx (1)
Learnt from: semnil5202
PR: depromeet/Took-FE#39
File: src/features/onboarding/ui/OnboardingCarousel.tsx:56-56
Timestamp: 2025-03-01T15:22:18.677Z
Learning: Swiper 라이브러리 사용 시, `modules` 속성에는 'swiper/modules'에서 import한 모듈만 포함되어야 하며, 로컬에서 정의한 React 컴포넌트는 포함하지 않아야 합니다. 커스텀 pagination UI는 별도의 JSX로 구현하는 것이 적절합니다.
🔇 Additional comments (8)
src/features/onboarding/ui/OnboardingFeature.tsx (1)
13-24: 컴포넌트 구조가 깔끔하고 잘 구성되어 있습니다.Props에 대한 적절한 타입 정의와 명확한 컴포넌트 구조로 이루어져 있습니다.
onCompleteprop이 하위 컴포넌트들에게 적절히 전달되고 있습니다.src/features/onboarding/ui/OnboardingSlide.tsx (1)
5-8: 인터페이스에 JSDoc 주석 추가가 필요합니다.
OnboardingSlideProps인터페이스에 대한 설명 주석이 없습니다. 코드의 일관성과 문서화를 위해 JSDoc 주석을 추가해 주세요.+/** + * OnboardingSlide 컴포넌트의 Props 타입 정의 + * @param description - 슬라이드에 표시될 설명 텍스트 배열 + * @param imageUrl - 슬라이드에 표시될 이미지 URL + */ interface OnboardingSlideProps { description: string[]; imageUrl: string; }src/app/login/page.tsx (2)
11-13: 함수 네이밍과 구현이 명확합니다.
handleOnboardingComplete함수가 명확한 목적을 가지고 있으며, 이전 리뷰에서 제기된 중복 이벤트 핸들러 문제가 잘 해결되었습니다.
26-26: 조건부 렌더링이 깔끔하게 구현되었습니다.
showOnboarding상태에 따라OnboardingFeature와LoginFeature컴포넌트를 조건부로 렌더링하는 방식이 간결하고 효과적입니다.src/features/onboarding/ui/OnboardingCarousel.tsx (4)
41-57: Swiper 모듈 설정이 올바르게 구현되었습니다.이전 코드리뷰에서 지적된 내용을 반영하여
Pagination모듈을 올바르게 사용하고 있습니다.swiper/modules에서 가져온 모듈만modules배열에 포함시킨 것이 좋습니다. 커스텀 페이지네이션 스타일링도 적절하게pagination옵션을 통해 구현되었습니다.
67-73: 커스텀 페이지네이션 요소 배치가 잘되어 있습니다.페이지네이션 요소를 절대 위치(
absolute)로 설정하고 반응형 디자인(sm,md,lg브레이크포인트)을 적용한 점이 좋습니다. 이를 통해 다양한 화면 크기에서 일관된 사용자 경험을 제공할 수 있습니다.
26-32: edge case 처리가 잘 되어 있습니다.
handleNext함수에서 마지막 슬라이드에 도달했을 때onComplete콜백을 호출하고, 그렇지 않은 경우에만 다음 슬라이드로 이동하는 로직이 잘 구현되어 있습니다. 또한swiperRef.current가 존재하는지 확인하는 null 체크도 잘 되어 있습니다.
77-79: 공유 컴포넌트 사용이 적절합니다.
Button과Typography공유 컴포넌트를 활용하여 일관된 디자인 시스템을 적용한 점이 좋습니다. 특히Typography컴포넌트를 통해 텍스트 스타일을 일관되게 관리할 수 있습니다.
📌 개요
📋 변경사항
기능
화면
🎸 기타
✅ 체크사항
Summary by CodeRabbit
New Features
Chores