-
Notifications
You must be signed in to change notification settings - Fork 1
fix: 날짜피커 생성 #159
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: 날짜피커 생성 #159
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! 이 풀 리퀘스트는 기존에 사용하던 외부 라이브러리 기반의 날짜 피커를 제거하고, 사용자 경험을 개선하기 위해 직접 개발한 스크롤 가능한 날짜 피커로 교체하는 것을 목표로 합니다. 특히 모바일 환경에서의 날짜 선택 편의성을 높이기 위해 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
날짜 피커를 기존 react-day-picker 기반의 캘린더에서 커스텀 스크롤 피커로 교체하는 작업을 리뷰했습니다. 모바일 환경에 더 적합한 UI를 제공하려는 의도가 좋아 보입니다. 새로 추가된 ScrollableDatePicker 컴포넌트는 잘 구현되었지만, 몇 가지 잠재적인 버그와 개선점을 발견했습니다. 특히 AlbumInfos 컴포넌트의 ref 전달 방식 변경은 기존 기능을 손상시킬 수 있는 심각한 문제이므로 확인이 필요합니다. 자세한 내용은 각 파일에 대한 코멘트를 참고해주세요.
| function PickerColumn({ items, value, label, onChange }: PickerColumnProps) { | ||
| const containerRef = React.useRef<HTMLDivElement>(null); | ||
| const isScrollingRef = React.useRef(false); | ||
| const scrollTimeoutRef = React.useRef<NodeJS.Timeout | null>(null); |
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.
scrollTimeoutRef의 타입을 NodeJS.Timeout | null로 지정하셨습니다. 이 컴포넌트는 'use client' 지시어가 있어 브라우저 환경에서 실행되므로, NodeJS.Timeout 타입이 항상 보장되지 않을 수 있습니다. 브라우저와 Node.js 환경 양쪽에서 호환성을 보장하기 위해 ReturnType<typeof setTimeout> | null 타입을 사용하는 것이 더 안전합니다.
| const scrollTimeoutRef = React.useRef<NodeJS.Timeout | null>(null); | |
| const scrollTimeoutRef = React.useRef<ReturnType<typeof setTimeout> | null>(null); |
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.
반영
| // Debounce: wait for scroll to stop | ||
| scrollTimeoutRef.current = setTimeout(() => { | ||
| if (!containerRef.current) return; | ||
|
|
||
| const scrollTop = containerRef.current.scrollTop; | ||
| const index = Math.round(scrollTop / ITEM_HEIGHT); | ||
| const clampedIndex = Math.max(0, Math.min(index, items.length - 1)); | ||
|
|
||
| // Snap to exact position | ||
| containerRef.current.scrollTop = clampedIndex * ITEM_HEIGHT; | ||
|
|
||
| const newValue = items[clampedIndex]; | ||
| if (newValue !== undefined && newValue !== value) { | ||
| onChange(newValue); | ||
| } | ||
|
|
||
| isScrollingRef.current = false; | ||
| }, 100); | ||
| }; |
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.
setTimeout 콜백 함수에서 value prop을 직접 사용하고 있습니다. setTimeout이 실행되기 전에 value prop이 변경될 경우, 콜백은 오래된(stale) value 값을 사용하여 비교하게 되어 의도치 않은 동작을 유발할 수 있습니다. 이 문제를 해결하기 위해 useRef를 사용하여 항상 최신 value 값을 참조하도록 하는 것이 좋습니다.
// PickerColumn 상단에 추가
const valueRef = React.useRef(value);
valueRef.current = value;
// setTimeout 콜백 내부 수정
scrollTimeoutRef.current = setTimeout(() => {
// ...
const newValue = items[clampedIndex];
if (newValue !== undefined && newValue !== valueRef.current) {
onChange(newValue);
}
// ...
}, 100);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 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.
| {/* Top fade gradient */} | ||
| <div | ||
| className='pointer-events-none absolute inset-x-0 top-0 z-20 h-24' | ||
| style={{ | ||
| background: 'linear-gradient(to bottom, white 0%, transparent 100%)', | ||
| }} | ||
| /> | ||
|
|
||
| {/* Bottom fade gradient */} | ||
| <div | ||
| className='pointer-events-none absolute inset-x-0 bottom-0 z-20 h-24' | ||
| style={{ | ||
| background: 'linear-gradient(to top, white 0%, transparent 100%)', | ||
| }} | ||
| /> | ||
| </div> |
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.
상단 및 하단 fade-out 효과에 white 색상이 하드코딩되어 있습니다. 다크 모드 등 다른 테마를 지원할 경우 UI가 어색해 보일 수 있습니다. 인라인 스타일 대신 Tailwind CSS의 그라데이션 유틸리티 클래스(bg-gradient-to-b from-white, bg-gradient-to-t from-white)를 사용하면 테마와 일관성을 유지하고 코드를 더 깔끔하게 만들 수 있습니다.
| {/* Top fade gradient */} | |
| <div | |
| className='pointer-events-none absolute inset-x-0 top-0 z-20 h-24' | |
| style={{ | |
| background: 'linear-gradient(to bottom, white 0%, transparent 100%)', | |
| }} | |
| /> | |
| {/* Bottom fade gradient */} | |
| <div | |
| className='pointer-events-none absolute inset-x-0 bottom-0 z-20 h-24' | |
| style={{ | |
| background: 'linear-gradient(to top, white 0%, transparent 100%)', | |
| }} | |
| /> | |
| </div> | |
| {/* Top fade gradient */} | |
| <div | |
| className='pointer-events-none absolute inset-x-0 top-0 z-20 h-24 bg-gradient-to-b from-white to-transparent' | |
| /> | |
| {/* Bottom fade gradient */} | |
| <div | |
| className='pointer-events-none absolute inset-x-0 bottom-0 z-20 h-24 bg-gradient-to-t from-white to-transparent' | |
| /> |
요약
기존에 Calander 쓰던 날짜피커 커스텀으로 직접 제작
구현 사항
📸 스크린샷
Need Review
Reference
📜 리뷰 규칙
Reviewer는 아래 P5 Rule을 참고하여 리뷰를 진행합니다.
P5 Rule을 통해 Reviewer는 Reviewee에게 리뷰의 의도를 보다 정확히 전달할 수 있습니다.