Conversation
Summary of ChangesHello @yummjin, 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은 홈 및 일정 페이지에 일정 조회 기능을 통합하여 사용자에게 동적인 일정 정보를 제공합니다. 기존 목업 데이터를 실제 API 호출로 대체하고, 달력 컴포넌트를 개선하여 선택된 날짜에 따라 일정을 정확하게 표시하도록 업데이트되었습니다. 또한, 데이터 로딩 중 사용자 경험을 향상시키기 위해 스켈레톤 로더가 추가되었습니다. 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. Changelog
Activity
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
✅ 잘한 점
- 홈 및 일정 페이지에서 일정 목록 조회 API 연동 및 달력에서 일정 조회 API 연동 기능이 성공적으로 구현되었습니다.
ScheduleListSkeleton컴포넌트를 추가하여 로딩 상태를 사용자에게 명확하게 보여주는 점이 좋습니다.ScheduleListItem에서function선언을constarrow function으로 변경하여 코드 컨벤션을 준수한 점이 좋습니다.ScheduleCalendarBase.css.ts에서.react-calendar__month-view__days__day--neighboringMonth스타일을display: 'none'에서visibility: 'hidden', pointerEvents: 'none'으로 변경하여 접근성과 레이아웃 안정성을 개선한 점이 좋습니다.END_POINT.SCHEDULE.MEMBER_LIST상수가BIG_SNAKE_CASE규칙을 잘 따르고 있습니다.
❌ 위반 사항
- [TanStack Query 네이밍 규칙 준수]
getScheduleCalendarQuery의queryKey에request파라미터가 포함되어 있지 않아, 요청 파라미터가 변경될 때 캐싱이 제대로 동작하지 않거나 오래된 데이터가 표시될 수 있습니다. - [접근성 준수]
ScheduleList컴포넌트의CalendarIcon이 시각적으로만 사용되는 아이콘임에도aria-hidden="true"속성이 없어 스크린 리더 사용자에게 불필요하게 읽힐 수 있습니다. - [성능]
ScheduleFilterTab컴포넌트에서Button의keyprop으로filter.label을 사용하고 있습니다.filter.label은 중복될 가능성이 있어 React 리스트 렌더링 시 성능 저하나 예기치 않은 동작을 유발할 수 있습니다.filter.value는 고유하므로 더 적합합니다.
🔧 개선 제안
SchedulePage.tsx에서scheduleList를 필터링하는 로직이dayjs(item.meetingAt).isAfter(dayjs(selectedDate))로 되어 있어,selectedDate당일의 일정은 표시되지 않습니다.selectedDate를 포함하여 당일 이후의 일정을 표시하려면isSameOrAfter를 사용하거나,selectedDate에 해당하는 일정을 정확히 표시하려면isSame(selectedDate, 'day')를 사용하는 것이 좋습니다. API에서 이미 날짜 필터링을 제공한다면 클라이언트 측 필터링은 불필요할 수 있습니다.SchedulePage.tsx의date상태가 초기값으로undefined를 가지고 있어, 초기 API 호출 시date파라미터가 누락될 수 있습니다.selectedDate의 초기값을 기반으로date상태를 초기화하여 API 호출의 일관성을 유지하는 것이 좋습니다.
| request?: CrewScheduleCalendarRequest | ||
| ) => | ||
| queryOptions({ | ||
| queryKey: [...scheduleQueries.all, 'getScheduleCalendar', crewId], |
There was a problem hiding this comment.
[TanStack Query 네이밍 규칙 준수] getScheduleCalendarQuery의 queryKey에 request 파라미터가 포함되어 있지 않습니다. request 파라미터(예: yearMonth)가 변경될 때 TanStack Query가 데이터를 다시 가져오지 않을 수 있어 오래된 데이터가 표시될 위험이 있습니다. request를 queryKey에 포함하여 캐싱 및 재요청이 올바르게 동작하도록 해야 합니다.
| queryKey: [...scheduleQueries.all, 'getScheduleCalendar', crewId], | |
| queryKey: [...scheduleQueries.all, 'getScheduleCalendar', crewId, request], |
| items={scheduleList.filter((item) => | ||
| dayjs(item.meetingAt).isAfter(dayjs(selectedDate)) |
There was a problem hiding this comment.
현재 필터링 로직 dayjs(item.meetingAt).isAfter(dayjs(selectedDate))은 selectedDate 당일의 일정을 제외하고 그 이후의 일정만 표시합니다. 만약 selectedDate를 포함하여 당일 이후의 일정을 표시하려면 isSameOrAfter를 사용하는 것이 좋습니다. 또는 selectedDate에 해당하는 일정을 정확히 표시하려면 isSame(selectedDate, 'day')를 고려해볼 수 있습니다. API에서 이미 날짜 필터링을 제공한다면 클라이언트 측 필터링은 불필요할 수 있습니다.
| items={scheduleList.filter((item) => | |
| dayjs(item.meetingAt).isAfter(dayjs(selectedDate)) | |
| items={scheduleList.filter((item) => | |
| dayjs(item.meetingAt).isSameOrAfter(dayjs(selectedDate), 'day') | |
| )} |
| SCHEDULE: { | ||
| LIST: (crewId: number) => `crews/${crewId}/schedules`, | ||
| CALENDAR: (crewId: number) => `crews/${crewId}/schedules/calendar`, | ||
| MEMBER_LIST: 'members/me/schedules', |
There was a problem hiding this comment.
상수에 api의 역할을 좀더 담아서 JOINED_LIST는 어떨까요?
|
|
||
| const { data: scheduleCalendarList = [] } = useQuery({ | ||
| ...scheduleQueries.getScheduleCalendarQuery(crewId, { | ||
| yearMonth: dayjs(selectedDate).format('YYYY-MM'), |
There was a problem hiding this comment.
const formatDate = (date: string | Date, format = 'YYYY.MM') => {
return dayjs(date).format(format);
};
dayjs가 군데군데 쓰여서 날짜 포맷팅 유틸로 분리 및 확장해봐도 괜찮을거 같네여!
2 participants
🛠️ 변경 사항
세부 변경 내용
🔍 관련 이슈
📸 스크린샷 / GIF (선택)
🔄 연관 작업