-
Notifications
You must be signed in to change notification settings - Fork 1
fix : 베타테스트 전 QA #166
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
fix : 베타테스트 전 QA #166
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Summary of ChangesHello @caseBread, 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은 베타 테스트 전 QA 과정에서 발견된 문제들을 수정하고 사용자 경험을 개선하기 위한 변경 사항들을 포함합니다. 주요 변경 사항으로는 날짜 선택기의 유효성 검사 로직 강화, 4컷 이미지 다운로드 시 로딩 상태 표시, 사용자 행동 분석을 위한 Google Analytics 이벤트 추적 추가, 그리고 UI 텍스트 및 스피너 컴포넌트의 유연성 향상이 있습니다. 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. 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
베타 테스트 전 QA를 위한 수정 사항들을 리뷰했습니다. 전반적으로 QA 항목들을 잘 반영하셨습니다. 몇 가지 코드 개선 사항을 제안드립니다. scrollable-date-picker에서는 헬퍼 함수들을 useCallback으로 메모이징하고 순수 함수를 컴포unlimited 밖으로 빼내어 성능과 가독성을 높일 수 있습니다. ScreenAlbum4Cut에서는 중복 코드를 리팩토링하여 유지보수성을 개선할 여지가 있습니다. ScreenAlbumSidebar에서는 useEffect의 의존성 배열에 누락된 값이 있어 수정이 필요합니다. 마지막으로 Spinner 컴포넌트의 기본값이 변경되었는데, 이 부분이 의도된 것인지 확인하고 관련 주석을 수정할 필요가 있어 보입니다. 자세한 내용은 각 파일의 리뷰 코멘트를 참고해주세요.
| useEffect(() => { | ||
| if (isOpen) { | ||
| trackGaEvent(GA_EVENTS.view_albumsidebar, { | ||
| album_id: albumId, | ||
| access_type: informData?.myRole === 'MAKER' ? 'creator' : 'member', | ||
| }); | ||
| } | ||
| }, [isOpen]); |
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의 의존성 배열에 albumId와 informData가 누락되었습니다. trackGaEvent 콜백 함수가 이 값들을 사용하므로, 의존성 배열에 추가하여 isOpen 상태가 변경될 때 뿐만 아니라 albumId나 informData가 변경될 때도 이벤트가 올바르게 추적되도록 해야 합니다.
| useEffect(() => { | |
| if (isOpen) { | |
| trackGaEvent(GA_EVENTS.view_albumsidebar, { | |
| album_id: albumId, | |
| access_type: informData?.myRole === 'MAKER' ? 'creator' : 'member', | |
| }); | |
| } | |
| }, [isOpen]); | |
| useEffect(() => { | |
| if (isOpen) { | |
| trackGaEvent(GA_EVENTS.view_albumsidebar, { | |
| album_id: albumId, | |
| access_type: informData?.myRole === 'MAKER' ? 'creator' : 'member', | |
| }); | |
| } | |
| }, [isOpen, albumId, informData]); |
| const clamp = (value: number, min: number, max: number) => | ||
| Math.min(Math.max(value, min), max); | ||
|
|
||
| const getDaysInMonth = (year: number, month: number) => { | ||
| return new Date(year, month, 0).getDate(); | ||
| }; |
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 getMinMonthForYear = (year: number) => { | ||
| if (minDate && year === minYear) { | ||
| return minDate.getMonth() + 1; | ||
| } | ||
| return 1; | ||
| }; | ||
|
|
||
| const getMaxMonthForYear = (year: number) => { | ||
| if (maxDate && year === maxYear) { | ||
| return maxDate.getMonth() + 1; | ||
| } | ||
| return 12; | ||
| }; | ||
|
|
||
| const getMinDayForMonth = (year: number, month: number) => { | ||
| if (minDate && year === minYear && month === minDate.getMonth() + 1) { | ||
| return minDate.getDate(); | ||
| } | ||
| return 1; | ||
| }; | ||
|
|
||
| const getMaxDayForMonth = (year: number, month: number) => { | ||
| const daysInMonth = getDaysInMonth(year, month); | ||
| if (maxDate && year === maxYear && month === maxDate.getMonth() + 1) { | ||
| return Math.min(daysInMonth, maxDate.getDate()); | ||
| } | ||
| return daysInMonth; | ||
| }; |
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.
이 헬퍼 함수들은 minDate, maxDate와 같은 props에 의존합니다. React.useCallback으로 메모이제이션하여 매 렌더링마다 함수가 재생성되는 것을 방지하면 성능을 최적화할 수 있습니다. 이전 제안에 따라 getDaysInMonth를 컴포넌트 외부로 옮기면 getMaxDayForMonth의 메모이제이션이 더 효과적입니다.
const getMinMonthForYear = React.useCallback((year: number) => {
if (minDate && year === minYear) {
return minDate.getMonth() + 1;
}
return 1;
}, [minDate, minYear]);
const getMaxMonthForYear = React.useCallback((year: number) => {
if (maxDate && year === maxYear) {
return maxDate.getMonth() + 1;
}
return 12;
}, [maxDate, maxYear]);
const getMinDayForMonth = React.useCallback((year: number, month: number) => {
if (minDate && year === minYear && month === minDate.getMonth() + 1) {
return minDate.getDate();
}
return 1;
}, [minDate, minYear]);
const getMaxDayForMonth = React.useCallback((year: number, month: number) => {
const daysInMonth = getDaysInMonth(year, month);
if (maxDate && year === maxYear && month === maxDate.getMonth() + 1) {
return Math.min(daysInMonth, maxDate.getDate());
}
return daysInMonth;
}, [maxDate, maxYear, getDaysInMonth]);
| export default function Spinner({ | ||
| color = '#ffffff', // 기본값 (tailwind primary-400 정도) | ||
| size = 64, | ||
| }: SpinnerProps) { |
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.
요약
구현 사항
📸 스크린샷
Need Review
Reference
📜 리뷰 규칙
Reviewer는 아래 P5 Rule을 참고하여 리뷰를 진행합니다.
P5 Rule을 통해 Reviewer는 Reviewee에게 리뷰의 의도를 보다 정확히 전달할 수 있습니다.