Conversation
Summary of ChangesHello @gyogyo05, 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
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.
Code Review
이 PR은 피드백 슬라이드의 모바일 뷰에서 발생하는 레이아웃 깨짐 현상을 수정하는 것을 목표로 하고 있습니다. 전반적으로 ReactionButtons 컴포넌트의 스타일링을 개선하고 FeedbackSlidePage의 모바일 레이아웃 관련 CSS 클래스를 수정하여 문제를 해결한 것으로 보입니다. 코드 가독성을 높이기 위한 리팩토링도 일부 포함되어 있습니다. 몇 가지 코드 중복 및 오타로 보이는 부분을 발견하여 수정을 제안합니다. 또한, 탭 UI의 클래스네임 생성 로직을 함수로 추출하여 중복을 제거하고 유지보수성을 높이는 방안을 제안합니다.
I am having trouble creating individual review comments. Click here to see my feedback.
src/components/feedback/ReactionButtons.tsx (55-61)
baseBtn 변수에 이미 포함된 클래스들이 className 문자열에 중복으로 사용되고 있습니다. 코드 가독성과 유지보수성을 위해 중복된 클래스들을 제거하는 것이 좋습니다.
className={`${baseBtn} ${widthClass} ${buttonClassName ?? ''} ${
isLastOdd ? 'col-span-2 justify-self-start' : ''
} ${
reaction.active
? 'bg-gray-900 border-main-variant1 text-main-variant2 text-body-m-bold'
: 'bg-gray-200 border-gray-400 text-black hover:border-gray-600'
}`}
src/components/feedback/ReactionButtons.tsx (69)
font-body-m 클래스명은 typography.css에 정의되어 있지 않아 오타로 보입니다. 의도하신 것이 text-body-m (일반 굵기) 이라면 수정이 필요합니다. 버튼이 활성화 상태일 때 부모 요소에 text-body-m-bold가 적용되는데, 카운트 숫자는 일반 굵기를 유지하려는 의도였다면 text-body-m으로 수정하는 것이 맞을 것 같습니다.
className={`tabular-nums text-right min-w-0 ${reaction.active ? 'text-body-m' : ''}`}
src/pages/FeedbackSlidePage.tsx (210-212)
이 버튼의 className을 결정하는 로직이 '댓글' 탭 버튼(222-226행)에서도 거의 동일하게 반복되고 있습니다. 코드 중복을 줄이고 유지보수성을 높이기 위해 이 로직을 헬퍼 함수로 추출하는 것을 고려해 보세요. 예를 들어, 다음과 같은 함수를 사용할 수 있습니다.
const getTabClassName = (isActive: boolean) =>
`flex-1 py-3 text-body-m-bold transition-colors ${
isActive ? 'text-main border-b-2 border-main-variant1' : 'text-gray-600'
}`;그리고 각 버튼에서 className={getTabClassName(mobileTab === 'script')} 와 같이 사용할 수 있습니다.
References
- Extract complex or repetitive conditional
classNamelogic into a helper function, using utilities likeclsxto improve readability and maintainability.
|
@claude 코드 리뷰해줘 |
|
Claude finished @gyogyo05's task in 1m 44s —— View job 코드 리뷰 완료 ✅모바일 레이아웃 개선 작업 잘 진행하셨습니다! 전반적으로 깔끔한 수정이지만, 몇 가지 개선할 수 있는 부분을 발견했습니다. 주요 개선 사항✅ 잘된 점
🔍 발견된 이슈1. 🐛 존재하지 않는 CSS 클래스 사용 (높은 우선순위)위치: className={`tabular-nums text-right min-w-0 ${reaction.active ? 'font-semibold' : ''}`}문제:
권장 수정: className={`tabular-nums text-right min-w-0`}부모에서 이미 bold 처리되므로 자식에서 별도 font-weight 지정 불필요 2.
|
| 파일 | 주요 변경 | 영향도 |
|---|---|---|
| ReactionButtons.tsx | gap, padding 축소 / 변수 추출 | 중간 |
| FeedbackSlidePage.tsx | 모바일 스크롤 개선 / 탭 리팩토링 | 높음 |
| Router.tsx | 헤더 컴포넌트화 | 낮음 |
✅ 최종 의견
모바일 레이아웃 개선 목표는 잘 달성하셨습니다! 위의 이슈 중:
- **이슈 chore: 프로젝트 세팅 #1 (font-semibold)**은 수정 권장
- **이슈 chore/프로젝트 세팅 #2 (justify-between)**는 실제 디자인 의도에 따라 선택
- 나머지는 참고사항
특히 스크롤 레이아웃(overflow-y-auto, min-h-0) 개선은 모바일 UX에 큰 도움이 될 것 같습니다. 고생하셨습니다! 👍
|
@claude 코드 리뷰해줘 |
|
Claude finished @AndyH0ng's task in 1m 57s —— View job 코드 리뷰 완료 ✅모바일 레이아웃 개선 작업 수고하셨습니다! 공통 레이아웃 컴포넌트 추출과 스크롤 구조 개선이 잘 이루어졌습니다. 다만 중요한 버그 1개와 개선 사항들을 발견했습니다. 🚨 Critical Issue - 스크롤 버그 발견1. feedback/video 모바일에서 대본 섹션 스크롤 버그 (높은 우선순위)위치: 문제: 현재 구조: scriptTabContent={
<div className="px-4 py-4"> {/* ❌ 이 wrapper가 문제! */}
<ScriptSection
slides={projectSlides}
slideChangeTimes={slideChangeTimes}
currentTime={currentTime}
onSeek={requestSeek}
/>
</div>
}
className="flex-1 min-w-0 rounded-lg p-4 overflow-y-auto flex flex-col gap-2 bg-gray-100 focus:outline-none"이중 래퍼 구조 때문에:
해결 방법: FeedbackSlidePage.tsx (올바른 패턴): scriptTabContent={
<div className="px-4 py-4">
<SlideTitle fallbackTitle={`슬라이드 ${slideIndex + 1}`} readOnly />
<div className="mt-3 bg-gray-200 rounded-lg px-4 py-3 h-48 overflow-y-auto">
{/* 스크립트 콘텐츠 */}
</div>
</div>
}여기서는 권장 수정: 옵션 1 - ScriptSection을 wrapper 없이 직접 사용 (추천) scriptTabContent={
<ScriptSection
slides={projectSlides}
slideChangeTimes={slideChangeTimes}
currentTime={currentTime}
onSeek={requestSeek}
/>
}
옵션 2 - ScriptSection의 패딩 제거 후 wrapper에서 처리
|
| 이슈 | 우선순위 | 위치 | 상태 |
|---|---|---|---|
| Video 대본 스크롤 버그 | 🚨 높음 | FeedbackVideoPage.tsx:106-114 | 수정 필요 |
| 중복 className | ReactionButtons.tsx:77-79 | 수정 권장 | |
| 활성화 색상 불일치 | 📝 낮음 | ReactionButtons.tsx:58-60 | 확인 필요 |
다음 단계:
- 가장 먼저 이슈 chore: 프로젝트 세팅 #1 (스크롤 버그) 수정 - 사용자 경험에 직접적 영향
- 이슈 #2는 코드 품질 개선 차원에서 수정 권장
- 이슈 #3은 디자인 의도 확인 후 결정
전반적으로 좋은 리팩토링이었습니다! 스크롤 버그만 수정하면 완벽할 것 같습니다. 👍
|
해당 문제 수정하고 머지합니다. |

📌 관련 이슈
✨ 변경 내용
💡 참고 사항