Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR implements UI detail changes for the study detail page, including tab navigation, a more menu dropdown, and various UI refinements for the 1.5th phase design update.
Key Changes:
- Added tabbed navigation for study detail page (intro, members, channel sections)
- Implemented a reusable more menu dropdown component for actions
- Refactored study detail page by extracting info section into separate component
- Updated header navigation to uncomment "스터디 둘러보기" link
Reviewed Changes
Copilot reviewed 16 out of 17 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/shared/ui/tabs/index.tsx | New reusable tabs component for section navigation |
| src/shared/ui/dropdown/more-menu.tsx | New dropdown menu component for action buttons |
| src/features/study/group/ui/study-info-section.tsx | Extracted study information display logic from detail page |
| src/features/study/group/ui/summary-study-info.tsx | New component for summary info card with action buttons |
| src/features/study/group/ui/group-study-detail-page.tsx | Refactored to use tabs and extracted components |
| src/widgets/study/group/ui/group-detail/Info-card.tsx | Updated styling for better spacing |
| src/widgets/home/header.tsx | Uncommented "스터디 둘러보기" navigation link |
| src/features/study/group/ui/group-study-list.tsx | Removed unused fetchNextPage variable |
| src/features/study/group/model/use-study-query.ts | Removed unused prefetch utility functions |
| src/features/study/group/api/get-gruoup-study-detail.ts | Removed debug console.log statements |
| src/features/study/group/application/api/*.ts | Fixed type naming convention (PascalCase) |
| src/features/my-page/ui/*.tsx | Updated import paths for relocated types |
| app/(service)/(nav)/study/[id]/page.tsx | Removed server-side prefetching logic |
Comments suppressed due to low confidence (1)
src/features/study/group/ui/study-info-section.tsx:1
- The className combines 'truncate' (which already includes overflow-hidden, text-ellipsis, and whitespace-nowrap) with redundant properties. Remove 'overflow-hidden text-ellipsis whitespace-nowrap' as they are already included in 'truncate'.
import dayjs from 'dayjs';
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| groupStudyId: number; | ||
| } | ||
|
|
||
| export default function StudyInfoSection({ |
There was a problem hiding this comment.
해당 컴포넌트도 특별한 이유가 없다면 분리하는 권장드립니다. 추후 재사용 및 유지 보수에 분리하면 도움이 될 것 같습니다. 예를 들어, basicInfoItems, summaryBasicInfoItems, getDurationText 의 경우 lib 폴더나 util로 분리하는 것이 좋지 않을까요? 추후 재사용될 가능성이 있기때문에 말이죠.
There was a problem hiding this comment.
@de24world
말씀하신 label과 value를 매칭시키는 함수를 utils로 빼려고합니다.
현재 해당 데이터를 사용하는 ui가 아이콘의 여부와 데이터 갯수에 따라 3가지가 존재하는데요.
- props로 원하는 데이터를 받고 label-value 형태의 데이터를 반환하는 함수 (utils로 데이터 가공에 중점)
- 위 함수와 아이콘을 조합하는 함수 (아이콘이 들어가면서 .tsx 파일로 구분되야하며 아이콘의 크기나 색깔에 대한 확장성도 고려해야하는지)
위 두가지 함수 분리 기준에 대한 피드백 받고싶습니다!
There was a problem hiding this comment.
말씀하신데로,
- 데이터 가공(매핑)은 utils(.ts)
- 아이콘 등 UI 조합은 .tsx
UI와 데이터 로직을 명확히 분리하면 확장성과 유지보수성이 크게 향상됩니다!
| return ( | ||
| <div className="flex w-full gap-600"> | ||
| <div className="flex flex-1 flex-col gap-500"> | ||
| <Image |
There was a problem hiding this comment.
모바일뷰에서는 해당 이미지 문제가 없나요? fill, sizes 를 쓰거나 혹은 responsive custom hook을 만드는 방법을 추천드립니다.
| <p className="font-designer-20b">기본 정보</p> | ||
| {/* <div>프로필박스</div> */} | ||
| <div className="grid grid-cols-4 grid-rows-3 gap-150"> | ||
| {basicInfoItems(studyDetail?.basicInfo).map((item, index) => { |
There was a problem hiding this comment.
해당 코드는 불필요한 리렌더링 야기시킬 수 있을거 같습니다. 가능하다면 useMemo를 사용해보는 것도 좋을 것 같습니다.
예)
const items = useMemo(() => mapBasicInfo(basicInfo), [basicInfo]);
items.map(item => <InfoCard title={item.label} icon={<item.icon />} />);
|
추가로 github copilot 자동리뷰 추가하였습니다. 확인하시고, copilot에 제시한 내용이 괜찮다고 생각하시면, 수정하시고 copilot이 준 피드백이 별로다 하면 resolve conversation 눌러서 무시하시면 될듯 싶습니다. |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
🌱 연관된 이슈
☘️ 작업 내용
🍀 참고사항
스크린샷 (선택)