[FE-Fix] 확정되지 않은 일정에서 페이지 변경 시 selectedIndex가 유지되는 문제 수정 #280
Conversation
|
Note Reviews pausedUse the following commands to manage reviews:
WalkthroughThis PR introduces several naming updates in schedule-related components. In the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
dioo1461
force-pushed
the
bugfix/fe/ongoing-pagination
branch
from
f21c302 to
b3e06b6
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
frontend/src/features/shared-schedule/ui/FinishedSchedules/FinishedScheduleListItem.tsx (2)
17-20: Remove commented-out code.The commented-out props are no longer needed and should be removed to maintain clean code.
- sharedEventDto: SharedEventDto; - // startDate: Date; - // endDate: Date; + sharedEventDto: SharedEventDto;
56-63: Consider internationalizing the fallback message.The fallback message for unspecified schedules is hardcoded in Korean. Consider using an i18n solution for better maintainability and future localization support.
- {sharedEventDto ? getDateTimeRangeString( - sharedEventDto.startDateTime, - sharedEventDto.endDateTime) - : '조율되지 않은 일정이에요'} + {sharedEventDto ? getDateTimeRangeString( + sharedEventDto.startDateTime, + sharedEventDto.endDateTime) + : t('schedule.unspecified')}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📥 Commits
Reviewing files that changed from the base of the PR and between b3e06b60f6b95ee17d887d2c73217dd7c979f093 and 7a8b23142b5fee22c3f2c6b6ef0647128ed69284.
📒 Files selected for processing (4)
frontend/src/features/shared-schedule/model/finishedSchedules.ts(1 hunks)frontend/src/features/shared-schedule/ui/FinishedSchedules/FinishedScheduleList.tsx(1 hunks)frontend/src/features/shared-schedule/ui/FinishedSchedules/FinishedScheduleListItem.tsx(3 hunks)frontend/src/features/shared-schedule/ui/FinishedSchedules/finishedScheduleListItem.css.ts(0 hunks)
💤 Files with no reviewable changes (1)
- frontend/src/features/shared-schedule/ui/FinishedSchedules/finishedScheduleListItem.css.ts
🔇 Additional comments (2)
frontend/src/features/shared-schedule/model/finishedSchedules.ts (1)
5-12:Details
❓ Verification inconclusive
LGTM! Schema change properly handles unspecified schedules.
The modification to allow null values in
SharedEventDtoSchemais well-implemented and aligns with the need to handle unspecified schedules. The use ofz.unionwith explicit null handling maintains type safety.Let's verify the schema's usage across the codebase:
🏁 Script executed:
#!/bin/bash # Search for usages of SharedEventDto type and SharedEventDtoSchema rg "SharedEventDto|SharedEventDtoSchema" -t typescriptLength of output: 91
Below is a new script to search for the schema and type usages in TypeScript files. (The earlier use of
-t typescriptcaused an error since that file type isn’t recognized. Please run the following command to verify manually that the schema is referenced where expected.)#!/bin/bash # Searching for usages of SharedEventDto and SharedEventDtoSchema in TypeScript files rg "SharedEventDto|SharedEventDtoSchema" -g "*.ts"LGTM with a note for manual verification
The change to allow
nullvalues inSharedEventDtoSchemais well implemented and aligns with our needs to handle unspecified schedules. However, because our initial automated search didn’t yield results due to file type filtering, please ensure by manually verifying (using the script above) that this schema is used appropriately throughout the codebase.frontend/src/features/shared-schedule/ui/FinishedSchedules/FinishedScheduleList.tsx (1)
38-38: LGTM! Simplified prop passing improves maintainability.The change to pass the entire
sharedEventDtoinstead of individual date properties reduces prop drilling and improves maintainability.
dioo1461
force-pushed
the
bugfix/fe/ongoing-pagination
branch
from
7a8b231 to
0f28440
Compare
|
@coderabbitai 이 pr에는 리뷰를 멈춰주세요. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
frontend/src/features/shared-schedule/ui/FinishedSchedules/FinishedScheduleListItem.tsx (2)
18-19: Remove commented code.Remove the commented-out props as they are no longer needed and maintaining commented code can lead to confusion.
- // startDate: Date; - // endDate: Date;
56-63: Consider i18n for the fallback text and performance optimization.Two suggestions for improvement:
- The hardcoded Korean text '조율되지 않은 일정이에요' should be moved to an i18n configuration for better maintainability and internationalization support.
- Consider memoizing the component to prevent unnecessary re-renders.
+import { memo } from 'react'; +import { useTranslation } from 'react-i18next'; -const MeetDate = ({ sharedEventDto }: { sharedEventDto: SharedEventDto }) => ( +const MeetDate = memo(({ sharedEventDto }: { sharedEventDto: SharedEventDto }) => { + const { t } = useTranslation(); + return ( <Text color={vars.color.Ref.Netural[600]} typo='b3R'> {sharedEventDto ? getDateTimeRangeString( sharedEventDto.startDateTime, sharedEventDto.endDateTime) - : '조율되지 않은 일정이에요'} + : t('schedule.unspecified')} </Text> + ); +}); + +MeetDate.displayName = 'MeetDate';frontend/src/components/Icon/custom/CheckGraphic.tsx (2)
2-4: Remove unused propsstrokeandfill.The
strokeandfillprops are declared but not utilized in the SVG. Consider removing them to avoid confusion.-export const CheckGraphic = ({ clickable = false, className, width = 24, height = 24 , stroke = "white", fill = "white", ...rest }: IconProps) => { +export const CheckGraphic = ({ clickable = false, className, width = 24, height = 24, ...rest }: IconProps) => {
17-147: Consider making filter IDs unique per instance.The SVG uses fixed IDs for filters and masks (e.g.,
clip0_894_193,mask0_894_193). This could cause conflicts if multiple instances of the component are rendered on the same page.Consider generating unique IDs using a hook or prop:
import { useId } from 'react'; export const CheckGraphic = ({ ... }) => { const uniqueId = useId(); return ( <svg ...> <g clipPath={`url(#clip-${uniqueId})`}> {/* Update other IDs similarly */} </g> <defs> <clipPath id={`clip-${uniqueId}`}> {/* ... */} </clipPath> </defs> </svg> ); };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📥 Commits
Reviewing files that changed from the base of the PR and between 7a8b23142b5fee22c3f2c6b6ef0647128ed69284 and 0f28440.
📒 Files selected for processing (14)
frontend/src/components/Icon/component/CheckGraphic.tsx(0 hunks)frontend/src/components/Icon/custom/CheckGraphic.tsx(1 hunks)frontend/src/components/Icon/custom/ClockGraphic.tsx(1 hunks)frontend/src/components/Icon/index.ts(0 hunks)frontend/src/features/shared-schedule/model/finishedSchedules.ts(1 hunks)frontend/src/features/shared-schedule/ui/Fallbacks/FinishedFallback.tsx(1 hunks)frontend/src/features/shared-schedule/ui/Fallbacks/OngoingFallback.tsx(1 hunks)frontend/src/features/shared-schedule/ui/Fallbacks/UpcomingFallback.tsx(1 hunks)frontend/src/features/shared-schedule/ui/FinishedSchedules/FinishedScheduleList.tsx(1 hunks)frontend/src/features/shared-schedule/ui/FinishedSchedules/FinishedScheduleListItem.tsx(3 hunks)frontend/src/features/shared-schedule/ui/FinishedSchedules/finishedScheduleListItem.css.ts(0 hunks)frontend/src/features/shared-schedule/ui/OngoingSchedules/OngoingScheduleList.tsx(3 hunks)frontend/src/features/shared-schedule/ui/OngoingSchedules/ScheduleDetails.tsx(2 hunks)frontend/src/hooks/usePagination.ts(1 hunks)
💤 Files with no reviewable changes (3)
- frontend/src/features/shared-schedule/ui/FinishedSchedules/finishedScheduleListItem.css.ts
- frontend/src/components/Icon/component/CheckGraphic.tsx
- frontend/src/components/Icon/index.ts
✅ Files skipped from review due to trivial changes (3)
- frontend/src/features/shared-schedule/ui/Fallbacks/FinishedFallback.tsx
- frontend/src/features/shared-schedule/ui/Fallbacks/OngoingFallback.tsx
- frontend/src/features/shared-schedule/ui/Fallbacks/UpcomingFallback.tsx
🚧 Files skipped from review as they are similar to previous changes (5)
- frontend/src/hooks/usePagination.ts
- frontend/src/features/shared-schedule/ui/OngoingSchedules/OngoingScheduleList.tsx
- frontend/src/features/shared-schedule/model/finishedSchedules.ts
- frontend/src/features/shared-schedule/ui/OngoingSchedules/ScheduleDetails.tsx
- frontend/src/features/shared-schedule/ui/FinishedSchedules/FinishedScheduleList.tsx
🔇 Additional comments (5)
frontend/src/components/Icon/custom/ClockGraphic.tsx (2)
10-10: Verify the clock hand orientation after rotation change.The rotation angle change from
135to-135degrees will affect the clock hand's orientation. Please verify that this adjustment correctly positions the clock hand as intended.
14-14: Verify the gradient appearance after coordinate adjustment.The linearGradient's x1 coordinate has been significantly adjusted from a large positive value to a very small negative value (-1.8593e-06). While this value is more typical for gradient coordinates, please verify that the gradient renders as expected across different screen sizes and resolutions.
frontend/src/features/shared-schedule/ui/FinishedSchedules/FinishedScheduleListItem.tsx (1)
23-28: LGTM! Clean prop restructuring.The component prop changes are well-implemented, maintaining clean code structure while adapting to the new data model.
frontend/src/components/Icon/custom/CheckGraphic.tsx (2)
6-16: Well-structured SVG root element with proper accessibility!The SVG element is well-implemented with:
- Proper dimension handling with width/height props
- Accessibility support via aria-label
- Clean conditional styling for clickable state
152-152: LGTM! Proper display name setup.Setting the display name helps with debugging and follows React best practices.
|
@dioo1461: I'll pause the automated reviews for this PR. ✅ Actions performedReviews paused. |
#️⃣ 연관된 이슈>
📝 작업 내용> 이번 PR에서 작업한 내용을 간략히 설명해주세요(이미지 첨부 가능)
🙏 여기는 꼭 봐주세요! > 리뷰어가 특별히 봐주었으면 하는 부분이 있다면 작성해주세요
Summary by CodeRabbit
Refactor
ScheduleContentstoScheduleDetails.FinishedScheduleListby passing the entiresharedEventDtoobject to child components, simplifying data management.New Features
CheckGraphiccomponent for enhanced iconography.Bug Fixes
ClockGraphicandCheckGraphiccomponents to reflect their new locations, ensuring proper functionality across the application.