feat: group main page & MainHeader component#30
Conversation
|
Caution Review failedThe pull request is closed. Walkthrough이 변경 사항은 그룹 관련 페이지 및 공통 UI 컴포넌트, 커스텀 훅, 모달, 캐러셀, 카드, 헤더, 검색바 등 다양한 React 컴포넌트와 스타일 컴포넌트를 새롭게 도입합니다. 또한, 그룹 페이지 라우트가 추가되고, 기존 로그인 페이지의 Wrapper 스타일 컴포넌트가 공통 컴포넌트로 교체되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant GroupPage
participant MainHeader
participant MyGroupBox
participant RecruitingGroupCarousel
participant Modal
User->>GroupPage: 그룹 페이지 접속
GroupPage->>MainHeader: 헤더 렌더링(type: group)
GroupPage->>MyGroupBox: 내 그룹 리스트 렌더링
GroupPage->>RecruitingGroupCarousel: 모집 그룹 캐러셀 렌더링
User->>MainHeader: 완료된 그룹 버튼 클릭
MainHeader->>GroupPage: leftButtonClick 호출
GroupPage->>Modal: CompletedGroupModal 표시
User->>MyGroupBox: 더보기 클릭
MyGroupBox->>GroupPage: onMyGroupsClick 호출
GroupPage->>Modal: MyGroupModal 표시
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (12)
src/components/common/IconButton.tsx (1)
3-7: IconButton 컴포넌트 개선 제안기본 구조는 좋지만 다음과 같은 개선을 고려해보세요:
- 접근성 향상:
alt속성과role속성 지원- 유연한 크기 설정: props를 통한 크기 조정 가능
- 클릭 이벤트: onClick 핸들러 props 추가
개선된 구현 예시:
-export const IconButton = styled.img` - width: 24px; - height: 24px; - cursor: pointer; -`; +interface IconButtonProps { + size?: number; + onClick?: () => void; +} + +export const IconButton = styled.img<IconButtonProps>` + width: ${({ size = 24 }) => size}px; + height: ${({ size = 24 }) => size}px; + cursor: pointer; + + &:hover { + opacity: 0.8; + } +`;src/components/common/Blank.tsx (1)
15-18: styled 컴포넌트 네이밍 개선을 고려해보세요.
Wrapper라는 이름이 다른 공통 컴포넌트(src/components/common/Wrapper.tsx)와 혼동될 수 있습니다. 더 구체적인 이름을 사용하는 것이 좋겠습니다.-const Wrapper = styled.div` +const BlankWrapper = styled.div`src/hooks/useInfiniteCarousel.ts (3)
11-13: 성능 최적화를 위한 메모이제이션 개선이 필요합니다.현재 구현에서는
groups배열이 변경될 때마다 전체 배열을 재생성합니다. 대용량 데이터에서는 성능 이슈가 발생할 수 있습니다.const infiniteGroups = useMemo(() => { - return Array(CLONE_COUNT).fill(groups).flat(); + if (groups.length === 0) return []; + return Array(CLONE_COUNT).fill(groups).flat(); }, [groups]);
44-51: 무한 스크롤 재설정 로직의 임계값을 상수화하세요.하드코딩된
100px 임계값을 상수로 정의하여 유지보수성을 높이는 것이 좋겠습니다.+const RESET_THRESHOLD = 100; + const handleScroll = useCallback(() => { // ... 기존 코드 ... const maxScroll = container.scrollWidth; - if (scrollLeft < 100 || scrollLeft + container.offsetWidth > maxScroll - 100) { + if (scrollLeft < RESET_THRESHOLD || scrollLeft + container.offsetWidth > maxScroll - RESET_THRESHOLD) { const middleCard = cardRefs.current[middleIndex]; if (middleCard) { const left = middleCard.offsetLeft + middleCard.offsetWidth / 2 - center; container.scrollLeft = left; } } }, [groups.length, middleIndex]);
69-69: setTimeout 사용보다는 더 안정적인 방법을 고려해보세요.초기화 로직에서
setTimeout을 사용하는 것보다requestAnimationFrame이나ResizeObserver를 사용하는 것이 더 안정적일 수 있습니다.-const timer = setTimeout(initializeScroll, 0); +const timer = requestAnimationFrame(initializeScroll);그리고 cleanup에서도 수정이 필요합니다:
return () => { - clearTimeout(timer); + cancelAnimationFrame(timer); container.removeEventListener('scroll', handleScroll); };src/components/group/RecruitingGroup.tsx (1)
11-11: 장르 목록을 상수로 관리하는 것이 좋습니다.GENRE 배열이 하드코딩되어 있는데, 이를 더 유연하게 관리할 수 있도록 개선할 수 있습니다.
다음과 같이 개선할 수 있습니다:
-const GENRE = ['문학', '과학·IT', '사회과학', '인문학', '예술']; +const GENRE = ['문학', '과학·IT', '사회과학', '인문학', '예술'] as const;또는 props로 받아서 더 유연하게 만들 수도 있습니다.
src/components/common/SearchBar.tsx (2)
23-27: width 속성이 중복되어 있습니다.23번 줄에서
width: 100%로 설정하고 27번 줄에서 다시width: calc(100% - 40px)로 설정하고 있습니다. 두 번째 선언이 첫 번째를 덮어씁니다.다음과 같이 수정하여 중복을 제거하세요:
- width: 100%; height: 40px; background-color: var(--color-darkgrey-main); border-radius: 12px; width: calc(100% - 40px);
9-16: 검색 기능 구현을 고려해보세요.현재는 UI만 제공하고 있지만, 실제 검색 기능을 위한 onChange 핸들러나 onSubmit 핸들러를 props로 받을 수 있도록 확장하는 것을 고려해보세요.
다음과 같이 확장할 수 있습니다:
interface SearchBarProps { placeholder: string; + onSearch?: (value: string) => void; + onChange?: (value: string) => void; }src/pages/group/Group.tsx (1)
130-141: 공통 스타일 컴포넌트 사용을 고려해보세요.현재 Wrapper 스타일이 다른 페이지에서도 재사용될 가능성이 높아 보입니다.
공통 컴포넌트로 분리하는 것을 고려해보세요:
+// src/components/common/PageWrapper.tsx +export const PageWrapper = styled.div` + display: flex; + position: relative; + flex-direction: column; + align-items: center; + justify-content: center; + min-width: 320px; + max-width: 767px; + height: 100%; + margin: 0 auto; + background-color: #121212; +`;src/components/group/RecruitingGroupCarousel.tsx (1)
62-66: ref 설정 방식을 최적화해주세요.현재 방식은 동작하지만 성능상 개선할 수 있습니다.
useCallback을 사용하여 불필요한 리렌더링을 방지해주세요:
+const setItemRef = useCallback((el: HTMLDivElement | null, index: number) => { + itemRefs.current[index] = el; +}, []); // 사용 시 <Item key={i} - ref={el => { - itemRefs.current[i] = el; - }} + ref={el => setItemRef(el, i)} >src/components/group/MyGroup.tsx (2)
29-31: 더 버튼의 onClick 핸들러를 구현해주세요.현재 빈 함수로 되어 있어 사용자 경험에 문제가 있을 수 있습니다.
다음과 같이 임시 처리하거나 실제 동작을 구현해주세요:
-<MoreButton onClick={() => {}}> +<MoreButton onClick={() => console.log('내 모임방 더보기')}>또는 페이지 라우팅이 필요한 경우:
+import { useNavigate } from 'react-router-dom'; +const navigate = useNavigate(); -<MoreButton onClick={() => {}}> +<MoreButton onClick={() => navigate('/my-groups')}>이 기능을 구현하는 이슈를 생성하시겠습니까?
74-82: 접근성을 위한 추가 속성을 고려해주세요.버튼의 접근성을 개선하기 위해 추가 속성을 제안합니다.
const MoreButton = styled.button` background: none; border: none; cursor: pointer; + padding: 4px; + border-radius: 4px; + &:focus { + outline: 2px solid var(--color-white); + outline-offset: 2px; + } > img { width: 24px; height: 24px; } `;그리고 컴포넌트에서:
-<MoreButton onClick={() => {}}> +<MoreButton onClick={() => {}} aria-label="내 모임방 더보기"> <img src={rightChevron} alt="내 모임방 버튼" /> </MoreButton>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (9)
src/assets/common/darkPeople.svgis excluded by!**/*.svgsrc/assets/common/leftArrow.svgis excluded by!**/*.svgsrc/assets/common/people.svgis excluded by!**/*.svgsrc/assets/common/right-Chevron.svgis excluded by!**/*.svgsrc/assets/header/bell.svgis excluded by!**/*.svgsrc/assets/header/findUser.svgis excluded by!**/*.svgsrc/assets/header/group-done.svgis excluded by!**/*.svgsrc/assets/header/header-logo.svgis excluded by!**/*.svgsrc/assets/searchBar/search.svgis excluded by!**/*.svg
📒 Files selected for processing (18)
src/components/common/Blank.tsx(1 hunks)src/components/common/IconButton.tsx(1 hunks)src/components/common/MainHeader.tsx(1 hunks)src/components/common/SearchBar.tsx(1 hunks)src/components/common/TitleHeader.tsx(1 hunks)src/components/common/Wrapper.tsx(1 hunks)src/components/group/MyGroup.tsx(1 hunks)src/components/group/MyGroupCard.tsx(1 hunks)src/components/group/RecruitingGroup.tsx(1 hunks)src/components/group/RecruitingGroupCard.tsx(1 hunks)src/components/group/RecruitingGroupCarousel.tsx(1 hunks)src/hooks/useInfiniteCarousel.ts(1 hunks)src/pages/group/Group.tsx(1 hunks)src/pages/index.tsx(2 hunks)src/pages/login/Login.tsx(1 hunks)src/pages/signup/SignupDone.tsx(1 hunks)src/pages/signup/SignupGenre.tsx(1 hunks)vite.config.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (8)
src/pages/index.tsx (1)
src/components/group/MyGroup.tsx (1)
Group(6-16)
src/components/group/MyGroupCard.tsx (1)
src/components/group/MyGroup.tsx (1)
Group(6-16)
src/components/group/RecruitingGroupCard.tsx (1)
src/components/group/MyGroup.tsx (1)
Group(6-16)
src/hooks/useInfiniteCarousel.ts (1)
src/components/group/MyGroup.tsx (1)
Group(6-16)
src/components/common/Blank.tsx (1)
src/components/common/Wrapper.tsx (1)
Wrapper(3-14)
src/components/common/SearchBar.tsx (1)
src/components/common/IconButton.tsx (1)
IconButton(3-7)
src/components/common/MainHeader.tsx (1)
src/components/common/IconButton.tsx (1)
IconButton(3-7)
src/components/group/RecruitingGroupCarousel.tsx (2)
src/components/group/MyGroup.tsx (1)
Group(6-16)src/components/group/RecruitingGroup.tsx (1)
RecruitingGroup(13-35)
🔇 Additional comments (23)
src/pages/signup/SignupGenre.tsx (1)
4-4: 에셋 경로 정리 승인에셋 파일을
common폴더로 이동하여 구조를 정리한 것이 좋습니다. 프로젝트의 에셋 관리 개선에 도움이 됩니다.src/pages/signup/SignupDone.tsx (1)
3-3: 에셋 경로 정리 승인다른 signup 페이지와 동일한 에셋 경로 정리가 일관성 있게 적용되었습니다.
src/components/common/TitleHeader.tsx (1)
4-4: CSS 변수 활용 개선 승인하드코딩된 색상 값을 CSS 변수로 변경한 것이 훌륭합니다. 테마 관리와 일관성 측면에서 좋은 개선사항입니다.
src/components/common/Wrapper.tsx (1)
3-14: 공통 Wrapper 컴포넌트 구현이 잘 되어 있습니다.레이아웃 컨테이너로서 적절한 flexbox 속성과 반응형 디자인 고려사항이 잘 구현되어 있습니다. 최소/최대 너비 설정과 중앙 정렬 로직이 모바일 우선 설계에 적합합니다.
src/components/common/Blank.tsx (1)
9-11: 컴포넌트 구현이 깔끔하고 유용합니다.스페이서 컴포넌트로서 적절한 props 인터페이스와 기본값 설정이 잘 되어 있습니다. CSS 변수 사용도 좋은 접근입니다.
src/hooks/useInfiniteCarousel.ts (1)
4-4: CLONE_COUNT 상수 정의가 적절합니다.무한 스크롤을 위한 복제 횟수를 상수로 정의하여 유지보수성을 높였습니다.
src/pages/login/Login.tsx (1)
5-5: 공통 컴포넌트 사용으로 코드 중복을 제거했습니다.로컬 Wrapper 컴포넌트를 공통 컴포넌트로 교체하여 코드 일관성과 유지보수성을 향상시켰습니다.
src/pages/index.tsx (2)
12-12: 새로운 Group 컴포넌트 import가 적절합니다.그룹 기능을 위한 컴포넌트 import가 깔끔하게 구현되었습니다.
24-24: 새로운 그룹 라우트 추가가 적절합니다.기존 라우팅 구조와 일관성을 유지하면서 새로운 그룹 페이지 라우트를 추가했습니다.
src/components/group/RecruitingGroupCard.tsx (2)
1-4: import 구문이 깔끔하게 정리되어 있습니다.필요한 모듈들이 명확하게 import되어 있고, 타입 import도 적절하게 분리되어 있습니다.
10-27: 컴포넌트 구조가 잘 설계되어 있습니다.forwardRef를 사용하여 ref 전달이 가능하고, 컴포넌트 구조가 명확합니다. 카드 레이아웃도 사용자 친화적으로 잘 구성되어 있습니다.
src/components/common/MainHeader.tsx (3)
8-10: 타입 정의가 명확합니다.MainHeaderProps 인터페이스가 명확하게 정의되어 있고, union type을 사용하여 허용되는 값들을 제한했습니다.
17-21: 조건부 렌더링 로직이 명확합니다.type prop에 따라 다른 아이콘과 alt text를 렌더링하는 로직이 명확하고 이해하기 쉽습니다. 접근성을 위한 alt text도 적절하게 제공되어 있습니다.
29-41: 헤더 스타일링이 적절합니다.고정 헤더로 설정되어 있고, 최대 너비 제한과 중앙 정렬이 잘 구현되어 있습니다. z-index도 적절하게 설정되어 있습니다.
src/components/group/RecruitingGroup.tsx (2)
87-97: 반응형 그리드 레이아웃이 잘 구현되어 있습니다.모바일에서는 1열, 더 큰 화면에서는 2열로 표시되는 반응형 디자인이 잘 구현되어 있습니다.
16-16: 필터링 로직에서 안전한 처리가 필요합니다.Group 인터페이스에서
genre는 optional 필드이지만 필터링에서 직접 비교하고 있습니다. genre가 undefined인 경우를 처리해야 합니다.다음과 같이 수정하여 안전하게 처리하세요:
- const filtered = useMemo(() => groups.filter(g => g.genre === selected), [groups, selected]); + const filtered = useMemo(() => groups.filter(g => g.genre === selected), [groups, selected]);또는 더 안전하게:
- const filtered = useMemo(() => groups.filter(g => g.genre === selected), [groups, selected]); + const filtered = useMemo(() => groups.filter(g => g.genre && g.genre === selected), [groups, selected]);Likely an incorrect or invalid review comment.
src/components/group/MyGroupCard.tsx (2)
35-45: 카드 스타일링이 잘 구현되어 있습니다.flex 레이아웃과 그라데이션 배경, 트랜지션 효과가 잘 구현되어 있습니다. scroll-snap과 will-change 속성도 적절하게 사용되어 성능을 고려한 것 같습니다.
64-72: 텍스트 오버플로우 처리가 적절합니다.제목이 길 경우 ellipsis 처리가 되어 있어 UI가 깨지지 않도록 잘 처리되어 있습니다.
src/pages/group/Group.tsx (1)
115-126: 컴포넌트 구조가 깔끔하게 잘 구성되어 있습니다.그룹 페이지의 레이아웃이 논리적으로 잘 구성되어 있고, 각 컴포넌트의 역할이 명확하게 분리되어 있습니다.
src/components/group/RecruitingGroupCarousel.tsx (2)
19-34: 스크롤 핸들러 로직이 잘 구현되어 있습니다.useCallback을 사용한 최적화와 수학적 계산이 정확하게 구현되어 있습니다. 스케일링 효과도 자연스럽게 적용될 것 같습니다.
50-57: 이벤트 리스너 관리가 잘 구현되어 있습니다.passive 이벤트 리스너 사용과 cleanup이 올바르게 처리되어 있습니다.
src/components/group/MyGroup.tsx (2)
22-23: 커스텀 훅을 사용한 로직 분리가 훌륭합니다.복잡한 무한 캐러셀 로직을 커스텀 훅으로 분리하여 컴포넌트의 가독성과 재사용성을 높였습니다.
44-48: 점 표시기 구현이 잘 되어 있습니다.current 상태와 연동하여 현재 위치를 직관적으로 표시하는 로직이 잘 구현되어 있습니다.
| let closestIdx = -1; | ||
| let minDist = Infinity; | ||
|
|
||
| cardRefs.current.forEach((card, index) => { | ||
| if (!card) return; | ||
| const cardCenter = card.offsetLeft + card.offsetWidth / 2; | ||
| const distance = Math.abs(center - (cardCenter - scrollLeft)); | ||
| const scale = Math.max(0.83, 1 - (distance / center) * 0.17); | ||
| card.style.transform = `scale(${scale})`; | ||
|
|
||
| if (distance < minDist) { | ||
| minDist = distance; | ||
| closestIdx = index; | ||
| } | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
스케일링 로직에서 성능 최적화가 필요합니다.
모든 카드에 대해 매번 DOM 스타일을 직접 조작하는 것은 성능상 비효율적입니다. requestAnimationFrame을 사용하거나 스케일링 로직을 최적화하는 것을 고려해보세요.
cardRefs.current.forEach((card, index) => {
if (!card) return;
const cardCenter = card.offsetLeft + card.offsetWidth / 2;
const distance = Math.abs(center - (cardCenter - scrollLeft));
const scale = Math.max(0.83, 1 - (distance / center) * 0.17);
- card.style.transform = `scale(${scale})`;
+
+ // 성능 최적화를 위해 transform 변경 최소화
+ if (Math.abs(parseFloat(card.style.transform.replace(/[^\d.]/g, '') || '1') - scale) > 0.01) {
+ card.style.transform = `scale(${scale})`;
+ }
if (distance < minDist) {
minDist = distance;
closestIdx = index;
}
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| let closestIdx = -1; | |
| let minDist = Infinity; | |
| cardRefs.current.forEach((card, index) => { | |
| if (!card) return; | |
| const cardCenter = card.offsetLeft + card.offsetWidth / 2; | |
| const distance = Math.abs(center - (cardCenter - scrollLeft)); | |
| const scale = Math.max(0.83, 1 - (distance / center) * 0.17); | |
| card.style.transform = `scale(${scale})`; | |
| if (distance < minDist) { | |
| minDist = distance; | |
| closestIdx = index; | |
| } | |
| }); | |
| let closestIdx = -1; | |
| let minDist = Infinity; | |
| cardRefs.current.forEach((card, index) => { | |
| if (!card) return; | |
| const cardCenter = card.offsetLeft + card.offsetWidth / 2; | |
| const distance = Math.abs(center - (cardCenter - scrollLeft)); | |
| const scale = Math.max(0.83, 1 - (distance / center) * 0.17); | |
| // 성능 최적화를 위해 transform 변경 최소화 | |
| if ( | |
| Math.abs( | |
| parseFloat(card.style.transform.replace(/[^\d.]/g, "") || "1") | |
| - scale | |
| ) > 0.01 | |
| ) { | |
| card.style.transform = `scale(${scale})`; | |
| } | |
| if (distance < minDist) { | |
| minDist = distance; | |
| closestIdx = index; | |
| } | |
| }); |
🤖 Prompt for AI Agents
In src/hooks/useInfiniteCarousel.ts around lines 24 to 38, the current scaling
logic directly manipulates the DOM style for every card on each call, which is
inefficient. To fix this, wrap the scaling and style updates inside a
requestAnimationFrame callback to batch DOM updates and improve performance.
This ensures the style changes are applied optimally within the browser's
rendering cycle.
| useEffect(() => { | ||
| const container = scrollRef.current; | ||
| if (!container || sections.length === 0) return; | ||
|
|
||
| const mid = Math.floor(sections.length / 2); | ||
| const midItem = itemRefs.current[mid]; | ||
| if (midItem) { | ||
| const centerX = container.offsetWidth / 2; | ||
| const targetScroll = midItem.offsetLeft + midItem.offsetWidth / 2 - centerX; | ||
| container.scrollTo({ left: targetScroll, behavior: 'auto' }); | ||
| } | ||
| handleScroll(); | ||
| }, [sections.length, handleScroll]); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
초기 스크롤 위치 설정 로직을 개선해주세요.
현재 로직은 올바르지만 예외 처리와 성능 최적화가 필요합니다.
다음과 같이 개선하는 것을 권장합니다:
useEffect(() => {
const container = scrollRef.current;
- if (!container || sections.length === 0) return;
+ if (!container || sections.length === 0) return;
+
+ // 레이아웃 계산이 완료된 후 실행
+ const timer = setTimeout(() => {
const mid = Math.floor(sections.length / 2);
const midItem = itemRefs.current[mid];
if (midItem) {
const centerX = container.offsetWidth / 2;
const targetScroll = midItem.offsetLeft + midItem.offsetWidth / 2 - centerX;
container.scrollTo({ left: targetScroll, behavior: 'auto' });
}
handleScroll();
+ }, 0);
+
+ return () => clearTimeout(timer);
}, [sections.length, handleScroll]);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| useEffect(() => { | |
| const container = scrollRef.current; | |
| if (!container || sections.length === 0) return; | |
| const mid = Math.floor(sections.length / 2); | |
| const midItem = itemRefs.current[mid]; | |
| if (midItem) { | |
| const centerX = container.offsetWidth / 2; | |
| const targetScroll = midItem.offsetLeft + midItem.offsetWidth / 2 - centerX; | |
| container.scrollTo({ left: targetScroll, behavior: 'auto' }); | |
| } | |
| handleScroll(); | |
| }, [sections.length, handleScroll]); | |
| useEffect(() => { | |
| const container = scrollRef.current; | |
| if (!container || sections.length === 0) return; | |
| // 레이아웃 계산이 완료된 후 실행 | |
| const timer = setTimeout(() => { | |
| const mid = Math.floor(sections.length / 2); | |
| const midItem = itemRefs.current[mid]; | |
| if (midItem) { | |
| const centerX = container.offsetWidth / 2; | |
| const targetScroll = midItem.offsetLeft + midItem.offsetWidth / 2 - centerX; | |
| container.scrollTo({ left: targetScroll, behavior: 'auto' }); | |
| } | |
| handleScroll(); | |
| }, 0); | |
| return () => clearTimeout(timer); | |
| }, [sections.length, handleScroll]); |
🤖 Prompt for AI Agents
In src/components/group/RecruitingGroupCarousel.tsx around lines 36 to 48,
improve the initial scroll position logic by adding null checks for
itemRefs.current and ensuring mid index is within bounds to prevent runtime
errors. Also, debounce or throttle the handleScroll call if it triggers
expensive operations to optimize performance. Refactor the useEffect to minimize
unnecessary executions by verifying dependencies and adding early returns for
edge cases.
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
src/components/group/RecruitingGroupBox.tsx (1)
11-11: 하드코딩된 장르 배열을 상수로 분리하는 것이 좋습니다.GENRE 배열이 하드코딩되어 있어 유지보수성이 떨어집니다.
별도의 상수 파일로 분리하거나 서버에서 가져오는 것을 고려해보세요:
// constants/genres.ts export const GENRES = ['문학', '과학·IT', '사회과학', '인문학', '예술'] as const; export type Genre = typeof GENRES[number];
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
src/components/group/GroupCard.tsx(1 hunks)src/components/group/MyGroupBox.tsx(1 hunks)src/components/group/MyGroupCard.tsx(1 hunks)src/components/group/RecruitingGroupBox.tsx(1 hunks)src/components/group/RecruitingGroupCarousel.tsx(1 hunks)src/hooks/useInfiniteCarousel.ts(1 hunks)src/pages/group/Group.tsx(1 hunks)src/pages/group/MyGroup.tsx(1 hunks)src/pages/index.tsx(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/pages/group/MyGroup.tsx
🚧 Files skipped from review as they are similar to previous changes (5)
- src/pages/index.tsx
- src/components/group/MyGroupCard.tsx
- src/hooks/useInfiniteCarousel.ts
- src/pages/group/Group.tsx
- src/components/group/RecruitingGroupCarousel.tsx
🧰 Additional context used
🧬 Code Graph Analysis (2)
src/components/group/GroupCard.tsx (1)
src/components/group/MyGroupBox.tsx (1)
Group(6-16)
src/components/group/RecruitingGroupBox.tsx (2)
src/components/group/MyGroupBox.tsx (1)
Group(6-16)src/components/group/GroupCard.tsx (1)
GroupCard(11-32)
🔇 Additional comments (6)
src/components/group/MyGroupBox.tsx (3)
6-16: Group 인터페이스 타입 정의가 적절합니다.선택적 필드들이 명확하게 정의되어 있고, 그룹 데이터 구조를 잘 표현하고 있습니다.
84-94: 접근성 개선을 위한 스크롤 설정이 우수합니다.스크롤바 숨김 처리와 scroll-snap 기능이 잘 구현되어 사용자 경험을 향상시킵니다.
34-42: 캐러셀 렌더링 로직에서 잠재적 성능 이슈가 있습니다.
infiniteGroups.map()에서key={g.id}-${i}를 사용하고 있는데, 무한 스크롤 시 동일한 그룹이 여러 번 렌더링되면서 key 중복 문제가 발생할 수 있습니다.- key={`${g.id}-${i}`} + key={`${g.id}-${Math.floor(i / groups.length)}-${i % groups.length}`}또는 useInfiniteCarousel 훅에서 고유한 식별자를 제공하는 것을 권장합니다.
Likely an incorrect or invalid review comment.
src/components/group/GroupCard.tsx (2)
61-71: 텍스트 오버플로우 처리가 잘 구현되어 있습니다.
word-break: keep-all과text-overflow: ellipsis조합으로 한국어 텍스트 처리가 적절히 되어 있습니다.
97-107: 타입별 데드라인 스타일링이 명확합니다.recruiting과 ongoing 타입에 따른 색상 구분이 사용자에게 명확한 정보를 제공합니다.
src/components/group/RecruitingGroupBox.tsx (1)
94-96: 반응형 그리드 레이아웃이 잘 구현되어 있습니다.미디어 쿼리를 사용한 반응형 그리드 설정이 적절하며, 브레이크포인트 선택도 합리적입니다.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
src/components/group/MyGroupModal.tsx (1)
13-86: 더미 데이터를 별도 파일로 분리하고 URL 중복을 제거해주세요.현재 모든 그룹에서 동일한 긴 Canva URL을 사용하여 코드 가독성이 떨어지고 실제 환경과 차이가 있습니다.
별도의 상수 파일로 분리하여 관리하는 것을 권장합니다:
+// src/data/mockData.ts +export const dummyMyGroups: Group[] = [ + { + id: '1', + title: '시집만 읽는 사람들 3월', + participants: 15, + coverUrl: 'https://example.com/cover1.jpg', + // ... 다른 속성들 + }, + // ... 서로 다른 coverUrl을 가진 다른 그룹들 +];그리고 메인 파일에서 import하여 사용:
+import { dummyMyGroups } from '@/data/mockData'; -const dummyMyGroups: Group[] = [ - // ... 긴 더미 데이터 -];src/pages/group/Group.tsx (1)
13-110: 더미 데이터를 별도 파일로 분리해주세요.MyGroupModal.tsx와 동일한 문제로 긴 URL이 반복되어 코드 가독성을 저해하고 있습니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
src/components/common/MainHeader.tsx(1 hunks)src/components/group/CompletedGroupModal.tsx(1 hunks)src/components/group/MyGroupBox.tsx(1 hunks)src/components/group/MyGroupModal.tsx(1 hunks)src/pages/group/Group.tsx(1 hunks)src/pages/index.tsx(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/components/group/CompletedGroupModal.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- src/pages/index.tsx
- src/components/common/MainHeader.tsx
🧰 Additional context used
🧬 Code Graph Analysis (2)
src/pages/group/Group.tsx (4)
src/components/group/RecruitingGroupCarousel.tsx (2)
Section(6-9)RecruitingGroupCarousel(15-73)src/components/group/MyGroupBox.tsx (2)
Group(6-17)MyGroupBox(24-53)src/components/common/Wrapper.tsx (1)
Wrapper(3-14)src/components/group/MyGroupModal.tsx (1)
MyGroupModal(88-127)
src/components/group/MyGroupModal.tsx (2)
src/components/group/MyGroupBox.tsx (1)
Group(6-17)src/components/group/GroupCard.tsx (1)
GroupCard(11-32)
🔇 Additional comments (4)
src/components/group/MyGroupModal.tsx (1)
88-127: 모달 컴포넌트 구현이 잘 되어 있습니다.필터링 탭 기능과 상태 관리가 적절히 구현되어 있고, 토글 방식의 필터링도 사용자 경험을 고려한 좋은 구현입니다.
src/pages/group/Group.tsx (1)
118-141: 페이지 컴포넌트 구성이 잘 되어 있습니다.여러 컴포넌트를 조합하여 페이지를 구성하고, 모달 상태 관리가 적절히 구현되어 있습니다.
src/components/group/MyGroupBox.tsx (2)
24-53: 무한 캐러셀 구현이 잘 되어 있습니다.
useInfiniteCarousel훅을 사용하여 복잡한 로직을 분리하고, 컴포넌트 구조가 명확하게 구성되어 있습니다. 이전 리뷰에서 지적된 빈 onClick 핸들러 문제도 올바르게 해결되었습니다.
86-111: 캐러셀 스타일링이 적절합니다.스크롤 스냅 기능과 숨겨진 스크롤바, 부드러운 스크롤 동작이 잘 구현되어 있어 사용자 경험이 향상될 것으로 보입니다.
| const Wrapper = styled.div` | ||
| display: flex; | ||
| position: relative; | ||
| flex-direction: column; | ||
| align-items: center; | ||
| justify-content: center; | ||
| min-width: 320px; | ||
| max-width: 767px; | ||
| height: 100%; | ||
| margin: 0 auto; | ||
| background-color: #121212; | ||
| `; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
중복된 Wrapper 컴포넌트를 제거해주세요.
이미 src/components/common/Wrapper.tsx에 동일한 컴포넌트가 존재합니다. 중복 정의를 제거하고 공통 컴포넌트를 사용하세요.
다음과 같이 수정하세요:
+import { Wrapper } from '@/components/common/Wrapper';
-const Wrapper = styled.div`
- display: flex;
- position: relative;
- flex-direction: column;
- align-items: center;
- justify-content: center;
- min-width: 320px;
- max-width: 767px;
- height: 100%;
- margin: 0 auto;
- background-color: #121212;
-`;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const Wrapper = styled.div` | |
| display: flex; | |
| position: relative; | |
| flex-direction: column; | |
| align-items: center; | |
| justify-content: center; | |
| min-width: 320px; | |
| max-width: 767px; | |
| height: 100%; | |
| margin: 0 auto; | |
| background-color: #121212; | |
| `; | |
| // At the top of src/pages/group/Group.tsx, replace the local Wrapper definition with: | |
| import { Wrapper } from '@/components/common/Wrapper'; |
🤖 Prompt for AI Agents
In src/pages/group/Group.tsx around lines 143 to 154, there is a duplicated
Wrapper styled component that already exists in
src/components/common/Wrapper.tsx. Remove the local Wrapper definition and
instead import and use the common Wrapper component from
src/components/common/Wrapper.tsx to avoid duplication and maintain consistency.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
src/components/group/CompletedGroupModal.tsx (3)
94-94: GroupCard 컴포넌트의 일관성을 개선하세요.완료된 그룹에 대한 적절한 타입을 전달하고 자체 닫힌 태그를 사용하는 것이 좋습니다.
-<GroupCard key={group.id} group={group}></GroupCard> +<GroupCard key={group.id} group={group} type="completed" />참고:
GroupCard컴포넌트가 완료된 그룹에 대한 타입을 지원하지 않는다면, 컴포넌트를 확장하여 완료된 그룹에 대한 적절한 UI를 표시하도록 고려해보세요.
113-113: 모달 배경을 반투명으로 변경하는 것을 고려하세요.완전히 불투명한 배경보다는 반투명한 배경이 더 자연스러운 사용자 경험을 제공합니다.
- background: rgba(0, 0, 0, 1); + background: rgba(0, 0, 0, 0.8);
82-102: 전반적으로 잘 구성된 컴포넌트입니다.모달 구조와 반응형 레이아웃이 적절하게 구현되어 있습니다. 하지만 프로덕션 준비를 위해 위에서 언급한 개선사항들을 적용하는 것을 권장합니다.
추가로 고려해볼 점:
- 키보드 네비게이션 지원 (ESC키로 모달 닫기)
- 모달 외부 클릭 시 닫기 기능
- 로딩 상태 처리
- 에러 상태 처리
이러한 기능들을 구현하는 데 도움이 필요하시면 언제든 요청해주세요.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/components/group/CompletedGroupModal.tsx(1 hunks)src/components/group/GroupCard.tsx(1 hunks)src/components/group/MyGroupModal.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/components/group/MyGroupModal.tsx
- src/components/group/GroupCard.tsx
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/components/group/CompletedGroupModal.tsx (2)
src/components/group/MyGroupBox.tsx (1)
Group(6-17)src/components/group/GroupCard.tsx (1)
GroupCard(11-32)
🔇 Additional comments (1)
src/components/group/CompletedGroupModal.tsx (1)
1-9: 임포트와 인터페이스 정의가 적절합니다.타입 재사용과 필요한 의존성 임포트가 잘 구성되어 있습니다.
| <Overlay> | ||
| <Modal> | ||
| <TitleHeader | ||
| title="완료된 모임방" | ||
| leftIcon={<img src={leftArrow} alt="뒤로 가기" />} | ||
| onLeftClick={onClose} | ||
| /> | ||
| <Text>{userName}님이 참여했던 모임방들을 확인해보세요.</Text> | ||
| <Content> | ||
| {dummyCompletedGroups.map(group => ( | ||
| <GroupCard key={group.id} group={group}></GroupCard> | ||
| ))} | ||
| </Content> | ||
| </Modal> | ||
| </Overlay> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
모달의 접근성을 개선하세요.
모달 컴포넌트에 접근성 속성이 부족합니다.
<Overlay>
- <Modal>
+ <Modal role="dialog" aria-labelledby="modal-title" aria-modal="true">
<TitleHeader
+ id="modal-title"
title="완료된 모임방"
leftIcon={<img src={leftArrow} alt="뒤로 가기" />}
onLeftClick={onClose}
/>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <Overlay> | |
| <Modal> | |
| <TitleHeader | |
| title="완료된 모임방" | |
| leftIcon={<img src={leftArrow} alt="뒤로 가기" />} | |
| onLeftClick={onClose} | |
| /> | |
| <Text>{userName}님이 참여했던 모임방들을 확인해보세요.</Text> | |
| <Content> | |
| {dummyCompletedGroups.map(group => ( | |
| <GroupCard key={group.id} group={group}></GroupCard> | |
| ))} | |
| </Content> | |
| </Modal> | |
| </Overlay> | |
| <Overlay> | |
| <Modal role="dialog" aria-labelledby="modal-title" aria-modal="true"> | |
| <TitleHeader | |
| id="modal-title" | |
| title="완료된 모임방" | |
| leftIcon={<img src={leftArrow} alt="뒤로 가기" />} | |
| onLeftClick={onClose} | |
| /> | |
| <Text>{userName}님이 참여했던 모임방들을 확인해보세요.</Text> | |
| <Content> | |
| {dummyCompletedGroups.map(group => ( | |
| <GroupCard key={group.id} group={group}></GroupCard> | |
| ))} | |
| </Content> | |
| </Modal> | |
| </Overlay> |
🤖 Prompt for AI Agents
In src/components/group/CompletedGroupModal.tsx around lines 84 to 98, the modal
lacks accessibility attributes. Add appropriate ARIA roles such as role="dialog"
and aria-modal="true" to the Modal component, ensure the TitleHeader has an
accessible label or heading role, and manage focus by setting initial focus
inside the modal and trapping focus while it is open to improve keyboard
navigation and screen reader support.
| export interface Group { | ||
| id: string; | ||
| title: string; | ||
| participants: number; | ||
| maximumParticipants?: number; | ||
| userName?: string; | ||
| progress?: number; | ||
| coverUrl: string; | ||
| deadLine?: number; | ||
| genre?: string; | ||
| isOnGoing?: boolean; | ||
| } |
There was a problem hiding this comment.
지금 Group 인터페이스가 여러 파일에서 중복 정의되어있고, 옵셔널 필드가 많아서 타입 안정성이 떨어질 것 같아요.
types/group.ts 파일을 별도로 만들어서 컨텍스트별로 명확하게 타입을 정의하는건 어떨까요?
There was a problem hiding this comment.
동의합니다! 이 부분은 사실 API 연동하면서 데이터별로 포맷팅 진행할 예정이라 우선 많은 부분을 수용하도록 구현되어 있습니다.
| const handleScroll = useCallback(() => { | ||
| const container = scrollRef.current; | ||
| if (!container) return; | ||
|
|
||
| const centerX = container.offsetWidth / 2; | ||
| const scrollLeft = container.scrollLeft; | ||
|
|
||
| itemRefs.current.forEach(item => { | ||
| if (!item) return; | ||
| const itemCenter = item.offsetLeft + item.offsetWidth / 2; | ||
| const dist = Math.abs(itemCenter - scrollLeft - centerX); | ||
| const ratio = Math.min(dist / centerX, 1); | ||
| const scale = 1 - ratio * 0.1; | ||
| item.style.transform = `scale(${scale})`; | ||
| }); | ||
| }, []); |
There was a problem hiding this comment.
스케일 애니메이션 로직이 스크롤 핸들러 안에 들어있는 이유가 있나요?! 관심사 분리를 해주면 좋을 것 같아요. 그리고 매 스크롤마다 모든 아이템을 순회하면서 DOM 조작을 하고 있어 보여요.
일단 스케일 애니메이션 로직을 useScaleAnimation 같은 별도 훅으로 분리하는건 어떨까요? 그리고 현재 style.transform을 직접 조작하고 있는데, requestAnimationFrame을 사용해서 브라우저 렌더링 사이클에 맞춰서 애니메이션을 처리해보는 것도 좀 더 부드러운 애니메이션을 만들 수 있을 것 같습니다!
그리고 중앙에서 거리 계산 로직도 별도 함수로 빼내고, easing 함수로 좀 더 자연스러운 애니메이션을 만들 수도 있을 것 같아용
https://inpa.tistory.com/entry/%F0%9F%8C%90-requestAnimationFrame-%EA%B0%80%EC%9D%B4%EB%93%9C
There was a problem hiding this comment.
스케일 애니메이션 로직이 스크롤 핸들러 안에 들어있는 이유가 있나요?!
디자인 요구사항을 보면 스크롤이 변경됨에 따라 스케일이 실시간으로 함께 변경되는걸 요구하고 있어서 이렇게 구현했습니다! 또한, 각각의 요소에서 크기 변화를 요구하기에 이렇게 구현했습니다.
requestAnimationFrame을 사용해서 브라우저 렌더링 사이클에 맞춰서 애니메이션을 처리해보는 것도 좀 더 부드러운 애니메이션을 만들 수 있을 것 같습니다!
이 부분은 조금 더 자세히 설명해주실수 있나요?! 브라우저 렌더링 사이클에 맞춰서 애니메이션 처리를 어떻게 하면 좋을까요?
사실 디자인 요구사항에 맞추려고 직접 구현했는데 성능상으로 문제가 생긴다면 라이브러리를 사용해보는 것도 좋을 것 같다는 생각이 드네요!
좋은 의견 감사합니다!
| type?: 'recruiting' | 'ongoing'; | ||
| } | ||
|
|
||
| export const GroupCard = forwardRef<HTMLDivElement, Props>(({ group, type }, ref) => { |
There was a problem hiding this comment.
여기서 forwardRef를 사용하고 있는 특별한 이유가 있나요? 무한 캐러셀 아니면 스크롤 애니메이션 때문인 것 같은데 카드 컴포넌트가 표시, DOM 참조 제공 동시에 두 가지 책임을 가지고 있는 것 같아요! ref랑 type props가 섞여서 복잡할 것 같고 다른 곳에서 GroupCard를 사용할 일이 있을지는 봐야겠지만? 항상 ref를 고려해야 할 것 같아서 data attribute 같은걸 사용해보는건 어떨까용? 의존성 제거도 좋을 것 같아요.
There was a problem hiding this comment.
좋은 의견 감사합니다! 저도 개선의 필요성이 느껴지네요!
There was a problem hiding this comment.
CompletedGroupModal이랑 MyGroupModal이랑 구조가 거의 동일한데 BaseModal 같은 컴포넌트를 만들어서 공통 레이아웃을 처리하고, 각 모달은 내용 부분만 children으로 받는 구조는 어떨까요? 탭 기능도 useTabs 훅으로 분리할 수도 있을 것 같아요.
There was a problem hiding this comment.
CompletedGroupModal이랑 MyGroupModal이랑 구조가 거의 동일한데 BaseModal 같은 컴포넌트를 만들어서 공통 레이아웃을 처리하고 , 각 모달은 내용 부분만 children으로 받는 구조는 어떨까요?
각 모달의 디자인 요소가 제법 다른게 첫 번째 이유고 두 번째는 두 모달에서 사용하고 있는 데이터가 달라서(내 모임방에서 3가지 데이터를 더 필요로함) 결국 API를 받아오고 컴포넌트에 뿌릴때 다르게 처리해야 합니다. 때문에 컴포넌트 분리시 더 복잡해진다고 판단해서 각각 구현했습니다.
탭 기능도 useTabs 훅으로 분리할 수도 있을 것 같아요.
해당 화면에서 사용하고 있는 탭은 서버로부터 받아오는 것이 아닌 프론트에서 저장해서 사용할 고정된 탭입니다. 또한, 단순 상태로 선언되어 관리되고 있기 때문에 커스텀 훅으로 분리할 필요성을 못 느꼈습니다.
자세한 리뷰 감사합니다! 제가 설계한 의도는 위와 같은데 혹시 추가적인 의견 있으면 남겨주시면 감사하겠습니다.
3 participants
#️⃣연관된 이슈
[UI] 모임방 메인 페이지 #18
[UI] 내 모임방 페이지, 완료된 모임방 페이지 #31
📝작업 내용
group-main 페이지 구현을 진행했습니다.
스크린샷 (선택)
모임 메인 페이지
2025-07-08.12.15.35.mov
영상을 보시면 화면 늘렸을때 양 옆이 동일하게 늘어나지는 않습니다. 그러나 동영상처럼 양 옆 크기가 저렇게 연속적이고 동적으로 변할 상황은 없다고 판단했습니다. 때문에 화면 렌더링 시에 적절한 위치로 자리 잡도록 구현해 두었습니다.
완료된 모임방 모달
내 모임방 모달
💬리뷰 요구사항(선택)
생각보다 복잡한 로직이 많이 들어가게 되어서 가능하시다면 로직 개선점 한번 같이 봐주시면 좋을 것 같아요~
Summary by CodeRabbit