[FEAT] 신 기능 소개 팝업 구현 #411
[FEAT] 신 기능 소개 팝업 구현 #411
Conversation
Summary of ChangesHello @i-meant-to-be, 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은 사용자에게 새로운 기능을 효과적으로 알리기 위한 모달 시스템을 도입합니다. 새로운 기능 데이터의 구조를 정의하고, 해당 데이터를 기반으로 모달 UI를 렌더링하며, 사용자가 모달을 일정 기간 동안 보지 않도록 설정할 수 있는 로직을 포함합니다. 이는 사용자 경험을 개선하고 중요한 업데이트를 놓치지 않도록 돕는 것을 목표로 합니다. 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
|
Walkthrough업데이트 모달(정적 SVG 자산, UpdateModal 컴포넌트·Wrapper, 스토리북, 패치노트 타입/상수)을 추가하고 라우터에 통합했으며, PR 자동 설정을 담당하던 GitHub Actions 워크플로우 파일을 삭제했습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant Browser as Browser
participant Router as Router
participant Wrapper as UpdateModalWrapper
participant Storage as localStorage
participant Modal as ModalWrapper
participant Update as UpdateModal
User->>Browser: 앱 접근
Browser->>Router: 라우트 렌더링
Router->>Wrapper: UpdateModalWrapper 마운트
Wrapper->>Storage: dismissal 데이터 조회
Storage-->>Wrapper: 저장값 또는 null
Wrapper->>Wrapper: 버전 비교 및 경과일(7일) 판단
alt 모달 열기 조건 충족
Wrapper->>Modal: open()
Modal->>Update: 렌더(PatchNoteData, isChecked, onChecked)
Update-->>User: 팝업 표시
User->>Update: "자세히 보기" 클릭
Update->>Browser: 링크 새 탭 열기
User->>Update: 체크박스 선택 및 닫기
Update->>Wrapper: onChecked(true)
Wrapper->>Storage: dismissal 저장(버전, now)
Wrapper->>Modal: close()
else 모달 미표시
Wrapper->>Wrapper: 아무 동작 없음
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![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은 신규 기능 소개를 위한 모달을 구현합니다. 전반적으로 잘 작성되었지만, 몇 가지 개선점을 제안합니다. 가장 중요한 점은 '다시 보지 않기' 기능이 패치 노트 버전과 연동되지 않아 새로운 업데이트가 있어도 사용자가 모달을 보지 못할 수 있다는 점입니다. 또한, Storybook 설정 오류와 타입 정의 불일치, 디버깅용 console.log 코드 등 수정이 필요한 부분들이 있습니다. 자세한 내용은 각 파일에 남긴 코멘트를 확인해주세요.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@src/components/UpdateModal/UpdateModal.stories.tsx`:
- Around line 14-19: The Story "Default" uses args that don't match
UpdateModalProps; update Default.args to supply the expected props: provide a
realistic "data" object matching the component's data shape, include "isChecked"
as a boolean, and add "onChecked" as a handler (e.g., a noop or action) so
TypeScript compiles; locate the Default export in UpdateModal.stories.tsx and
replace the current title/description-only args with an object containing data,
isChecked, and onChecked to match the UpdateModal component's props.
In `@src/components/UpdateModal/UpdateModalWrapper.tsx`:
- Around line 6-16: DISMISS_DATE_KEY is a fixed key so old dismiss records
persist across patch versions; update storage to be version-aware by namespacing
the key with the current LATEST_PATCH_NOTE.version (or store an object {version,
date}) and update getSafeDateFromStorage to read the versioned key (or parse the
stored object), returning null when the stored version differs or the date is
invalid; adjust any setters that write DISMISS_DATE_KEY accordingly so they
write the versioned key/object as well.
In `@src/constants/patch_note.ts`:
- Around line 4-9: The PatchNoteData interface currently declares image and link
as required strings but comments and UpdateModal usage treat them as optional;
update the interface (PatchNoteData) to make image and link optional (e.g.,
image?: string; link?: string) so the type matches actual usage and avoid
forcing callers to provide those properties; search for usages of PatchNoteData
and adjust any code that assumed required values to handle undefined if
necessary (especially in the UpdateModal consumer).
🧹 Nitpick comments (3)
src/components/UpdateModal/NoticeAsset.tsx (1)
7-13: 장식용 SVG 접근성 속성 추가 권장.
의미 없는 장식 요소라면 스크린리더 노이즈를 줄이기 위해aria-hidden/focusable를 추가하는 편이 좋습니다.♿ 제안 수정
<svg width="309" height="59" viewBox="0 0 309 59" fill="none" xmlns="http://www.w3.org/2000/svg" className={className} + aria-hidden="true" + focusable="false" >src/components/UpdateModal/MegaphoneAsset.tsx (1)
7-13: 장식용 SVG 접근성 속성 추가 권장.
아이콘이 장식 목적이라면 스크린리더 노이즈 방지를 위해aria-hidden/focusable추가가 좋습니다.♿ 제안 수정
<svg width="160" height="131" viewBox="0 0 160 131" fill="none" xmlns="http://www.w3.org/2000/svg" className={className} + aria-hidden="true" + focusable="false" >src/components/UpdateModal/UpdateModal.tsx (1)
57-65: 체크박스에 라벨을 연결해 접근성을 개선하세요.텍스트 클릭으로 토글되지 않고 스크린리더 문맥도 약합니다.
label연결과event.currentTarget.checked사용을 권장합니다.♿️ 제안 수정안
- <div className="flex w-full flex-row items-center space-x-[8px]"> - <input - type="checkbox" - className="border-gray size-[24px] rounded-[4px]" - checked={isChecked} - onChange={() => onChecked(!isChecked)} - /> - <p>일주일 간 보지 않기</p> - </div> + <label + htmlFor="update-modal-hide-week" + className="flex w-full flex-row items-center space-x-[8px]" + > + <input + id="update-modal-hide-week" + type="checkbox" + className="border-gray size-[24px] rounded-[4px]" + checked={isChecked} + onChange={(e) => onChecked(e.currentTarget.checked)} + /> + <span>일주일 간 보지 않기</span> + </label>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/components/UpdateModal/UpdateModalWrapper.tsx`:
- Around line 19-30: The onClose passed to useModal captures the initial
isChecked value (stale closure) so checkbox state never persists; fix by
tracking the latest checkbox state in a ref (e.g., create isCheckedRef and
update it whenever isChecked changes) and reference isCheckedRef.current inside
the onClose callback, or alternatively move the onClose logic out of the initial
useModal call and call a function that reads the current isChecked state when
closing; update the onClose callback used in useModal (and references to
isChecked in UpdateModalWrapper) to use the ref/current-value approach so the
correct checked state is saved to localStorage (STORAGE_KEY with
LATEST_PATCH_NOTE.version and dismissedAt).
🧹 Nitpick comments (1)
src/components/UpdateModal/UpdateModalWrapper.tsx (1)
78-88: 불필요한 Fragment를 제거할 수 있습니다.
<></>Fragment가 자식 요소가 하나뿐이므로 불필요합니다.♻️ 제안 수정
return ( - <> - <ModalWrapper> - <UpdateModal - data={LATEST_PATCH_NOTE} - isChecked={isChecked} - onChecked={setIsChecked} - /> - </ModalWrapper> - </> + <ModalWrapper> + <UpdateModal + data={LATEST_PATCH_NOTE} + isChecked={isChecked} + onChecked={setIsChecked} + /> + </ModalWrapper> );
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/components/UpdateModal/UpdateModal.tsx`:
- Around line 68-77: The checkbox in UpdateModal lacks an associated label and
uses onChange={() => onChecked(!isChecked)}, which can break accessibility and
race-prone toggling; add a unique id on the input (e.g., checkboxId), wrap the
visible text in a <label htmlFor={checkboxId}> or use the label to contain the
input, and change the onChange to read the event value (onChange={(e) =>
onChecked(e.target.checked)}). Apply the same fix to the second checkbox
instance (the other block at lines ~91-99) and ensure the input has an
accessible name (visible text or aria-label) for screen readers.
- Around line 105-110: The "자세히 보기" button in UpdateModal can be rendered even
when there's no link, leading to a non-functional UX; update the component
(UpdateModal) so that the button is either not rendered or rendered disabled
when the link prop/state is falsy: check the link value used by
handleDetailButtonClick and conditionally render the button only if link exists,
or render a disabled button (add aria-disabled and remove or no-op the onClick)
when link is missing, and adjust classes to use the disabled styling so it
visually communicates non-interactivity.
|
/gemini review |
useon
left a comment
useon
left a comment
There was a problem hiding this comment.
숀 ~!! 신기능 소개 팝업 깔끔하게 구현해 주셔서 감사합니다 ㅎㅎㅎ!!
코드리뷰가 제미나이까지 생겨서 너무 좋네요.. 얘네가 일차적으로 잡아줘서 다른 부분에 대한 리뷰들을 남겼습니다요!! 편하게 보시고 궁금한 것 있으면 언제든 남겨주십시오 ~!!! ^___ ^
| @@ -0,0 +1,115 @@ | |||
| import MegaphoneAsset from './MegaphoneAsset'; | |||
| import NoticeAsset from './NoticeAsset'; | |||
There was a problem hiding this comment.
해당 부분들은 왜 svg로 관리하려고 했는지 궁금해요 !!
There was a problem hiding this comment.
벡터를 사용하는 것에서는 딱히 이유를 생각해본 적 없이 단순하게 '해상도 달라져도 안 깨지니까...' 정도의 입장에서 쓰고 있었는데, 썬데이께 설명을 드리기 위해 생각해보니 조금 더 설득력 있는 근거가 필요할 것 같더군요. 그래서 나름의 이유를 조금 찾아보았는데요.
일단 첫 번째 이유로는 먼저 언급한 '해상도 깨져도 깔끔하게 렌더링'된다는 장점 때문입니다. 개인적으로 이미지 깨지는 걸 싫어하기도 하고 디자이너가 작업해 준 에셋이 가능한 그대로 문제 없이 반영되었으면 하는 마음에 벡터 적용이 가능한 에셋은 계속 벡터를 써 왔습니다.
두 번째부터는 Gemini의 도움을 통해 확인한 정보인데요. 벡터 이미지의 장점은 PNG, JPEG 등 일반 이미지에 비해 스타일링이 편하다는 점입니다. <svg className="..."></svg>의 코드를 통해 바로 원하는 스타일을 바로 적용할 수 있으니까요.
세 번째가 가장 궁금하실 수 있는 부분이라고 생각하는데요, 렌더링 과정에서 발생할 지도 모르는 성능상 문제도 없습니다. 정확한 수치인지는 모르겠으나 보통 SVG 이미지는 용량이 수 백 KB에서 MB 단위를 넘어갈 때 렌더링에 지연을 일으킨다고 합니다. 그러나 현재 첨부된 2건의 SVG 컴포넌트는 각각 3 KB와 15 KB 정도에 불과하기 때문에, 성능상 큰 문제도 없을 것이라는 게 Gemini의 의견이었네요.
이 정도까지가 벡터를 사용함에 있어 참고할 만한 정보일 것 같습니다. 향후 썬데이께서도 벡터 이미지 사용하실 때 알고 계시면 좋을 것 같아요. 생각해 볼 기회를 주신 부분에 감사하다는 말씀을 드립니다...!
There was a problem hiding this comment.
숀의 생각이 궁금해서 여쭤봤어요! 이해가 아주 쏙쏙갑니다요 ㅎㅎ 성능상의 이슈가 지금은 없기 때문에 svg기반으로 가도 괜찮을 것 같네요! 성능 이슈가 발생하면 그때 개선해도 좋을 것 같아요. 이미지가 엄청 많은 서비스가 아니라서 ..
저는 그동안 피그마에서 export하기도 편하고, 이미지에 대한 스타일링을 직접 적용할 일이 없었어서 png로 사용을 했거든요. 이미지를 2배로 뽑으면 깨질 일이 없더라고요 (현업자와의 코드리뷰에서 배운 꿀팁이었답니다.. !)
그러면 혹시 피그마에서 svg xml 방식으로 바로 뽑아내는 방법이 있나요? 제가 찾았을 때는 이미지 파일로 저장만 있어서 나중에 알려주시면 감사하겠슴니다 !!!! 👍
| <UpdateModal | ||
| data={LATEST_PATCH_NOTE} | ||
| isChecked={isChecked} | ||
| onChecked={setIsChecked} |
There was a problem hiding this comment.
set함수를 바로 넘기기 보다
const handleChecked = (next:
boolean) => {
setIsChecked(next);
isCheckedRef.current = next;
}
요런식으로 함수를 만들면 checked에 따라 ref를 갱신하는 useEffect를 하나 줄일 수도 있을 것 같아요 !!!
There was a problem hiding this comment.
오... 날카로운 의견이네요. 반영했습니다! 추가로 말씀주신 것처럼 useEffect도 같이 제거했어요.
| // 버전이 동일하다면, 7일이 지났는지 확인 | ||
| const now = new Date(); | ||
| const timeDiff = now.getTime() - dismissDate.getTime(); | ||
| const daysDiff = Math.floor(timeDiff / (1000 * 3600 * 24)); |
There was a problem hiding this comment.
요기 useEffect에 있는 로직을 책임별로 로직을 분리해 볼 수 있을 것 같아요!
저는 로컬스토리지 파싱 + 검증하는 함수랑 버전과 날짜를 비교해 모달을 띄울 수 있는지를 확인하는 함수로 분리하는 것을 생각했어요!!
그리고 useEffect 내부에서 해당 함수들을 호출해 처리하는 방법은 어떨까욥?!
There was a problem hiding this comment.
말씀 주신 대로는 아니지만 로직을 조금 분리하여 아래의 2개 헬퍼 함수로 분리했습니다:
- 2개의
Date간 날짜 차이를 계산하는 함수 - 로컬 저장소 값을 검증하고 모달 오픈 여부를 결정하는 함수
| }; | ||
|
|
||
| return ( | ||
| <div className="flex size-[1000px] flex-col rounded-[22px] bg-default-white"> |
There was a problem hiding this comment.
100% 비율로 확인을 했는데 이렇게 잘리는 경우가 발생해서 크기에 대한 수정이 필요할 것 같습니다. 모달 전체가 한 화면에 들어오는 크기면 좋을 것 같아요!! 피그마도 확인했는데 화면 전체에 모달이 어느 크기인지에 대해서는 안 나와있어서 써니랑 이야기 해 보시고 반영해 주세욥 ~!!
There was a problem hiding this comment.
이 부분 Discord #design 채널 채팅 내용에서 보셨겠지만 써니와의 논의 후 변경 사항 적용하여 반영하였습니다!
There was a problem hiding this comment.
논의와 작업 내용 확인했습니다 !!!!!! 쵝오입니다요 ~!!!
| {/* 버튼 영역 */} | ||
| <button | ||
| className="flex h-[88px] flex-row items-center justify-center bg-brand transition-all hover:bg-brand-hover" | ||
| onClick={handleDetailButtonClick} |
There was a problem hiding this comment.
요것은 기획적인 부분인데 .. 자세히 보기를 누르면 패치노트 링크가 새로운 탭에 열리는데 모달은 닫혀야 자연스럽지 않을까하는 생각인데 어떻게 생각하시나요?? 🧐
There was a problem hiding this comment.
동의합니다! '자세히 보기' 버튼 클릭 시 모달도 닫히도록 코드 추가해 두겠습니다. 동시에, 애초에 모달이 닫힐 때 체크박스 여부도 함께 확인하기 때문에, 결론적으로는 '일주일 간 보지 않기' 체크박스 체크 여부도 잘 반영한다는 점을 알립니다.
There was a problem hiding this comment.
2026-02-03.3.50.21.mov
아직 요 부분이 반영되지 않을 것 같아요 !
There was a problem hiding this comment.
헉 제가 다른 거 작업하다가 정신 놓고 빼버렸군요... 지금 반영했습니다! 꼼꼼한 확인 정말정말 감사합니다 😭😭😭
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/components/UpdateModal/UpdateModal.stories.tsx`:
- Around line 2-3: The PatchNoteImage import in UpdateModal.stories.tsx uses the
wrong relative path; update the import of PatchNoteImage (currently
'../assets/patchNote/0001.png') to the correct path
'../../assets/patchNote/0001.png' so it resolves to
src/assets/patchNote/0001.png, and remove or adjust the misleading comment "//
이미지 임포트는 아래와 같이" that was copied from src/constants/patch_note.ts since it
doesn't apply to this file's directory.
🧹 Nitpick comments (1)
src/components/UpdateModal/UpdateModal.tsx (1)
66-70: 불필요한 타입 단언(type assertion)을 제거하세요.
isPredefinedPatchNote(data)타입 가드 내부에서는 TypeScript가 이미data를PredefinedPatchNoteData로 추론하므로,(data as PredefinedPatchNoteData)단언이 불필요합니다.♻️ 제안 수정
<p className="text-[clamp(32px,2.8vw,45px)] font-bold text-brand"> - {(data as PredefinedPatchNoteData).title} + {data.title} </p> <div className="mb-[1.6%] mt-[0.8%] h-[2px] w-[10%] bg-brand" /> <p className="text-center text-[clamp(20px,1.5vw,24px)]"> - {(data as PredefinedPatchNoteData).description} + {data.description} </p>
useon
left a comment
useon
left a comment
There was a problem hiding this comment.
숀 ! 꼼꼼하게 반영해 주셔서 정말 감사합니다 !! ㅎㅎ
자세히 보기를 눌렀을 때 모달 꺼지는 내용은 아직 반영이 안 된 것 같아요 ! 확인해 주시면 감사하겠습니다 !!!! 👍🍀
| @@ -0,0 +1,115 @@ | |||
| import MegaphoneAsset from './MegaphoneAsset'; | |||
| import NoticeAsset from './NoticeAsset'; | |||
There was a problem hiding this comment.
숀의 생각이 궁금해서 여쭤봤어요! 이해가 아주 쏙쏙갑니다요 ㅎㅎ 성능상의 이슈가 지금은 없기 때문에 svg기반으로 가도 괜찮을 것 같네요! 성능 이슈가 발생하면 그때 개선해도 좋을 것 같아요. 이미지가 엄청 많은 서비스가 아니라서 ..
저는 그동안 피그마에서 export하기도 편하고, 이미지에 대한 스타일링을 직접 적용할 일이 없었어서 png로 사용을 했거든요. 이미지를 2배로 뽑으면 깨질 일이 없더라고요 (현업자와의 코드리뷰에서 배운 꿀팁이었답니다.. !)
그러면 혹시 피그마에서 svg xml 방식으로 바로 뽑아내는 방법이 있나요? 제가 찾았을 때는 이미지 파일로 저장만 있어서 나중에 알려주시면 감사하겠슴니다 !!!! 👍
| {/* 버튼 영역 */} | ||
| <button | ||
| className="flex h-[88px] flex-row items-center justify-center bg-brand transition-all hover:bg-brand-hover" | ||
| onClick={handleDetailButtonClick} |
There was a problem hiding this comment.
2026-02-03.3.50.21.mov
아직 요 부분이 반영되지 않을 것 같아요 !
| }; | ||
|
|
||
| return ( | ||
| <div className="flex size-[1000px] flex-col rounded-[22px] bg-default-white"> |
There was a problem hiding this comment.
논의와 작업 내용 확인했습니다 !!!!!! 쵝오입니다요 ~!!!
useon
left a comment
useon
left a comment
There was a problem hiding this comment.
숀 ~!! 수정 코드 및 동작 잘 되는지 !!!!! 타입 단언 부분만 한번 봐주세요 😆 미리 어프루브 남깁니다. 고생하셨어요 !!! 👍
| {(data as PredefinedPatchNoteData).title} | ||
| </p> | ||
| <div className="mb-[1.6%] mt-[0.8%] h-[2px] w-[10%] bg-brand" /> | ||
| <p className="text-center text-[clamp(20px,1.5vw,24px)]"> | ||
| {(data as PredefinedPatchNoteData).description} |
There was a problem hiding this comment.
위에 타입을 정해둬서 타입 단언하지 않아도 추론이 될 것 같아요 !!!!!
2 participants
🚩 연관 이슈
closed #410
📝 작업 내용
이 PR에서는 신 기능 업데이트 시, 해당 내용을 소개하는 모달을 구현합니다. 크게 다음 3가지 구현을 다룹니다:
신 기능 데이터 인터페이스
신 기능을 묘사할 데이터 타입을 인터페이스 형태로 정의하였습니다. 아래와 같아요:
인터페이스 정의
이 데이터 타입은 src\constants\patch_note.ts에 저장되어 있습니다. 패치 노트는 아래 2가지 유형으로 구분되는데요:
PredefinedPatchNoteDataImageOnlyPatchNoteData각 패치 노트의 유형에 대한 설명은 Notion 문서를 참고해주세요.
신규 패치 노트 작성이 필요할 때
위 인터페이스와 동일한 파일(이 데이터 타입은 src\constants\patch_note.ts)에서 배포할 현재 패치 노트 데이터를 상수로 관리합니다. 다시 말해, 신 기능 소개 모달의 내용을 바꿔야 할 경우, 이 파일의 상수 값을 변경해주면 된다는 거죠.
신 기능 소개 모달 구현
구현 측면
모달은 크게 2가지 컴포넌트로 구성되어 있습니다. 해당 컴포넌트들은 src\components\UpdateModal에서 찾아볼 수 있어요.
또한, '일주일 간 보지 않기' 체크박스를 누른 채로 모달을 닫는 경우, 닫는 시점의 시각을 로컬 저장소(local storage)에 저장하여, 일주일이 지났는지 여부를 계산할 수 있는 로직을
UpdateModalWrapper에 구현하였습니다.UI에 들어가는 데이터 위치
아래 이미지에는 위에서 소개한
PatchNoteData의 각 변수들이 UI의 어느 부분에 들어가는지를 설명한 도식입니다. 향후 개발 시 참고하세요.전역 라우터에 신 기능 소개 모달 추가
전역 라우터(src\routes\routes.tsx)에 신 기능 소개 모달(
UpdateModalWrapper)을 추가했습니다.그 외
자동 PR 세팅 스크립트를 제거했습니다.
🏞️ 스크린샷 (선택)
🗣️ 리뷰 요구사항 (선택)
Summary by CodeRabbit
릴리스 노트
새로운 기능
사용성
테스트/스토리
잡무