Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
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! 이 PR은 애플리케이션의 랜딩 페이지를 전면적으로 개편하여 사용자 경험을 향상시키는 것을 목표로 합니다. 새로운 랜딩 페이지는 스플래시 화면, 앱 기능들을 소개하는 캐러셀, 그리고 카카오 로그인 기능을 포함하며, 모바일 환경에 최적화된 반응형 디자인을 적용했습니다. 또한, 인증 확인 로직의 에러 처리 부분을 개선하여 안정성을 높였습니다. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
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
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
이 PR은 기존 랜딩 페이지를 새로운 디자인의 랜딩 페이지로 교체하는 작업을 포함하고 있습니다. 전반적으로 새로운 기능들이 잘 구현되었지만, 몇 가지 개선점과 잠재적인 버그가 보입니다.
주요 피드백 사항은 다음과 같습니다:
- 인증된 사용자가 메인 페이지로 리디렉션되지 않는 심각한 버그가 있습니다.
- 하드코딩된 URL과 색상 값들이 있어 유지보수성을 저해할 수 있습니다.
- 일부 컴포넌트에서 메모리 누수 가능성이 있고, 접근성 및 성능 개선의 여지가 있습니다.
- 코드에 남아있는
console.log문구들은 제거해야 합니다.
자세한 내용은 각 파일의 리뷰 코멘트를 참고해주세요.
| const ScreenNewRoot = () => { | ||
| const { isAuthed } = useCheckAuth(); | ||
| const [showSplash, setShowSplash] = useState(true); |
There was a problem hiding this comment.
로그인된 사용자가 메인 페이지로 리디렉션되지 않는 문제가 있습니다. useCheckAuth 훅이 onAuthed 콜백 없이 호출되어, 인증된 사용자가 랜딩 페이지를 보게 됩니다. 이전 ScreenRoot 컴포넌트처럼 onAuthed 콜백을 사용하여 /main으로 리디렉션하는 로직을 추가해야 합니다.
아래와 같이 수정하고, 파일 상단에 import { useRouter } from 'next/navigation';을 추가해주세요.
| const ScreenNewRoot = () => { | |
| const { isAuthed } = useCheckAuth(); | |
| const [showSplash, setShowSplash] = useState(true); | |
| const ScreenNewRoot = () => { | |
| const router = useRouter(); | |
| const { isAuthed } = useCheckAuth({ onAuthed: () => router.push('/main') }); | |
| const [showSplash, setShowSplash] = useState(true); |
| import { useSearchParams } from 'next/navigation'; | ||
| import { useEffect, useState } from 'react'; | ||
|
|
||
| const KAKAO_AUTH_URL = `https://dev.say-cheese.me/oauth2/authorization/kakao`; |
There was a problem hiding this comment.
개발 환경의 URL이 하드코딩되어 있습니다. 배포 환경(production, development 등)에 따라 동적으로 URL을 설정할 수 있도록 .env 파일과 같은 환경 변수를 사용하는 것이 좋습니다. 이렇게 하면 환경별로 코드를 수정할 필요가 없어지고, 보안에도 더 안전합니다.
| const KAKAO_AUTH_URL = `https://dev.say-cheese.me/oauth2/authorization/kakao`; | |
| const KAKAO_AUTH_URL = `${process.env.NEXT_PUBLIC_API_URL}/oauth2/authorization/kakao`; |
| useEffect(() => { | ||
| if (!api) return; | ||
|
|
||
| api.on('select', () => { | ||
| setCurrent(api.selectedScrollSnap()); | ||
| }); | ||
| }, [api]); |
There was a problem hiding this comment.
useEffect에서 api.on('select', ...)로 이벤트 리스너를 등록한 후, 컴포넌트가 언마운트될 때 이를 정리(clean up)하는 로직이 없습니다. 이는 메모리 누수를 유발할 수 있습니다. useEffect의 반환값으로 cleanup 함수를 추가하여 이벤트 리스너를 제거해주세요.
useEffect(() => {
if (!api) return;
const onSelect = () => {
setCurrent(api.selectedScrollSnap());
};
api.on('select', onSelect);
return () => {
api.off('select', onSelect);
};
}, [api]);
| <div | ||
| className='mt-auto flex h-[56px] w-full cursor-pointer items-center justify-center gap-2 rounded-[6px] bg-[#FEE500]' | ||
| onClick={handleKakaoLogin} | ||
| > |
There was a problem hiding this comment.
클릭 가능한 카카오 로그인 영역이 div 태그로 구현되어 있습니다. 스크린 리더 사용자나 키보드만으로 네비게이션하는 사용자를 위해 웹 접근성을 준수하는 것이 중요합니다. div 대신 시맨틱한 button 태그를 사용하고, type='button' 속성을 추가하는 것을 권장합니다. button으로 변경 후 className에서 cursor-pointer는 제거해도 됩니다.
<button
type='button'
className='mt-auto flex h-[56px] w-full items-center justify-center gap-2 rounded-[6px] bg-[#FEE500]'
onClick={handleKakaoLogin}
>
| width: 'device-width', | ||
| initialScale: 1, | ||
| maximumScale: 1, | ||
| userScalable: false, |
There was a problem hiding this comment.
뷰포트 설정에서 userScalable: false를 사용하면 사용자가 페이지를 확대/축소할 수 없게 됩니다. 이는 접근성 가이드라인(WCAG 2.1 - 1.4.4 Resize text)에 위배될 수 있으며, 저시력 사용자에게 불편을 초래할 수 있습니다. 꼭 필요한 경우가 아니라면 이 설정을 제거하거나 true로 변경하는 것을 고려해주세요.
| userScalable: false, | |
| userScalable: true, |
| const handleKakaoLogin = async () => { | ||
| try { | ||
| const kakaoUrl = redirect | ||
| ? `${KAKAO_AUTH_URL}${buildQuery({ redirect })}` | ||
| : KAKAO_AUTH_URL; | ||
|
|
||
| window.location.href = kakaoUrl; | ||
| } catch (err) { | ||
| console.error('카카오 인증 GET 요청 실패:', err); | ||
| } | ||
| }; |
There was a problem hiding this comment.
handleKakaoLogin 함수가 async로 선언되었지만 await 키워드를 사용하지 않습니다. 비동기 작업이 없으므로 async 키워드를 제거하는 것이 좋습니다. 또한, window.location.href 할당은 일반적으로 예외를 발생시키지 않으므로 try...catch 블록도 제거할 수 있습니다.
const handleKakaoLogin = () => {
const kakaoUrl = redirect
? `${KAKAO_AUTH_URL}${buildQuery({ redirect })}`
: KAKAO_AUTH_URL;
window.location.href = kakaoUrl;
};
| <div className='absolute bottom-0 z-10 flex h-[242px] w-full flex-col items-center bg-white px-4 pt-8 pb-5'> | ||
| {/* 텍스트 영역 */} | ||
| <div className='mb-6 flex flex-col items-center text-center'> | ||
| <h2 className='text-text-basic text-[24px] font-[600]'> | ||
| {slides[current].title} | ||
| </h2> | ||
| <p className='mt-1 text-[16px] font-[500] text-[#746181]'> | ||
| {slides[current].description} | ||
| </p> | ||
| </div> | ||
|
|
||
| {/* 인디케이터 점들 */} | ||
| <div className='flex gap-2'> | ||
| {slides.map((_, index) => ( | ||
| <button | ||
| key={index} | ||
| onClick={() => api?.scrollTo(index)} | ||
| className={`h-1.5 rounded-full transition-all ${ | ||
| current === index ? 'w-6 bg-gray-700' : 'w-1.5 bg-gray-200' | ||
| }`} | ||
| aria-label={`슬라이드 ${index + 1}로 이동`} | ||
| /> | ||
| ))} | ||
| </div> | ||
|
|
||
| {/* 카카오 로그인 버튼 */} | ||
| <div | ||
| className='mt-auto flex h-[56px] w-full cursor-pointer items-center justify-center gap-2 rounded-[6px] bg-[#FEE500]' | ||
| onClick={handleKakaoLogin} | ||
| > | ||
| <Image | ||
| src='/assets/login/kakao-logo.svg' | ||
| width={18} | ||
| height={18} | ||
| alt='카카오 로고' | ||
| /> | ||
| <span className='text-[15px] font-[600] text-[rgba(0,0,0,0.85)]'> | ||
| 카카오 로그인 | ||
| </span> | ||
| </div> | ||
| </div> | ||
| </section> |
There was a problem hiding this comment.
이 영역 전반에 걸쳐 하드코딩된 색상 값(text-[#746181], bg-[#FEE500], text-[rgba(0,0,0,0.85)])이 사용되고 있습니다. 프로젝트의 다른 부분에서는 bg-background-white-muted와 같이 Tailwind CSS 테마에 정의된 색상을 사용하고 있습니다. 일관성 있는 디자인 시스템을 유지하고 유지보수성을 높이기 위해, 이 색상들도 Tailwind 설정 파일에 추가하고 시맨틱한 이름으로 사용하는 것을 권장합니다.
예를 들어,
text-[#746181](116행) ->text-text-mutedbg-[#FEE500](137행) ->bg-kakao(카카오 브랜드 색상이므로 명확한 이름 사용)text-[rgba(0,0,0,0.85)](146행) ->text-black/85(Tailwind의 opacity modifier 사용)
| const { isAuthed } = useCheckAuth(); | ||
| const [showSplash, setShowSplash] = useState(true); | ||
|
|
||
| console.log('ScreenNewRoot - isAuthed:', isAuthed, 'showSplash:', showSplash); |
There was a problem hiding this comment.
디버깅 목적으로 사용된 console.log 문이 남아있습니다. 프로덕션 코드에 포함되지 않도록 병합 전에 제거해주세요.
| } | ||
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
| } catch (err: any) { | ||
| console.log('err 타입', err); |
There was a problem hiding this comment.
디버깅 목적으로 사용된 console.log 문이 남아있습니다. 프로덕션 코드에 포함되지 않도록 병합 전에 제거해주세요.
| if ( | ||
| status === 401 || | ||
| code === 401 || | ||
| err?.message?.includes('No refresh token') |
There was a problem hiding this comment.
에러 메시지 문자열('No refresh token')에 의존하여 에러를 처리하는 방식은 불안정할 수 있습니다. 라이브러리나 API의 에러 메시지가 변경되면 이 코드가 동작하지 않게 됩니다. 가능하면 에러 코드나 에러 타입을 통해 분기 처리를 하는 것이 더 안정적인 방법입니다.
1 participant
요약
구현 사항
📸 스크린샷
Need Review
Reference
📜 리뷰 규칙
Reviewer는 아래 P5 Rule을 참고하여 리뷰를 진행합니다.
P5 Rule을 통해 Reviewer는 Reviewee에게 리뷰의 의도를 보다 정확히 전달할 수 있습니다.