-
Notifications
You must be signed in to change notification settings - Fork 1
[FEAT] 랜딩페이지 UI 구현 #89
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
Conversation
Walkthrough이 변경 사항은 Changes
Assessment against linked issues
Possibly related PRs
Suggested reviewers
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
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (4)
src/constants/swiperOptions.ts (2)
11-13: 자동 재생 시간을 조정하는 것이 좋습니다.현재 3초(3000ms)로 설정된 자동 재생 시간은 사용자가 콘텐츠를 충분히 읽기에는 다소 짧을 수 있습니다. 사용자 경험 향상을 위해 5초(5000ms) 정도로 늘리는 것을 고려해보세요.
autoplay: { - delay: 3000, + delay: 5000, },
3-18: 타입 안전성을 개선하면 좋겠습니다.
swiperOptions객체에 대한 명시적인 타입 정의를 추가하면 유지보수성이 향상될 것 같습니다.+ import { SwiperOptions } from 'swiper/types'; + - export const swiperOptions = { + export const swiperOptions: SwiperOptions = {src/routes/route.tsx (1)
4-4: import 순서를 수정해주세요.eslint 규칙에 따라 import 문의 순서를 조정해야 합니다.
@/경로의 import를 먼저 선언해주세요.import { createBrowserRouter } from 'react-router-dom'; import { App } from '@/App'; - import { Landing } from '@/pages/Landing'; import { Link } from '@/pages/Link'; import { MapView } from '@/pages/MapView'; + import { Landing } from '@/pages/Landing';src/pages/Landing.tsx (1)
11-11: 이미지 배열을 상수로 분리하고 타입을 정의해주세요.하드코딩된 이미지 배열을 별도의 상수 파일로 분리하고, 적절한 타입을 정의하면 유지보수성이 향상될 것 같습니다.
- const img = ['current', 'extract', 'maker', 'makers', 'youtube']; + // src/constants/images.ts + export const LANDING_IMAGES = ['current', 'extract', 'maker', 'makers', 'youtube'] as const; + export type LandingImage = typeof LANDING_IMAGES[number];
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
⛔ Files ignored due to path filters (8)
.yarn/cache/swiper-npm-11.1.15-30ca9116f5-3faabc5b33.zipis excluded by!**/.yarn/**,!**/*.zip.yarn/install-state.gzis excluded by!**/.yarn/**,!**/*.gzpublic/landing/current.pngis excluded by!**/*.pngpublic/landing/extract.pngis excluded by!**/*.pngpublic/landing/maker.pngis excluded by!**/*.pngpublic/landing/makers.pngis excluded by!**/*.pngpublic/landing/youtube.pngis excluded by!**/*.pngyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (4)
package.json(1 hunks)src/constants/swiperOptions.ts(1 hunks)src/pages/Landing.tsx(1 hunks)src/routes/route.tsx(2 hunks)
🧰 Additional context used
🪛 eslint
src/pages/Landing.tsx
[error] 1-1: react-router-dom import should occur after import of @/constants/swiperOptions
(import/order)
🔇 Additional comments (2)
package.json (2)
Line range hint 1-89: 전반적인 package.json 구조가 잘 구성되어 있습니다
의존성들이 잘 구성되어 있고, 개발 도구들도 적절하게 설정되어 있습니다.
40-40: Swiper 라이브러리 의존성 추가 검토 필요
다음 사항들을 고려해주시기 바랍니다:
- 버전 고정을 위해
^11.1.15대신11.1.15로 변경하는 것이 안정성 측면에서 좋을 것 같습니다. - 번들 사이즈 최적화를 위해 필요한 모듈만 import하는 것을 권장드립니다.
✅ Verification successful
Let me check how Swiper is being imported in the codebase to verify the bundle size impact concern.
Swiper 의존성 관련 검토 결과
분석 결과 다음과 같은 사항들이 확인되었습니다:
-
현재 코드베이스에서는 필요한 모듈만 선택적으로 import하고 있어 번들 사이즈 최적화가 잘 되어있습니다:
swiper/react에서 핵심 컴포넌트만 importswiper/modules에서 필요한 기능(Pagination, Navigation, Autoplay)만 import
-
보안 취약점 검사 결과, 현재 사용 중인 버전 11.1.15는 안전합니다. (발견된 취약점은 6.5.1 이전 버전에만 해당)
-
버전 고정(
^제거)에 대한 초기 제안은 유효하나, Swiper의 안정적인 업데이트 기록을 고려할 때 선택사항으로 조정합니다.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify Swiper package size and security
# Check bundle size impact
curl -s https://bundlephobia.com/api/size?package=swiper@11.1.15 | jq '{
size: .size,
gzippedSize: .gzipSize,
dependencyCount: .dependencyCount
}'
# Check for known vulnerabilities
gh api graphql -f query='
{
securityVulnerabilities(first: 5, ecosystem: NPM, package: "swiper") {
nodes {
advisory {
summary
severity
publishedAt
}
vulnerableVersionRange
firstPatchedVersion {
identifier
}
}
}
}'
Length of output: 758
Script:
#!/bin/bash
# Search for Swiper imports in the codebase
rg "from ['|\"]swiper" -A 2
# Search for Swiper CSS imports
rg "@import.*swiper"
Length of output: 389
| <div className="absolute bottom-3 w-full max-w-[30rem] px-4 mb-5"> | ||
| <Button variant="primary" size="large" onClick={() => navigate('/map')}> | ||
| 시작하기 | ||
| </Button> | ||
| </div> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
버튼의 위치 지정 방식을 개선해주세요.
현재 absolute 포지셔닝을 사용하고 있는데, 이는 다양한 화면 크기에서 문제를 일으킬 수 있습니다. Flexbox를 활용한 레이아웃으로 변경하는 것이 좋겠습니다.
- <div className="absolute bottom-3 w-full max-w-[30rem] px-4 mb-5">
+ <div className="w-full max-w-[30rem] px-4 mb-5 mt-auto">Committable suggestion skipped: line range outside the PR's diff.
src/pages/Landing.tsx
Outdated
| {img.map((item, index) => ( | ||
| <SwiperSlide key={index} className="flex justify-center items-center "> | ||
| <img src={`/landing/${item}.png`} className="w-full h-full object-cover" /> | ||
| </SwiperSlide> | ||
| ))} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
접근성과 성능 개선이 필요합니다.
- 이미지에 alt 텍스트가 없어 스크린 리더 사용자의 접근성이 떨어집니다.
- 배열의 인덱스를 key로 사용하는 것은 권장되지 않습니다.
- 이미지 로딩 최적화를 위해 lazy loading을 적용하면 좋겠습니다.
- {img.map((item, index) => (
+ {LANDING_IMAGES.map((item) => (
- <SwiperSlide key={index} className="flex justify-center items-center ">
+ <SwiperSlide key={item} className="flex justify-center items-center">
- <img src={`/landing/${item}.png`} className="w-full h-full object-cover" />
+ <img
+ src={`/landing/${item}.png`}
+ alt={`${item} 설명 이미지`}
+ loading="lazy"
+ className="w-full h-full object-cover"
+ />
</SwiperSlide>
))}📝 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.
| {img.map((item, index) => ( | |
| <SwiperSlide key={index} className="flex justify-center items-center "> | |
| <img src={`/landing/${item}.png`} className="w-full h-full object-cover" /> | |
| </SwiperSlide> | |
| ))} | |
| {LANDING_IMAGES.map((item) => ( | |
| <SwiperSlide key={item} className="flex justify-center items-center"> | |
| <img | |
| src={`/landing/${item}.png`} | |
| alt={`${item} 설명 이미지`} | |
| loading="lazy" | |
| className="w-full h-full object-cover" | |
| /> | |
| </SwiperSlide> | |
| ))} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (3)
src/pages/Landing.tsx (3)
1-10: 임포트 순서를 개선해주세요코드의 일관성을 위해 임포트 순서를 다음과 같이 정리하는 것이 좋습니다:
- 외부 라이브러리 (
react-router-dom,swiper)- 스타일 (
swiper/css)- 내부 컴포넌트 (
@/components/*)- 상수 (
@/constants/*)+ import { Swiper, SwiperSlide } from 'swiper/react'; + import { useNavigate } from 'react-router-dom'; + + import 'swiper/css'; + import 'swiper/css/pagination'; + + import { Button } from '@/components/common/Button'; + import { Icon } from '@/components/common/Icon'; + import { Body1 } from '@/components/common/Typography'; + + import { swiperOptions } from '@/constants/swiperOptions'; - import { useNavigate } from 'react-router-dom'; - import { Swiper, SwiperSlide } from 'swiper/react'; - - import 'swiper/css'; - import 'swiper/css/pagination'; - import { Button } from '@/components/common/Button'; - import { Icon } from '@/components/common/Icon'; - import { Body1 } from '@/components/common/Typography'; - import { swiperOptions } from '@/constants/swiperOptions';🧰 Tools
🪛 eslint
[error] 1-1:
react-router-domimport should occur after import of@/constants/swiperOptions(import/order)
11-12: 상수에 대한 문서화가 필요합니다
LANDING_IMAGES배열의 각 이미지가 어떤 내용을 나타내는지 주석으로 설명해주세요. 또한 이미지 파일명을 더 명확하게 지정하면 좋겠습니다.+ // 랜딩 페이지 캐러셀에 표시될 이미지 목록 + // current: 현재 위치 화면 + // extract: 정보 추출 화면 + // maker: 제작자 정보 + // makers: 제작자 목록 + // youtube: 유튜브 연동 화면 const LANDING_IMAGES = ['current', 'extract', 'maker', 'makers', 'youtube'];
17-21: 반응형 디자인 개선이 필요합니다현재 고정된 마진과 크기 값들이 모든 화면 크기에서 최적의 레이아웃을 보장하지 않을 수 있습니다. Tailwind의 반응형 클래스를 활용하여 개선해보세요.
- <div className="flex flex-col items-center justify-center gap-5 mt-32"> + <div className="flex flex-col items-center justify-center gap-5 mt-16 md:mt-24 lg:mt-32"> - <Icon name="findyLogo1" className="w-2/3 h-20" /> + <Icon name="findyLogo1" className="w-2/3 h-16 md:h-20" />
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
src/pages/Landing.tsx(1 hunks)
🧰 Additional context used
🪛 eslint
src/pages/Landing.tsx
[error] 1-1: react-router-dom import should occur after import of @/constants/swiperOptions
(import/order)
🔇 Additional comments (2)
src/pages/Landing.tsx (2)
35-39: 버튼의 위치 지정 방식을 개선해주세요
현재 absolute 포지셔닝을 사용하고 있는데, 이는 다양한 화면 크기에서 문제를 일으킬 수 있습니다. Flexbox를 활용한 레이아웃으로 변경하는 것이 좋겠습니다.
22-33: Swiper 설정을 검증해주세요
현재 Swiper 구성이 올바르게 작동하는지 확인이 필요합니다:
- 이미지 크기가 일관되게 유지되는지
- 자동 재생 시간이 사용자 경험에 적절한지
- 터치/스와이프 동작이 모바일에서 잘 작동하는지
✅ Verification successful
Swiper 설정이 적절하게 구성되어 있습니다
Swiper 설정을 검토한 결과:
- 모든 이미지 파일이
public/landing디렉토리에 정상적으로 존재합니다 - 슬라이더 설정이 최적화되어 있습니다:
- 3초 자동 재생 간격 (delay: 3000)
- 터치/스와이프를 위한 pagination과 navigation 모듈 활성화
- 무한 루프 (loop: true)
- 한 번에 하나의 슬라이드만 표시 (slidesPerView: 1)
- 슬라이드 간 적절한 간격 (spaceBetween: 10)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Swiper 설정 검증
# swiperOptions 상수의 구성 확인
ast-grep --pattern 'export const swiperOptions = {
$$$
}'
# 이미지 파일 존재 여부 확인
for img in current extract maker makers youtube; do
fd "${img}.png" public/landing
done
Length of output: 1463
관련 이슈
close #84
📑 작업 내용
swiper라이브러리 설치 후 캐러셀 구현하였습니다.2024-12-09.11.53.32.mov
💬 리뷰 중점 사항/기타 참고 사항
Summary by CodeRabbit
새로운 기능
Landing컴포넌트 추가.Landing컴포넌트를 기본 뷰로 지정.문서화
swiperOptions.ts파일에 Swiper 구성 옵션 추가.