Open
Conversation
hanjiwon1108
requested review from
GAMZAMANDU,
Copilot,
jaehwan02,
kwonminjae5700 and
obtuse-triangle
There was a problem hiding this comment.
Pull Request Overview
This PR enhances mobile responsiveness for the StudentDetail page and the calendar in FieldTraining.
- Wraps Calendar components in responsive containers and adjusts styling for mobile.
- Refactors StudentDetail layout, button sizes, and visibility to improve mobile UX.
- Removes an unused import and updates placeholder handlers.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/features/users/components/field-training.tsx | Added responsive wrappers around Calendar, removed unused Label import, and simplified handlers. |
| src/features/users/components/StudentDetail.tsx | Updated container and typography classes, adjusted buttons and separators for mobile layouts. |
Comments suppressed due to low confidence (3)
src/features/users/components/field-training.tsx:125
- [nitpick] The empty onClick handler is a placeholder without a TODO or implementation; consider adding a TODO comment or removing the handler until the feature is implemented to avoid confusion.
</div>
src/features/users/components/StudentDetail.tsx:61
- [nitpick] The prop name 'datas' passed to the Employment component is unconventional English; consider renaming it to 'data' or 'items' for clarity.
component: (data) => (
src/features/users/components/StudentDetail.tsx:163
- The "취소" button hides its text on small screens, leaving only the icon visible; ensure there is an appropriate aria-label on the Button component for screen readers when text is hidden.
>
Comment on lines
114
to
124
| <div className='w-full overflow-x-auto md:overflow-visible'> | ||
| <div className='flex min-w-full justify-center'> | ||
| <Calendar | ||
| mode='range' | ||
| selected={updateDate} | ||
| onSelect={setUpdateDate} | ||
| className='transform-origin-center w-auto max-w-full scale-[0.85] rounded-lg border border-border p-1 sm:scale-100 sm:p-2' | ||
| showOutsideDays={false} | ||
| /> | ||
| </div> | ||
| </div> |
There was a problem hiding this comment.
[nitpick] The responsive Calendar wrapper layout is duplicated for both date ranges; consider extracting this into a reusable component to DRY up the code and improve maintainability.
jaehwan02
approved these changes
Jul 14, 2025
Member
|
브랜치 컨벤션 PR 컨벤션 좀 지키세요. |
Member
1. 캘린더의 반응형이 제대로 동작하지 않습니다.
2. 각 요소들에 계획되지 않은 css 가 생겼습니다. main 브랜치의 디자인과 비교하고 수정해 주시길 바랍니다.
|
guhyunwoo
approved these changes
Sep 9, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
💡 개요
StudentDetail 페이지 모바일 지원
Calender 반응형 구현
📃 작업내용
StudentDetail과 field-training, 반응형 구현 (모바일)
🔀 변경사항
📸 스크린샷