-
Notifications
You must be signed in to change notification settings - Fork 1
[7팀] {치이이즈} 프론트엔드 코드리뷰 #56
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
base: review
Are you sure you want to change the base?
Conversation
Summary of ChangesHello @dasosann, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! 이 Pull Request는 치이이즈 프론트엔드 프로젝트의 초기 구조를 설정하고 핵심 기능을 구현하는 광범위한 변경 사항을 포함합니다. Next.js 15를 기반으로 프로젝트 환경을 구축하고, shadcn/ui 및 vaul과 같은 라이브러리를 활용하여 UI 컴포넌트를 통합했습니다. 앨범 생성, 사진 업로드, 상세 보기, 공유 등 서비스의 주요 흐름을 구성하는 페이지와 기능들이 추가되었으며, Zustand를 통한 상태 관리와 NCP Presigned URL을 이용한 효율적인 파일 업로드 시스템이 마련되었습니다. 또한, 온보딩 및 카카오 소셜 로그인 기능이 도입되어 사용자 경험의 시작점을 완성했습니다. 전반적으로 프로젝트의 기반을 다지고 핵심적인 사용자 상호작용을 가능하게 하는 데 중점을 둔 변경입니다. Highlights
Ignored Files
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
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.
Code Review
전반적으로 Next.js 프로젝트의 초기 설정이 매우 체계적으로 잘 구성되어 있습니다. Docker, ESLint, Prettier, TailwindCSS 설정과 함께 디자인 토큰 시스템을 자동화하는 스크립트까지 포함되어 있어 인상적입니다. 다만, 몇 가지 개선점을 제안드립니다. 주로 의존성 버전 관리, 서버 컴포넌트의 타입 안정성, 클라이언트 및 서버 환경에서의 인증 처리 일관성 등에 대한 내용입니다. 아래의 상세 리뷰를 확인해주세요.
| }: { | ||
| params: Promise<{ albumId: string }>; | ||
| }) { | ||
| const { albumId } = await params; |
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.
| params: Promise<{ albumId: string }>; | ||
| }; | ||
|
|
||
| export default async function Page({ params }: PageProps) { | ||
| const { albumId } = await params; |
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.
페이지 컴포넌트의 params prop 타입이 Promise로 잘못 지정되었습니다. App Router에서 params는 Promise가 아닌 일반 객체로 전달됩니다. 이로 인해 타입 에러가 발생하며, await params 구문도 올바르게 동작하지 않습니다. 아래와 같이 수정해야 합니다.
| params: Promise<{ albumId: string }>; | |
| }; | |
| export default async function Page({ params }: PageProps) { | |
| const { albumId } = await params; | |
| params: { albumId: string }; | |
| }; | |
| export default async function Page({ params }: PageProps) { | |
| const { albumId } = params; |
| params: Promise<{ | ||
| albumId: string; | ||
| }>; | ||
| }; | ||
|
|
||
| export default async function Page({ params }: PageProps) { | ||
| const { albumId } = await params; |
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.
| "@radix-ui/react-slot": "^1.2.3", | ||
| "@tanstack/react-query": "^5.90.2", | ||
| "@tanstack/react-query-devtools": "^5.90.2", | ||
| "axios": "^1.12.2", |
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.
| params: Promise<{ | ||
| albumId: string; | ||
| }>; | ||
| } | ||
|
|
||
| export default async function Page({ params }: PageProps) { | ||
| const { albumId } = await params; |
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.
| <body | ||
| className={`${pretendard.className} antialiased`} | ||
| style={{ | ||
| maxWidth: '430px', | ||
| width: '100vw', | ||
| margin: '0 auto', | ||
| background: '#fff', | ||
| }} | ||
| > |
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.
<body> 태그에 레이아웃 관련 스타일이 인라인으로 적용되어 있습니다. 유지보수성과 코드 가독성을 위해 이 스타일들을 globals.css 파일로 옮겨 클래스로 관리하는 것이 좋습니다.
예를 들어, globals.css에 아래와 같이 클래스를 정의하고,
.app-layout {
max-width: 430px;
width: 100vw;
margin: 0 auto;
background: #fff;
}<body> 태그에 className으로 적용할 수 있습니다.
| <body | |
| className={`${pretendard.className} antialiased`} | |
| style={{ | |
| maxWidth: '430px', | |
| width: '100vw', | |
| margin: '0 auto', | |
| background: '#fff', | |
| }} | |
| > | |
| <body | |
| className={`${pretendard.className} antialiased app-layout`} | |
| > |
src/app/oauth/callback/route.ts
Outdated
| request.headers.get('host') || | ||
| 'localhost:3000'; | ||
|
|
||
| const redirectPath = data.result.isOnboarded ? '/main' : '/create-album'; |
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.
신규 사용자의 경우 (isOnboarded가 false) /create-album 페이지로 리디렉션하고 있습니다. 프로젝트에 /onboarding 페이지가 존재하는 것으로 보아, 신규 사용자는 온보딩 과정을 먼저 거치는 것이 의도된 흐름일 수 있습니다. 현재 로직이 의도된 동작이 맞는지 확인해보시고, 만약 온보딩 페이지로 보내는 것이 맞다면 리디렉션 경로를 /onboarding으로 수정하는 것을 고려해보세요.
| const redirectPath = data.result.isOnboarded ? '/main' : '/create-album'; | |
| const redirectPath = data.result.isOnboarded ? '/main' : '/onboarding'; |
| function Drawer({ | ||
| ...props | ||
| }: React.ComponentProps<typeof DrawerPrimitive.Root>) { | ||
| useEffect(() => { | ||
| const cleanup = () => { | ||
| document.body.removeAttribute('data-scroll-locked'); | ||
| document.body.style.paddingLeft = ''; | ||
| document.body.style.paddingRight = ''; | ||
| document.body.style.marginLeft = 'auto'; | ||
| document.body.style.marginRight = 'auto'; | ||
| document.body.style.overflow = ''; | ||
| }; | ||
|
|
||
| const observer = new MutationObserver(() => { | ||
| if (document.body.hasAttribute('data-scroll-locked')) { | ||
| cleanup(); | ||
| } | ||
| }); | ||
|
|
||
| observer.observe(document.body, { attributes: true }); | ||
|
|
||
| return () => { | ||
| observer.disconnect(); | ||
| cleanup(); | ||
| }; | ||
| }, []); | ||
| return <DrawerPrimitive.Root data-slot='drawer' {...props} />; | ||
| } |
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.
useEffect와 MutationObserver를 사용하여 vaul 라이브러리가 추가하는 스크롤 잠금 관련 속성들을 강제로 제거하고 있습니다. 이는 라이브러리의 기본 동작을 우회하는 핵(hack)적인 방법으로, 예기치 않은 부작용을 일으킬 수 있습니다. vaul 라이브러리는 스크롤 방지 동작을 제어하는 preventScroll prop을 제공합니다. 이 prop을 DrawerPrimitive.Root에 전달하여 스크롤 방지 기능을 비활성화하는 것이 더 안정적인 방법입니다. useEffect 훅을 제거하고 아래와 같이 수정하는 것을 권장합니다.
function Drawer({
...props
}: React.ComponentProps<typeof DrawerPrimitive.Root>) {
return <DrawerPrimitive.Root data-slot='drawer' preventScroll={false} {...props} />;
}
| ], | ||
| rules: { | ||
| '@typescript-eslint/no-empty-object-type': 'off', // 빈 타입 객체 허용 | ||
| '@next/next/no-img-element': 'off', |
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.
| const parts = ref.split('.'); | ||
| if (parts.length > 0) parts[0] = parts[0].trim(); |
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.
토큰 참조 값을 파싱할 때 ref.split('.').map(p => p.trim())과 같이 각 부분을 trim()하는 것이 더 안전합니다. 현재 코드는 첫 번째 부분만 trim()하고 있어 Color . Blue . 500과 같이 중간에 공백이 있는 참조를 올바르게 처리하지 못할 수 있습니다.
| const parts = ref.split('.'); | |
| if (parts.length > 0) parts[0] = parts[0].trim(); | |
| const parts = ref.split('.').map(p => p.trim()); |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
feat : 앨범 진입 페이지 api 추가 연동
feat: 앨범 생성 lighthouse 개선
fix : 사진상세 하단 swiper가 pc에서도 잘 뜨게 수정
Revert "fix : 사진상세 하단 swiper가 pc에서도 잘 뜨게 수정"
fix : 시연 테스트중 발견한 이슈수정
fix: 촬영 시각은 받은 시간 그대로 변환
refactor: lighthouse 성능개선
fix : next,react 보안문제 해결을 위해 안정적 패치 버전으로 변경
[7팀] {치이이즈} 프론트엔드 코드리뷰
✨ 리뷰를 요청드리는 주요 부분
가장 고민되는 지점을 아래 형식으로 정리했습니다.
(zustand에 이미지 저장 후 화면에 불러오기 )
src\feature\create-album\utils\saveFilesToStore.tssrc\app\album\[albumId]\select\page.tsxsrc\feature\album-select\components\SelectAlbumBody.tsx( ncp에서 이미지 불러오기 )
src\app\album\detail\[albumId]\page.tsx💬 치이이즈팀이 고민하고 있는 부분
여기서 zustand에 저장된 이미지가 많아질 경우에 이 이미지들을 사용자에게 blob형태로 한번에 보여줄때 어떻게 최적화 시킬 수가 있을지가 궁금합니다.
💌 요청드리는 리뷰 방향
a. 가장 고민이었던 ConfirmModal (혹은, 이 링크) 컴포넌트에서,
b. 디자인 주입하는 상태 (
cancelClassName,confirmClassName) 로 커스텀하는것이 맞을지, 혹은 디자인을 변경할것이라면 컴포넌트를 아예 새로만드는것이 맞았는지c. 재사용성과 책임 분리가 괜찮은 방향인지 피드백 해주시면 감사하겠습니다.
바쁘신 와중에 시간 내어 리뷰해주셔서 감사합니다.
편하게 피드백 부탁드립니다! 🥹