Conversation
- 중복 파일 삭제 (src/components/ScriptBox.tsx) - ScriptBoxEmogi → ScriptBoxEmoji 리네임 - treshcanIcon → trashcanIcon 리네임 - props 네이밍 개선 (isEmogiClick → isEmojiOpen) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- 외부 클릭 시 닫기 - Escape 키로 닫기 - position (top/bottom), align (start/end) 지원 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- ScriptBox를 Header와 Content로 분리 - Header에서 모든 팝오버 상태를 자체 관리하도록 변경 - props drilling 최소화 - 불필요한 slideId prop 제거 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- SlideTitle, ScriptBoxEmoji, ScriptHistory, Opinion 컴포넌트 리팩토링 - 공통 Popover 컴포넌트 사용으로 중복 코드 제거 - 각 컴포넌트가 자체 상태를 관리하도록 변경 - ScriptBoxHeader에서 상태 관리 로직 제거 (props drilling 해소) - 데이터 구조를 인터페이스로 명시화 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
피그마 디자인 분석 후 색상 값 정렬: - gray-500 → gray-600 (보조 텍스트: 시간, 카운트) - gray-300 → gray-400 (아바타 플레이스홀더) - red-500 → error (삭제 버튼) - placeholder 색상을 gray-600으로 통일 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Popover 컴포넌트: - aria-haspopup, aria-expanded, aria-controls 추가 - role="dialog"와 aria-label 속성 지원 - Escape 키로 닫을 때 트리거로 포커스 복귀 - 팝오버 열릴 때 첫 번째 포커스 가능 요소로 포커스 이동 각 컴포넌트: - 모든 버튼에 aria-label 추가 - 아이콘에 aria-hidden="true" 추가 - 입력 필드에 aria-label 추가 - 토글 버튼에 aria-expanded 추가 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Tailwind v4에서 outline-1이 이미 outline-style: solid를 포함하므로 중복 outline 클래스 제거 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- vite-plugin-svgr 설치 및 설정 - SVG 아이콘을 React 컴포넌트로 임포트하도록 변경 - TypeScript 타입 지원 추가 (vite-plugin-svgr/client) - img 태그 대신 SVG 컴포넌트 사용으로 스타일링 용이 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Summary of ChangesHello @AndyH0ng, 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는 스크립트 뷰어 컴포넌트의 전반적인 구조를 개선하고, 재사용 가능한 UI 요소를 도입하며, 개발 환경 설정을 최적화하는 데 중점을 둡니다. 주요 변경 사항으로는 ScriptBox의 모듈화, 접근성을 고려한 Popover 컴포넌트 추가, SVGR을 통한 SVG 아이콘 관리 효율화, 그리고 타입 정의 확장을 포함합니다. 이러한 변경은 코드의 유지보수성을 높이고, 일관된 디자인을 적용하며, 개발 생산성을 향상시키는 데 기여합니다. 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
|
![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
전반적으로 훌륭한 리팩토링입니다. ScriptBox 컴포넌트를 기능별로 작은 컴포넌트로 분리하여 코드의 응집도를 높이고 재사용성을 개선한 점이 인상적입니다. 특히, 접근성을 고려한 공용 Popover 컴포넌트 도입과 vite-plugin-svgr을 활용한 SVG 아이콘 관리 방식 개선은 프로젝트의 유지보수성과 확장성을 크게 향상시킬 것입니다.
몇 가지 추가적인 개선점을 제안합니다. Popover 컴포넌트의 접근성을 더욱 향상시키기 위한 방안과, Tailwind CSS 클래스에서 사용된 매직 넘버를 디자인 시스템 토큰으로 관리하여 일관성을 높이는 방법에 대한 의견을 남겼습니다. 해당 내용들을 검토하고 반영해주시면 코드가 더욱 견고해질 것입니다.
| const [isOpen, setIsOpen] = useState(false); | ||
| const popoverRef = useRef<HTMLDivElement>(null); | ||
| const triggerRef = useRef<HTMLDivElement>(null); | ||
| const contentRef = useRef<HTMLDivElement>(null); | ||
| const popoverId = useId(); | ||
|
|
||
| const handleToggle = useCallback(() => { | ||
| setIsOpen((prev) => !prev); | ||
| }, []); | ||
|
|
||
| const handleClose = useCallback(() => { | ||
| setIsOpen(false); | ||
| // 팝오버 닫힐 때 트리거로 포커스 이동 | ||
| triggerRef.current?.querySelector('button')?.focus(); | ||
| }, []); |
There was a problem hiding this comment.
팝오버가 닫힐 때 포커스를 복원하는 방식이 특정 구조(button 태그)에 의존하고 있어 유연성이 떨어집니다. triggerRef.current?.querySelector('button') 대신, 팝오버가 열릴 때의 document.activeElement를 useRef에 저장해두었다가 닫힐 때 그 요소로 포커스를 되돌리는 것이 더 안정적이고 재사용성 높은 방법입니다.
const [isOpen, setIsOpen] = useState(false);
const popoverRef = useRef<HTMLDivElement>(null);
const triggerRef = useRef<HTMLDivElement>(null);
const contentRef = useRef<HTMLDivElement>(null);
const lastFocusedElement = useRef<HTMLElement | null>(null);
const popoverId = useId();
const handleToggle = useCallback(() => {
setIsOpen((prev) => {
if (!prev) {
lastFocusedElement.current = document.activeElement as HTMLElement;
}
return !prev;
});
}, []);
const handleClose = useCallback(() => {
setIsOpen(false);
// 팝오버 닫힐 때 이전에 포커스된 요소로 포커스 이동
lastFocusedElement.current?.focus();
}, []);
| <div | ||
| ref={triggerRef} | ||
| onClick={handleToggle} | ||
| aria-haspopup="dialog" | ||
| aria-expanded={isOpen} | ||
| aria-controls={isOpen ? popoverId : undefined} | ||
| > | ||
| {typeof trigger === 'function' ? trigger({ isOpen }) : trigger} | ||
| </div> |
There was a problem hiding this comment.
접근성 관점에서 div와 같은 정적(non-interactive) 요소에 onClick 핸들러를 직접 사용하는 것은 좋지 않습니다. 스크린 리더 사용자와 키보드 사용자가 해당 요소를 인지하고 상호작용하기 어렵기 때문입니다. 또한, aria-* 속성들은 실제 상호작용하는 요소(이 경우 trigger 내부의 button)에 직접 적용되어야 더 의미론적으로 올바릅니다.
이 문제를 해결하기 위해 div 래퍼를 제거하고, React.cloneElement를 사용하여 trigger로 전달된 요소에 onClick과 aria-* 속성들을 직접 주입하는 방식으로 리팩토링하는 것을 권장합니다. 이렇게 하면 Popover 컴포넌트의 유연성을 유지하면서도 시맨틱하고 접근성 높은 코드를 작성할 수 있습니다.
There was a problem hiding this comment.
Pull request overview
이 PR은 대본 입력 및 관리를 위한 ScriptBox 컴포넌트를 모듈화하고 재사용 가능한 Popover 컴포넌트를 도입하여 코드 구조를 개선했습니다.
주요 변경사항:
- ScriptBox 컴포넌트를 ScriptBoxHeader, ScriptBoxContent, ScriptBoxEmoji, ScriptHistory, SlideTitle, Opinion 등 세부 컴포넌트로 분리하여 단일 책임 원칙을 준수
- 접근성(ARIA)과 사용자 경험(Escape 키, 외부 클릭 처리)을 고려한 공통 Popover 컴포넌트 구현
- vite-plugin-svgr 도입으로 SVG 아이콘을 React 컴포넌트로 직접 import하여
currentColor사용 가능
Reviewed changes
Copilot reviewed 19 out of 25 changed files in this pull request and generated 14 comments.
Show a summary per file
| File | Description |
|---|---|
| vite.config.ts | SVGR 플러그인 추가 |
| tsconfig.app.json | vite-plugin-svgr 클라이언트 타입 정의 추가 |
| src/types/script.ts | HistoryItem, OpinionItem 인터페이스 정의 |
| src/types/navigation.ts | TabKey, TabItem 타입 정의 |
| src/components/common/Popover.tsx | 재사용 가능한 Popover 컴포넌트 구현 |
| src/components/common/index.ts | Popover export 추가 |
| src/components/script-box/ScriptBox.tsx | 메인 컴포넌트 리팩토링 및 단순화 |
| src/components/script-box/ScriptBoxHeader.tsx | 헤더 영역 분리 |
| src/components/script-box/ScriptBoxContent.tsx | 대본 입력 영역 분리 |
| src/components/script-box/ScriptBoxEmoji.tsx | 이모지 표시 및 팝오버 로직 분리 (파일명 수정) |
| src/components/script-box/ScriptHistory.tsx | 변경 기록 팝오버 개선 |
| src/components/script-box/SlideTitle.tsx | 슬라이드 제목 편집 로직 개선 |
| src/components/script-box/Opinion.tsx | 의견 표시 및 답글 기능 개선 |
| src/components/script-box/index.ts | 배럴 export 추가 |
| src/assets/icons/*.svg | SVG stroke 속성을 currentColor로 변경 |
| src/assets/icons/trashcanIcon.svg | 파일명 오타 수정 (treshcan → trashcan) |
| src/pages/SlidePage.tsx | ScriptBox 컴포넌트 통합 |
| src/constants/navigation.ts | TabItem 타입 적용 |
| package.json | vite-plugin-svgr 의존성 추가 |
| // 팝오버 열릴 때 첫 번째 포커스 가능한 요소로 포커스 이동 | ||
| useEffect(() => { | ||
| if (!isOpen || !contentRef.current) return; | ||
|
|
||
| const focusableElements = contentRef.current.querySelectorAll<HTMLElement>( | ||
| 'button, [href], input, select, textarea, [tabindex]:not([tabindex="-1"])', | ||
| ); | ||
|
|
||
| if (focusableElements.length > 0) { | ||
| focusableElements[0].focus(); | ||
| } | ||
| }, [isOpen]); |
There was a problem hiding this comment.
Popover가 열릴 때 자동으로 첫 번째 포커스 가능한 요소로 포커스를 이동시키는데, 이것이 모든 상황에 적합하지 않을 수 있습니다. 예를 들어 ScriptBoxEmoji 팝오버는 단순히 정보를 표시하는 용도이므로 자동 포커스가 불필요할 수 있습니다. autoFocus prop을 추가하여 이 동작을 선택적으로 제어할 수 있도록 하는 것을 권장합니다.
| <div | ||
| className={clsx( | ||
| 'flex gap-3 py-3 pr-4', | ||
| opinion.isReply ? 'pl-15' : 'pl-4', |
There was a problem hiding this comment.
Tailwind CSS v4에서 pl-15는 기본 제공되지 않는 값입니다. 다만, src/styles/index.css에서 --spacing-15: 3.75rem으로 정의되어 있으므로 이 값은 작동할 것입니다. 일관성을 위해 다른 커스텀 spacing 값들도 동일하게 정의하는 것을 권장합니다.
| opinion.isReply ? 'pl-15' : 'pl-4', | |
| opinion.isReply ? 'pl-[var(--spacing-15)]' : 'pl-4', |
| const handleSave = () => { | ||
| setTitle(editTitle); | ||
| onSave?.(editTitle); |
There was a problem hiding this comment.
SlideTitle의 Popover가 닫힐 때 editTitle 상태가 초기화되지 않습니다. 사용자가 입력을 시작한 후 저장하지 않고 팝오버를 닫으면, 다음에 팝오버를 열었을 때 이전에 입력했던 미저장 내용이 남아있게 됩니다. Popover가 닫힐 때 editTitle을 title로 리셋하는 로직을 추가하는 것을 권장합니다.
| <div className="flex flex-col gap-1"> | ||
| <div className="flex items-center justify-between"> | ||
| <div className="flex items-center gap-2"> | ||
| <span className="max-w-50 truncate text-sm font-semibold text-gray-800"> |
There was a problem hiding this comment.
Tailwind CSS v4에서 max-w-50는 기본 제공되지 않는 값입니다. src/styles/index.css의 @theme 블록에 커스텀 spacing으로 정의하거나, 픽셀 값을 직접 사용하는 것을 권장합니다 (예: max-w-[12.5rem]).
| <span className="max-w-50 truncate text-sm font-semibold text-gray-800"> | |
| <span className="max-w-[12.5rem] truncate text-sm font-semibold text-gray-800"> |
| role="dialog" | ||
| aria-label={ariaLabel} | ||
| aria-modal="false" |
There was a problem hiding this comment.
Popover 컴포넌트가 role="dialog"와 aria-modal="false"를 사용하고 있습니다. 하지만 popover는 일반적으로 dialog가 아니라 보조 컨텐츠를 표시하는 용도입니다. ARIA 1.2부터 지원되는 role="menu" (메뉴형 팝오버인 경우) 또는 일반 컨텐츠 컨테이너로 처리하는 것이 더 적절할 수 있습니다. 또는 트리거 버튼에 aria-haspopup="true" 대신 구체적인 값 (예: aria-haspopup="menu")을 사용하는 것을 권장합니다.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
와우 리펙토링 감사합니다!! 고생 많으셨습니다. |
📌 관련 이슈
✨ 변경 내용
ScriptBox컴포넌트 고도화: 대본 입력 및 관리를 위한ScriptBox구조를 리팩토링하고,ScriptBoxHeader,ScriptBoxContent,ScriptBoxEmoji,ScriptHistory,Opinion,SlideTitle등으로 세분화하여 책임을 분산했습니다.Popover컴포넌트 도입: 접근성(ARIA)과 사용자 경험(Escape 키 처리, 외부 클릭 닫기 등)을 고려한 범용 팝오버 컴포넌트를 구현하여ScriptHistory,Opinion,SlideTitle등에서 공통으로 사용하도록 개선했습니다.vite-plugin-svgr을 도입하여 SVG 파일을 React 컴포넌트처럼 직접 import 하여 스타일링과 접근성 제어가 용이하도록 설정했습니다.src/types/script.ts와src/types/navigation.ts를 통해HistoryItem,OpinionItem,TabItem등 데이터 모델에 대한 명확한 인터페이스를 정의했습니다.💡 참고 사항
스크린샷, 참고 링크, 추가 설명 등 (없으면 생략 가능)