refactor-skeleton-global: 주요 페이지 전역 Skeleton UI 적용 및 로딩 경험 개선#308
Merged
ljh130334 merged 24 commits intorefactor-skeletonfrom Feb 25, 2026
Merged
refactor-skeleton-global: 주요 페이지 전역 Skeleton UI 적용 및 로딩 경험 개선#308ljh130334 merged 24 commits intorefactor-skeletonfrom
ljh130334 merged 24 commits intorefactor-skeletonfrom
Conversation
- 기존 빈 Fragment(A) → FeedPostSkeleton 3개 표시(B)로 로딩 UX 개선 - 헤더/NavBar 유지한 채로 스켈레톤 영역만 교체 - 최소 0.5초 로딩 보장으로 깜빡임 방지
- OtherFeedSkeleton 신규 생성: 프로필+팔로워+게시물 영역 포함, paddingTop prop으로 헤더 높이 대응 - Feed 내 피드 탭: FeedPostSkeleton → OtherFeedSkeleton(paddingTop=136)으로 교체 - MyFeed 프로필 로딩 중 빈 화면(<>) → OtherFeedSkeleton으로 교체 - MyFeedPage: getOtherProfile → getMyProfile 교체로 프로필 미노출 버그 수정
- A: 평탄한 구조(9개 파일 혼재), Profile/TotalBar 로직이 OtherFeedSkeleton에만 결합 - B: base/feed 도메인 분리 + ProfileSkeleton/TotalBarSkeleton 독립 추출 → 조합형 설계 - 재사용성: Profile/TotalBar 스켈레톤 독립 사용 가능, import 경로 간소화 (개별 경로 → '@/shared/ui/Skeleton')
- A: 로딩 중 빈 화면(<>) 표시 - B: FeedPostSkeleton + 댓글 스켈레톤(Circle+Text) × 5 표시 - 최소 0.5초 로딩으로 깜빡임 방지, 레이아웃 사전 노출로 CLS 개선
- A는 범용 LoadingSpinner → 탭 전환 시 콘텐츠 구조 미리보기 불가 - B는 탭별 스켈레톤(피드: FeedPostSkeleton, 책: BookSkeletonItem) → 레이아웃 미리 표시 - 최소 500ms 로딩으로 깜빡임 방지
- A는 LoadingSpinner로 전체 페이지 로딩 표시 → 메뉴 구조 확인 불가 - B는 프로필만 스켈레톤, 고정 메뉴는 즉시 표시 → 로딩 중에도 페이지 탐색 가능 - 최소 500ms 로딩으로 프로필 스켈레톤 깜빡임 방지
- A는 LoadingSpinner 중앙 표시 → 메시지 구조 미리보기 불가 - B는 날짜 구분선 + 메시지 4개 스켈레톤 → 실제 메시지 레이아웃 미리 표시 - 최소 500ms 로딩으로 초기 로드 시 깜빡임 방지
- 전체 페이지 로딩(A) → 고정 UI 즉시 표시 + 콘텐츠만 스켈레톤(B) - BannerSkeleton/BookSkeleton 분리로 섹션별 독립 제어 - 깜빡임 방지 위해 최소 500ms 로딩 + null 안전성 강화
- 전체 로딩(A) → 고정 UI + 콘텐츠 스켈레톤(B)으로 개선 - BannerSkeleton으로 배너 영역, 인라인 스켈레톤으로 책 정보 표시 - 최소 500ms 로딩으로 깜빡임 방지, IIFE로 타입 안전성 확보
- 로딩 없음(A) → 탭/필터 유지 + RecordItem 스켈레톤(B) - RecordItemSkeleton으로 프로필/콘텐츠/액션 영역 구조 유지 - 최소 500ms 로딩으로 깜빡임 방지, 탭/필터 조작 가능 상태 유지
- A: LoadingSpinner/텍스트로 로딩 표시 → 레이아웃 점프 발생 - B: 실제 컴포넌트 구조 기반 스켈레톤 + 500ms 최소 로딩 → 레이아웃 안정성 - Memory 4개, SearchBook 피드 3개 스켈레톤으로 깜빡임 방지 및 UX 개선
- A: LoadingSpinner/텍스트 로딩 표시 → 레이아웃 점프 및 깜빡임 발생 - B: 실제 컴포넌트 구조 기반 스켈레톤 + 500ms 최소 로딩 → 안정적 UX - Memory 3개, Search 5개, SearchBook 피드 3개 스켈레톤으로 일관된 로딩 경험 제공
- 기존: LoadingMessage 텍스트로 로딩 표시 - 개선: 검색 결과는 GroupCardSkeleton(5개), 최근 검색어는 RecentSearchTabsSkeleton으로 구조 일치 - 500ms 최소 로딩으로 깜빡임 방지 및 로딩 상태 일관성 확보
- 기존: 로딩 중... 텍스트로 표시 - 개선: MemberListSkeleton(5개)로 실제 멤버 리스트 구조와 동일하게 표시 - 500ms 최소 로딩으로 깜빡임 방지 및 일관된 로딩 경험 제공
- 기존: LoadingSpinner로 전체 화면 로딩 표시 - 개선: UserProfileItemSkeleton(5개)로 실제 프로필 리스트 구조와 동일하게 표시 - 초기 로딩 시 500ms 최소 로딩으로 깜빡임 방지 및 일관된 UX 제공
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (2)
Walkthrough다양한 페이지와 컴포넌트에 스켈레톤 기반 로딩 UI를 도입하고, 로딩 표시 일관화를 위해 Promise.all로 최소 500ms의 인위적 지연을 병렬 처리하도록 변경했습니다. Changes
Sequence Diagram(s)(생성 조건 미충족 — 생략) Estimated code review effort🎯 4 (Complex) | ⏱️ ~55분 Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
src/pages/groupMembers/GroupMembers.tsx (1)
24-30:⚠️ Potential issue | 🟡 Minor
!currentRoomId조건은 도달 불가능한 데드 코드입니다.Line 24에서
roomId || localStorage.getItem('currentRoomId') || '1'로 할당하므로currentRoomId는 최소'1'이 보장됩니다. 따라서 Line 26의if (!currentRoomId)분기는 절대 실행되지 않습니다.의도에 따라 fallback
'1'을 제거하거나, 데드 코드 분기를 제거하세요.옵션 A: fallback 제거 후 guard 유지
- const currentRoomId = roomId || localStorage.getItem('currentRoomId') || '1'; + const currentRoomId = roomId || localStorage.getItem('currentRoomId'); if (!currentRoomId) { setError('방 ID를 찾을 수 없습니다.'); setLoading(false); return; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/groupMembers/GroupMembers.tsx` around lines 24 - 30, currentRoomId is computed as roomId || localStorage.getItem('currentRoomId') || '1', so the subsequent guard if (!currentRoomId) is dead code; decide and apply one of two fixes: either remove the '1' fallback from the assignment to allow the guard to run (change currentRoomId = roomId || localStorage.getItem('currentRoomId')) and keep the if (!currentRoomId) block that calls setError and setLoading, or keep the '1' fallback and delete the entire if (!currentRoomId) branch (remove the setError/setLoading/return block); update references to currentRoomId accordingly in GroupMembers component to match your choice.src/components/feed/MyFeed.tsx (1)
17-35:⚠️ Potential issue | 🟠 Major500ms 최소 로딩 시간 미적용 — PR의 설계 목표와 불일치.
PR 전반에서 "500ms 최소 로딩 시간 패턴 전역 적용(Feed 최초 로드는 1초)"을 명시하고 있으나, 이 컴포넌트의
loadProfile에는 최소 지연이 없습니다. API가 빠르게 응답할 경우(예: 캐시 히트) 스켈레톤이 수십ms 동안 깜빡이는 현상이 그대로 발생합니다.🐛 수정 제안 (Promise.all로 최소 지연 보장)
useEffect(() => { const loadProfile = async () => { try { setLoading(true); - const response = await getMyProfile(); + const [response] = await Promise.all([ + getMyProfile(), + new Promise<void>(resolve => setTimeout(resolve, 500)), + ]); setProfileData(response.data); } catch (error) { console.error('프로필 정보 로드 실패:', error); } finally { setLoading(false); } }; loadProfile(); }, []);추가로, API 오류 시
loading이false로 전환되어도profileData가null인 채로 남아 Line 33의!profileData조건이 계속true를 반환하므로 스켈레톤이 무한정 표시될 수 있습니다. 에러 상태를 별도로 관리하여 사용자에게 적절한 피드백을 제공하는 것을 권장합니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/feed/MyFeed.tsx` around lines 17 - 35, The loadProfile async inside useEffect currently calls getMyProfile() directly and sets loading/profileData immediately, causing the missing 500ms minimum-skeleton delay and also leaves profileData null on API error (causing an infinite skeleton). Modify loadProfile to await Promise.all([getMyProfile(), new Promise(resolve => setTimeout(resolve, 500))]) (or equivalent sleep) so the skeleton shows at least 500ms, and add an error state (e.g., error / setError) that you set in the catch block and clear in success; then update the render logic (loading, profileData, error) so that on error you show an error/fallback UI instead of the persistent OtherFeedSkeleton. Ensure you update references to setLoading, setProfileData, getMyProfile, loading, and profileData accordingly.src/pages/groupSearch/GroupSearch.tsx (1)
50-59:⚠️ Potential issue | 🟡 Minor마운트 시
fetchRecentSearches이중 호출 및 누락된 의존성
useEffect두 개가 모두 마운트 시 실행됩니다.
- useEffect 1 (
[]): 항상fetchRecentSearches()호출- useEffect 2 (
[searchStatus]): 마운트 시searchStatus가'idle'이므로 조건 통과 → 동일하게 호출결과적으로 초기 렌더링 시 동일한 API 요청이 두 번 발생하고
setIsLoadingRecentSearches호출이 경합할 수 있습니다.useEffect 1은useEffect 2가 이미 마운트 시 커버하므로 제거하거나,useEffect 2에 마운트 스킵 로직(ref guard)을 추가하는 것이 적절합니다.또한
useEffect 2에서fetchRecentSearches가 의존성 배열에 없고// eslint-disable-next-line주석도 없어 lint 경고가 발생합니다(useEffect 1과 불일치).🐛 수정 제안 — useEffect 1 제거로 중복 호출 방지
- useEffect(() => { - fetchRecentSearches(); - // eslint-disable-next-line react-hooks/exhaustive-deps - }, []); - useEffect(() => { if (searchStatus === 'idle') { fetchRecentSearches(); } + // eslint-disable-next-line react-hooks/exhaustive-deps }, [searchStatus]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/groupSearch/GroupSearch.tsx` around lines 50 - 59, Remove the redundant mount-only effect to avoid double-calling fetchRecentSearches: delete the first useEffect that runs on [] and keep the second useEffect([searchStatus]); ensure fetchRecentSearches is referenced correctly (add it to the dependency array or wrap it with useCallback) so linting is satisfied, and preserve the existing condition that checks searchStatus === 'idle' (which controls setIsLoadingRecentSearches) to prevent concurrent state races.src/pages/search/Search.tsx (1)
117-119:⚠️ Potential issue | 🟡 Minor
fetchRecentSearches가useEffect의존성 배열에서 누락되었으며 eslint-disable 주석도 없습니다
fetchRecentSearches는 컴포넌트 본문에 정의된 함수로react-hooks/exhaustive-deps규칙 대상입니다. GroupSearch.tsx의 첫 번째useEffect에는// eslint-disable-next-line react-hooks/exhaustive-deps주석이 있지만 이 파일에는 없어 lint 경고가 발생합니다.🐛 수정 제안 — eslint-disable 주석 추가 또는 useCallback 래핑
옵션 1: eslint-disable 주석 추가
useEffect(() => { + // eslint-disable-next-line react-hooks/exhaustive-deps fetchRecentSearches(); }, []);옵션 2:
fetchRecentSearches를useCallback으로 래핑 후 deps에 추가-const fetchRecentSearches = async () => { +const fetchRecentSearches = useCallback(async () => { ... -}; +}, []); useEffect(() => { fetchRecentSearches(); }, [fetchRecentSearches]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/search/Search.tsx` around lines 117 - 119, The useEffect calling fetchRecentSearches is missing fetchRecentSearches from its dependency array which triggers react-hooks/exhaustive-deps; either wrap the fetchRecentSearches function in useCallback and add that callback to the useEffect deps, or keep the function as-is and add a single-line comment // eslint-disable-next-line react-hooks/exhaustive-deps immediately above the useEffect to silence the rule; modify the fetchRecentSearches declaration (wrap with useCallback) and the useEffect invocation (add fetchRecentSearches to deps) if you choose the callback approach, or add the eslint-disable comment above the useEffect if you prefer the quick fix.src/pages/mypage/Mypage.tsx (1)
38-49:⚠️ Potential issue | 🟠 MajorAPI 오류 시 스켈레톤이 무한히 표시됩니다
catch블록이 에러를 로그만 출력하고 별도의 에러 상태를 설정하지 않습니다.finally에서setLoading(false)가 실행된 후profile이null로 남아loading || !profile조건이 계속true가 되어 스켈레톤이 무한히 표시됩니다. AI 요약에서도 "failed to load" 브랜치가 제거된 것을 확인할 수 있어 이는 명백한 회귀(regression)입니다.🐛 수정 제안
const [profile, setProfile] = useState<GetMyProfileResponse['data'] | null>(null); const [loading, setLoading] = useState(true); +const [error, setError] = useState(false); ... } catch (error) { console.error('프로필 정보 로드 실패:', error); + setError(true); } finally {- {loading || !profile ? ( + {loading ? ( <ProfileSkeletonContainer> ... </ProfileSkeletonContainer> + ) : error || !profile ? ( + <div style={{ padding: '20px', color: 'var(--color-text-secondary_grey00)' }}> + 프로필 정보를 불러오지 못했습니다. + </div> ) : ( <UserProfile>Also applies to: 108-118
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/mypage/Mypage.tsx` around lines 38 - 49, The fetchProfile function currently only logs errors so the UI stays in "loading || !profile" and shows a skeleton forever; update fetchProfile's catch to set an explicit error state and ensure profile is cleared: call setProfile(null) and setProfileError(error) or setHasProfileError(true) (create that state with e.g., const [profileError, setProfileError] = useState(null) or boolean) inside the catch, then keep the finally calling setLoading(false); also update the render branch that currently uses loading || !profile to show an error UI when profileError is set (instead of the skeleton) so failed API calls display an error message rather than an infinite skeleton (refer to fetchProfile, getMyProfile, setProfile, setLoading).
🧹 Nitpick comments (26)
src/shared/ui/Skeleton/members/MemberListSkeleton.tsx (3)
27-29: 아이콘 스켈레톤의 불필요한 래퍼<div>및 인라인 스타일
Skeleton.Box와 동일한 크기(24×24)로 설정된<div>래퍼가marginLeft만을 위해 존재합니다.Skeleton.Box에 직접 스타일을 적용하거나 styled-component를 사용하면 래퍼를 제거할 수 있습니다.♻️ 래퍼 제거 예시
- <div style={{ width: 24, height: 24, marginLeft: 4 }}> - <Skeleton.Box width={24} height={24} /> - </div> + <Skeleton.Box width={24} height={24} style={{ marginLeft: 4 }} />
Skeleton.Box가styleprop을 지원하지 않는 경우, styled-component에서margin-left를 지정하는 방법을 사용하세요.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/shared/ui/Skeleton/members/MemberListSkeleton.tsx` around lines 27 - 29, The wrapper <div> around the icon skeleton is unnecessary; remove the extra element in MemberListSkeleton and apply the margin-left and size directly to Skeleton.Box instead (either by passing a style prop to Skeleton.Box or by creating a styled-component wrapper around Skeleton.Box that sets width:24px; height:24px; margin-left:4px). Ensure the Skeleton.Box usage in MemberListSkeleton preserves the 24×24 dimensions and the left margin after removing the outer div.
16-18:Skeleton.Circle주변 불필요한 래퍼<div>제거 권장
Skeleton.Circle을 감싸는<div>가 별도의 스타일이나 의미론적 역할 없이 추가되어 있습니다. 직접ProfileSection아래에 배치해도 동일하게 동작합니다.♻️ 불필요한 래퍼 제거
<ProfileSection> - <div> - <Skeleton.Circle width={36} /> - </div> + <Skeleton.Circle width={36} /> <MemberInfo>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/shared/ui/Skeleton/members/MemberListSkeleton.tsx` around lines 16 - 18, Remove the unnecessary wrapper div around Skeleton.Circle in MemberListSkeleton (the JSX under ProfileSection): replace the <div> that only contains <Skeleton.Circle width={36} /> by rendering <Skeleton.Circle width={36} /> directly, and if spacing/layout changes, adjust the parent ProfileSection or Skeleton.Circle props/classes instead of reintroducing a non-semantic wrapper.
2-8:shared/ui에서components/를 참조하는 레이어 역전 문제
src/shared/ui/Skeleton/하위 파일이@/components/members/MemberList.styled를 import하고 있습니다. 일반적인 의존성 방향은pages → components → shared/ui인데, 여기서는shared/ui → components로 역전되어 있습니다.이 패턴이 전파되면
shared/ui가 점점 특정 도메인 컴포넌트에 결합되어 재사용성이 떨어지게 됩니다. 또한 Line 14의cursor: 'default', pointerEvents: 'none'인라인 오버라이드는 이 결합의 부작용으로 발생한 것입니다.PR 설명에 명시된 CLS 안정성을 위해 실제 컴포넌트 스타일을 재사용하는 의도는 이해하지만,
MemberListSkeleton.styled.ts를 별도로 생성하여 필요한 레이아웃 스타일만 복사하거나, 공유 스타일을shared/하위로 이동하는 방식이 더 바람직합니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/shared/ui/Skeleton/members/MemberListSkeleton.tsx` around lines 2 - 8, The MemberListSkeleton is importing domain-specific styles from '@/components/members/MemberList.styled' which inverts the intended dependency graph; instead, create a local styled file (e.g., MemberListSkeleton.styled.ts) alongside MemberListSkeleton.tsx or move the shared layout styles into shared/ui and import those so shared/ui does not depend on components; update the import in MemberListSkeleton.tsx to use the new local/shared style module and remove the inline override (cursor: 'default', pointerEvents: 'none') by copying only the necessary layout styles (Container, MemberItem, ProfileSection, MemberInfo, MemberStatus equivalents) into the new styled file.src/shared/ui/Skeleton/todaywords/DateDividerSkeleton.tsx (1)
5-7: 인라인 스타일보다 styled-component 사용 권장프로젝트 내 다른 스켈레톤 컴포넌트들(
MessageListSkeleton.styled.ts,FeedPostSkeleton.styled.ts등)은 모두 emotion의styledAPI로 레이아웃을 정의하고 있는 반면, 이 파일만 인라인 스타일을 사용하고 있어 일관성이 떨어집니다.♻️ 스타일 분리 제안
DateDividerSkeleton.styled.ts파일을 추가하거나 같은 파일 내에 styled-component를 정의하는 방식이 더 일관적입니다.+import styled from '@emotion/styled'; import Skeleton from '../base/Skeleton'; +const Center = styled.div` + display: flex; + justify-content: center; +`; const DateDividerSkeleton = () => { return ( - <div style={{ display: 'flex', justifyContent: 'center' }}> + <Center> <Skeleton.Box width={120} height={45} borderRadius={16} /> - </div> + </Center> ); };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/shared/ui/Skeleton/todaywords/DateDividerSkeleton.tsx` around lines 5 - 7, The DateDividerSkeleton component currently uses an inline div style; replace it with an emotion styled-component for consistency: create (or add) a styled wrapper (e.g., DateDividerSkeletonContainer or StyledDateDivider) using emotion's styled API with display: flex and justify-content: center, export it from DateDividerSkeleton.styled.ts (or from the same file), import and use that styled wrapper in DateDividerSkeleton instead of the inline div while keeping the existing Skeleton.Box usage and props (width, height, borderRadius).src/shared/ui/Skeleton/todaywords/MessageItemSkeleton.tsx (1)
2-2:MessageListSkeleton.styled에 대한 긴밀한 결합(tight coupling) 고려 필요
MessageItemSkeleton이 독립적인 컴포넌트임에도 불구하고MessageListSkeleton.styled.ts의 스타일을 직접 가져와 사용하고 있습니다. 이로 인해MessageListSkeleton.styled.ts가 수정될 경우 예상치 못한 영향을 받을 수 있습니다.
MessageItemSkeleton.styled.ts를 별도로 분리하거나, 두 컴포넌트가 공유하는 스타일은 별도의 공통 스타일 파일로 이동하는 것을 권장합니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/shared/ui/Skeleton/todaywords/MessageItemSkeleton.tsx` at line 2, The current MessageItemSkeleton imports styles directly from MessageListSkeleton.styled (symbols: MessageSkeletonItem, UserInfoSkeleton, UserDetailsSkeleton) causing tight coupling; to fix, extract the styles used by MessageItemSkeleton into a new file (e.g., MessageItemSkeleton.styled) or a shared styles module and update MessageItemSkeleton to import from that new module, leaving MessageListSkeleton.styled untouched; ensure exported styled names match the ones used by MessageItemSkeleton so no other code changes are required.src/shared/ui/Skeleton/group/HotTopicSectionSkeleton.tsx (1)
15-20: 인라인 스타일을 styled 컴포넌트로 추출하는 것을 고려해 주세요.다른 그룹 스켈레톤 컴포넌트(
CommentSectionSkeleton,GroupBookSectionSkeleton)에서는cursor: 'default'정도만 인라인으로 사용하는 반면, 이 컴포넌트에서는padding,marginBottom,display,flexDirection,gap등 상당한 레이아웃 스타일이 인라인으로 적용되어 있습니다. 일관성과 유지보수성을 위해 별도의 styled 컴포넌트로 추출하면 좋겠습니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/shared/ui/Skeleton/group/HotTopicSectionSkeleton.tsx` around lines 15 - 20, HotTopicSectionSkeleton의 인라인 레이아웃 스타일(padding, marginBottom, display, flexDirection, gap 등)을 별도의 styled 컴포넌트로 추출해 유지보수성과 일관성을 높이세요; 예를 들어 HotTopicSectionSkeleton 컴포넌트 내부의 최상위 div와 텍스트/박스 래퍼(현재 스타일이 적용된 요소들)를 각각 StyledContainer, StyledHeader, StyledList 같은 이름의 styled-components로 분리하고 기존 JSX에서 해당 styled 컴포넌트로 교체해 주세요(참고 심볼: HotTopicSectionSkeleton, Skeleton.Text, Skeleton.Box). Ensure styles like cursor:'default' remain consistent with other skeleton components.src/shared/ui/Skeleton/searchBook/BookDetailSkeleton.tsx (1)
30-44: 반복되는 인라인 스타일 패턴을 공통 상수로 추출해 보세요.
{ cursor: 'default', pointerEvents: 'none' }이RecruitingGroupButton,WritePostButton,SaveButton세 곳에서 반복됩니다. 또한 Line 40의SaveButton은disabled속성과pointerEvents: 'none'을 동시에 사용하고 있어 중복입니다.제안: 공통 스타일 상수 추출
+const disabledStyle = { cursor: 'default', pointerEvents: 'none' as const }; + const BookDetailSkeleton = () => { return ( <BannerSection> ... <ButtonSection> - <RecruitingGroupButton - style={{ cursor: 'default', pointerEvents: 'none', marginTop: '5px' }} - > + <RecruitingGroupButton style={{ ...disabledStyle, marginTop: '5px' }}> <Skeleton.Text width={120} height={14} /> </RecruitingGroupButton> <RightArea> - <WritePostButton style={{ cursor: 'default', pointerEvents: 'none' }}> + <WritePostButton style={disabledStyle}> <Skeleton.Text width={80} height={14} /> </WritePostButton> - <SaveButton disabled style={{ cursor: 'default', pointerEvents: 'none' }}> + <SaveButton style={disabledStyle}> <Skeleton.Box width={24} height={24} /> </SaveButton> </RightArea> </ButtonSection> </BannerSection> ); };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/shared/ui/Skeleton/searchBook/BookDetailSkeleton.tsx` around lines 30 - 44, Extract a shared style constant (e.g. const DISABLED_POINTER_STYLE = { cursor: 'default', pointerEvents: 'none' }) and replace the inline style objects on RecruitingGroupButton and WritePostButton with that constant; for SaveButton remove the redundant pointerEvents from its inline style and rely on the disabled prop (or apply only cursor: 'default' if you need visual consistency) to avoid duplicating behavior, updating usages of RecruitingGroupButton, WritePostButton, and SaveButton in BookDetailSkeleton to reference the new constant or simplified style.src/shared/ui/Skeleton/group/GroupCardSkeleton.tsx (1)
27-27: 동일한 조건식 중복 — 변수로 추출하는 것을 권장합니다.같은 삼항 조건
type === 'search' || isRecommend이 width·height에서 각각 반복됩니다. 변수로 한 번만 계산하면 가독성이 향상되고 수정 시 누락 위험도 줄어듭니다.♻️ 리팩터 제안
const GroupCardSkeleton = ({ type = 'search', isRecommend = false }: Props) => { + const isCompact = type === 'search' || isRecommend; + const coverWidth = isCompact ? 60 : 80; + const coverHeight = isCompact ? 80 : 107; return ( <Card cardType={type} style={{ cursor: 'default', pointerEvents: 'none' }}> <CoverWrapper> <Cover as="div" cardType={type} isRecommend={isRecommend} style={{ background: 'none', display: 'flex', alignItems: 'center', justifyContent: 'center' }} > - <Skeleton.Box width={type === 'search' || isRecommend ? 60 : 80} height={type === 'search' || isRecommend ? 80 : 107} /> + <Skeleton.Box width={coverWidth} height={coverHeight} /> </Cover> </CoverWrapper>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/shared/ui/Skeleton/group/GroupCardSkeleton.tsx` at line 27, Extract the repeated ternary condition `type === 'search' || isRecommend` into a single boolean variable (e.g., `isCompact`) at the top of the GroupCardSkeleton component and use that variable for both Skeleton.Box props so width and height use the same computed flag; update the Skeleton.Box call (the line with `Skeleton.Box width={...} height={...}`) to reference `isCompact` for selecting 60/80 and 80/107 values respectively.src/shared/ui/Skeleton/group/GroupDetailSkeleton.styled.ts (1)
22-26: 그라디언트 내 하드코딩된 색상 값 —colors토큰과의 불일치.같은 파일의
SkeletonWrapper는${colors.black.main}을 사용하지만,BannerSkeletonSection의 그라디언트에서는 동일한 색상(#121212)을rgba(18, 18, 18, 0.3)(하드코딩 RGB) 및 CSS 커스텀 프로퍼티(var(--color-view-viewfield_background_black,#121212))로 표현하고 있습니다. 테마 토큰과 CSS 변수가 혼재되어 향후 색상 변경 시 누락될 수 있습니다.♻️ 리팩터 제안
export const BannerSkeletonSection = styled.div` display: flex; flex-direction: column; width: 100%; padding: 76px 20px 20px; gap: 20px; background: linear-gradient( 172deg, - rgba(18, 18, 18, 0.3) 5.94%, - var(--color-view-viewfield_background_black, `#121212`) 94% + ${colors.black.main}4D 5.94%, + ${colors.black.main} 94% ); `;
#1212124D는rgba(18, 18, 18, 0.3)의 hex 알파값(0.3 × 255 ≈ 76 = 0x4D)입니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/shared/ui/Skeleton/group/GroupDetailSkeleton.styled.ts` around lines 22 - 26, BannerSkeletonSection's gradient mixes hardcoded rgba and a CSS variable while SkeletonWrapper uses the theme token, causing inconsistency; replace the hardcoded rgba(18,18,18,0.3) and var(--color-view-viewfield_background_black, `#121212`) with the same theme token used in SkeletonWrapper (e.g., colors.black.main) and an alpha-consistent form (hex with alpha 4D or a rgba built from the token) so both gradient stops reference the single source of truth; update BannerSkeletonSection's background property to use colors.black.main for the opaque stop and the token+alpha for the translucent stop to match SkeletonWrapper and avoid future drift.src/shared/ui/Skeleton/group/BookSkeleton.tsx (1)
15-19: 인라인style대신 styled-component 사용을 권장합니다.
BookSkeletonDetails가 이미gap: 12px를 갖고 있으며, 내부 margin 역시GroupDetailSkeleton.styled.ts에 styled-component로 추가하면 나머지 스켈레톤 코드베이스의 스타일링 방식과 일관성을 유지할 수 있습니다.♻️ 리팩터 제안 (GroupDetailSkeleton.styled.ts에 추가)
// GroupDetailSkeleton.styled.ts +export const BookSkeletonMeta = styled.div` + display: flex; + flex-direction: column; + gap: 4px; + margin-top: 8px; +`; + +export const BookSkeletonLines = styled.div` + margin-top: 4px; +`;// BookSkeleton.tsx -import { BookSkeletonInfo, BookSkeletonDetails } from './GroupDetailSkeleton.styled'; +import { BookSkeletonInfo, BookSkeletonDetails, BookSkeletonMeta, BookSkeletonLines } from './GroupDetailSkeleton.styled'; ... - <div style={{ marginTop: '8px' }}> + <BookSkeletonMeta> <Skeleton.Text width={60} height={12} /> - <div style={{ marginTop: '4px' }}> + <BookSkeletonLines> <Skeleton.Text lines={3} height={12} gap={4} /> - </div> + </BookSkeletonLines> - </div> + </BookSkeletonMeta>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/shared/ui/Skeleton/group/BookSkeleton.tsx` around lines 15 - 19, Replace the inline styles in BookSkeleton.tsx with styled-components: add a new styled wrapper or update BookSkeletonDetails in GroupDetailSkeleton.styled.ts to include the vertical spacing (marginTop/gap) rules currently applied inline (the outer div marginTop: '8px' and inner div marginTop: '4px'), then import and use that styled component in BookSkeleton.tsx in place of the inline wrappers so spacing is driven by the styled-component and keeps styling consistent with the rest of the skeleton components.src/pages/feed/OtherFeedPage.tsx (1)
37-46:Promise<void>타입 명시로 불필요한 캐스트 제거 가능
new Promise(resolve => ...)는Promise<unknown>으로 추론되어as [...]캐스트가 필요해집니다. 같은 패턴을 사용하는FeedDetailPage.tsx는 캐스트 없이 정상 동작합니다. 타입 인수를 명시하면 캐스트 없이 일관성 있게 정리됩니다.♻️ 타입 단언 제거 제안
-const minLoadingTime = new Promise(resolve => setTimeout(resolve, 500)); -const [feedResponse, profileResponse] = (await Promise.all([ +const minLoadingTime = new Promise<void>(resolve => setTimeout(resolve, 500)); +const [feedResponse, profileResponse] = await Promise.all([ getOtherFeed(Number(userId)), getOtherProfile(Number(userId)), minLoadingTime, -])) as [ - Awaited<ReturnType<typeof getOtherFeed>>, - Awaited<ReturnType<typeof getOtherProfile>>, - void, -]; +]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/feed/OtherFeedPage.tsx` around lines 37 - 46, The temporary minLoadingTime Promise is inferred as Promise<unknown>, forcing the downstream cast; change its construction to explicitly be Promise<void> (e.g., const minLoadingTime: Promise<void> = new Promise(resolve => setTimeout(resolve, 500))); then remove the unnecessary as [...] type assertion from the Promise.all result so feedResponse and profileResponse can be correctly inferred from getOtherFeed and getOtherProfile respectively (references: minLoadingTime, getOtherFeed, getOtherProfile, feedResponse, profileResponse).src/pages/mypage/SavePage.styled.ts (1)
119-132:BookItem/LeftSection확장으로 중복 제거 가능
BookSkeletonItem은 기존BookItem과align-items: flex-start추가 및&:last-child제거 외에 동일한 스타일을 반복하며,BookSkeletonLeft는flex-direction: row(기본값) 유무만 다를 뿐LeftSection과 사실상 동일합니다.♻️ 스타일 컴포넌트 상속으로 중복 제거 제안
-export const BookSkeletonItem = styled.div` - width: 94.8%; - margin: 0 auto; - display: flex; - justify-content: space-between; - align-items: flex-start; - border-bottom: 1px solid ${colors.darkgrey.dark}; - padding: 12px; -`; - -export const BookSkeletonLeft = styled.div` - display: flex; - gap: 12px; -`; +export const BookSkeletonItem = styled(BookItem)` + align-items: flex-start; + &:last-child { + border-bottom: 1px solid ${colors.darkgrey.dark}; + } +`;
BookSkeletonLeft는LeftSection과 동일하므로 별도 정의 없이LeftSection을 직접 재사용할 수 있습니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/mypage/SavePage.styled.ts` around lines 119 - 132, BookSkeletonItem duplicates BookItem except for adding align-items: flex-start and removing the &:last-child rule, and BookSkeletonLeft duplicates LeftSection; refactor by making BookSkeletonItem extend or reuse BookItem (e.g., styled(BookItem)) and override only align-items (and remove the &:last-child override), and replace BookSkeletonLeft with direct reuse of LeftSection (no new styled definition) so you keep a single source of truth for these styles.src/pages/today-words/TodayWords.tsx (1)
94-101:minLoadingTime조건부 생성 권장
isRefresh가false(무한스크롤 추가 로드)일 때도 500ms 타이머가 항상 생성되어 resolve 후 GC 처리됩니다. 불필요한 타이머 생성을 방지하려면 조건부로 생성하세요.♻️ 수정 제안
- const minLoadingTime = new Promise(resolve => setTimeout(resolve, 500)); - const [response] = await Promise.all([ - getDailyGreeting({ - roomId: parseInt(roomId), - cursor: cursor || undefined, - }), - isRefresh ? minLoadingTime : Promise.resolve(), - ]); + const [response] = await Promise.all([ + getDailyGreeting({ + roomId: parseInt(roomId), + cursor: cursor || undefined, + }), + isRefresh ? new Promise(resolve => setTimeout(resolve, 500)) : Promise.resolve(), + ]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/today-words/TodayWords.tsx` around lines 94 - 101, Currently minLoadingTime promise is always allocated regardless of isRefresh, causing unnecessary timers; change the Promise.all call so the 500ms timer is only created when isRefresh is true (e.g., compute minLoadingTime conditionally or pass isRefresh ? new Promise(resolve => setTimeout(resolve, 500)) : Promise.resolve()), keeping the getDailyGreeting call intact; update references to minLoadingTime, isRefresh and the Promise.all([...]) invocation in TodayWords.tsx accordingly.src/shared/ui/Skeleton/search/MostSearchedBooksSkeleton.tsx (1)
14-20:getCurrentDate함수를 컴포넌트 외부로 이동 권장함수가 컴포넌트 본문 안에 정의되어 매 렌더마다 재생성됩니다. 스켈레톤 특성상 짧게 표시되므로 체감 영향은 없지만, 컴포넌트 외부에 선언하거나 모듈 수준 상수로 추출하면 더 명확합니다.
♻️ 수정 제안
+const getCurrentDate = () => { + const now = new Date(); + const month = String(now.getMonth() + 1).padStart(2, '0'); + const day = String(now.getDate()).padStart(2, '0'); + return `${month}.${day}. 기준`; +}; + const MostSearchedBooksSkeleton = () => { - const getCurrentDate = () => { - const now = new Date(); - const month = String(now.getMonth() + 1).padStart(2, '0'); - const day = String(now.getDate()).padStart(2, '0'); - return `${month}.${day}. 기준`; - }; - return (🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/shared/ui/Skeleton/search/MostSearchedBooksSkeleton.tsx` around lines 14 - 20, getCurrentDate is defined inside the MostSearchedBooksSkeleton component and gets recreated on every render; move getCurrentDate out of the component body and declare it at module scope (or compute a module-level constant like CURRENT_DATE using the same logic) so MostSearchedBooksSkeleton references the external function/constant instead of recreating it each render.src/pages/memory/Memory.tsx (1)
336-340: 인라인 스타일 — 스타일드 컴포넌트로 추출 가능
style={{ display: 'flex', flexDirection: 'column', gap: '40px' }}는Memory.styled.ts또는MemoryContent.styled.ts에 별도 컴포넌트로 추출하면 유지보수성이 향상됩니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/memory/Memory.tsx` around lines 336 - 340, Extract the inline container style into a styled component (e.g., MemoryContentContainer) in Memory.styled.ts or MemoryContent.styled.ts and replace the div in Memory.tsx that wraps the RecordItemSkeleton list with that styled component; specifically create a styled div with display:flex, flex-direction:column and gap:40px, export it, import it into Memory.tsx, and use it in place of the inline-styled div containing the Array.from(...).map(...) block.src/shared/ui/Skeleton/group/BannerSkeleton.tsx (1)
15-20: 인라인marginTop스타일 — 프로젝트 컨벤션과 불일치
MetaSkeletonRow는margin-top: 16px을 styled 정의 내에 보유하는데, 이 컴포넌트는marginTop: '20px',marginTop: '8px'을 인라인 스타일로 처리하고 있습니다.GroupDetailSkeleton.styled.ts에 해당 여백들을 상수 스타일로 추출하면 일관성을 유지할 수 있습니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/shared/ui/Skeleton/group/BannerSkeleton.tsx` around lines 15 - 20, BannerSkeleton uses inline marginTop values (marginTop: '20px' and '8px') that conflict with the component convention where MetaSkeletonRow holds margin-top in styled definitions; extract these spacing values as constants/styles inside GroupDetailSkeleton.styled.ts (e.g., export a BannerWrapper and a SubRow or spacing tokens matching MetaSkeletonRow) and replace the inline styles in BannerSkeleton.tsx to use those styled components or imported constants (locate BannerSkeleton.tsx and the MetaSkeletonRow symbol to mirror its pattern and update imports to use the new exports from GroupDetailSkeleton.styled.ts).src/shared/ui/Skeleton/feed/UserProfileItemSkeleton.tsx (1)
21-25:followbutton의cursor: 'default'— 부모의pointerEvents: 'none'으로 이미 차단됨Line 10에서
Wrapper에pointerEvents: 'none'을 설정했으므로, 하위.followbuttondiv의cursor: 'default'는 중복입니다.♻️ 제거 제안
- <div className="followbutton" style={{ cursor: 'default' }}> + <div className="followbutton">🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/shared/ui/Skeleton/feed/UserProfileItemSkeleton.tsx` around lines 21 - 25, The .followbutton div in UserProfileItemSkeleton.tsx redundantly sets style={{ cursor: 'default' }} even though the parent Wrapper already disables interactions via pointerEvents: 'none'; remove the inline cursor style from the .followbutton element (and any similar redundant cursor overrides) so the component relies on the Wrapper's pointerEvents setting; locate the followbutton JSX inside the conditional branch where type === 'followlist' and delete the cursor property only.src/pages/search/Search.tsx (1)
15-15: 다른 컴포넌트의 내부 styled 컴포넌트를 직접 임포트
BookListWrapper와List를@/components/search/BookSearchResult.styled에서 직접 가져오면 두 컴포넌트가 스타일 구현에 결합됩니다.BookSearchResult.styled가 리팩터링될 경우 Search.tsx도 함께 수정이 필요합니다. 공유 styled 컴포넌트로 분리하거나Search.styled에 별도 래퍼를 정의하는 것을 고려해 보세요.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/search/Search.tsx` at line 15, Search.tsx currently imports internal styled components BookListWrapper and List from BookSearchResult.styled which couples Search to that component's implementation; instead either (A) move the shared styles into a new shared styled module (e.g., create a shared/SearchLayout.styled exporting BookListWrapper and List) and update both Search.tsx and BookSearchResult to import from it, or (B) create local equivalents in Search.styled (define a local Wrapper/List styled-components used only by Search.tsx) and change the import in Search.tsx to use Search.styled; update all references to BookListWrapper and List in Search.tsx accordingly and remove the direct import from BookSearchResult.styled.src/pages/feed/MyFeedPage.tsx (1)
37-46: 불필요한as타입 캐스트 —Promise<void>로 단순화 가능
minLoadingTime의 타입이Promise<unknown>으로 추론되어 TypeScript가Promise.all반환 타입을 명확히 인식하지 못해as [...]캐스트가 추가되었습니다.new Promise<void>(...)로 타입을 명시하면 캐스트 없이 올바른 타입 추론이 가능합니다. 이 패턴은 PR 내 다른 파일들에서도 동일하게 적용할 수 있습니다.♻️ 수정 제안
- const minLoadingTime = new Promise(resolve => setTimeout(resolve, 500)); - const [feedResponse, profileResponse] = (await Promise.all([ + const minLoadingTime = new Promise<void>(resolve => setTimeout(resolve, 500)); + const [feedResponse, profileResponse] = await Promise.all([ getOtherFeed(Number(userId)), getMyProfile(), minLoadingTime, - ])) as [ - Awaited<ReturnType<typeof getOtherFeed>>, - Awaited<ReturnType<typeof getMyProfile>>, - void, - ]; + ]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/feed/MyFeedPage.tsx` around lines 37 - 46, The cast on the Promise.all result is unnecessary because minLoadingTime is inferred as Promise<unknown>; change the minLoadingTime creation to new Promise<void>(resolve => setTimeout(resolve, 500)) (or otherwise type it as Promise<void>) so TypeScript can infer Promise.all<[Awaited<ReturnType<typeof getOtherFeed>>, Awaited<ReturnType<typeof getMyProfile>>, void]> and you can remove the manual as [...] cast; update the same pattern anywhere else in the PR where minLoadingTime (or similar helper promises) is created.src/pages/groupSearch/GroupSearch.tsx (1)
354-369: 삭제 후fetchRecentSearches재호출 시 불필요한 500ms 스켈레톤 플래시아이템 삭제 후
fetchRecentSearches()를 다시 호출하면isLoadingRecentSearches가 true로 설정되어 스켈레톤이 500ms 동안 다시 표시됩니다. 삭제 직후 화면이 깜빡이는 UX 문제가 발생합니다. 낙관적 업데이트(삭제 즉시 로컬 상태에서 항목 제거)를 활용하고 불필요한 경우 재호출을 생략하는 것을 고려해 보세요.♻️ 개선 제안 — 낙관적 업데이트 적용
handleDelete={async (term: string) => { const x = recentSearches.find(i => i.searchTerm === term); if (!x) return; + setRecentSearches(prev => prev.filter(i => i.recentSearchId !== x.recentSearchId)); const res = await deleteRecentSearch(x.recentSearchId); - if (res.isSuccess) { - await fetchRecentSearches(); - } + if (!res.isSuccess) { + await fetchRecentSearches(); // 실패 시에만 재조회 + } }}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/groupSearch/GroupSearch.tsx` around lines 354 - 369, The UX blip comes from re-calling fetchRecentSearches which sets isLoadingRecentSearches true and shows the skeleton for ~500ms after a delete; change the inline handleDelete passed to RecentSearchTabs to perform an optimistic update instead: locate the handleDelete callback (the async (term: string) => { ... }) and immediately remove the item from the local recentSearches state (e.g., setRecentSearches(prev => prev.filter(i => i.searchTerm !== term))) before calling deleteRecentSearch, avoid toggling isLoadingRecentSearches or calling fetchRecentSearches on success, and only revert (or call fetchRecentSearches) if deleteRecentSearch returns an error to preserve consistency.src/pages/searchBook/SearchBook.tsx (1)
244-255: 도달 불가능한 폴백 문자열
if (error)블록 안에서error는 항상 truthy이므로|| '책 정보를 찾을 수 없습니다.'폴백은 절대 사용되지 않습니다.♻️ 수정 제안
- {error || '책 정보를 찾을 수 없습니다.'} + {error}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/searchBook/SearchBook.tsx` around lines 244 - 255, The JSX in SearchBook.tsx currently uses {error || '책 정보를 찾을 수 없습니다.'} inside the if (error) branch which is redundant because error is always truthy there; replace that expression to reliably show the actual error text (e.g., render error.message if error instanceof Error, otherwise String(error)) so the UI displays meaningful error details; locate the if (error) block (Wrapper/Header/IconButton leftArrow handleBackButton) and update the JSX to render the error string without the unreachable || fallback.src/pages/groupDetail/ParticipatedGroupDetail.tsx (5)
282-284: 인라인 스타일 대신 styled-components 사용 권장이 컴포넌트는 전반적으로 styled-components를 사용하고 있지만, Line 282와 284에서 인라인
style을 사용하고 있어 일관성이 떨어집니다. 매 렌더마다 새 객체가 생성되는 점도 있으므로,ParticipatedGroupDetail.styled에 해당 스타일을 정의하는 것이 좋습니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/groupDetail/ParticipatedGroupDetail.tsx` around lines 282 - 284, Replace the inline-styled containers with styled-components defined in ParticipatedGroupDetail.styled: create a ColumnContainer (display: flex; flex-direction: column; gap: 12px) to replace the outer div and an IconRow (display: flex; align-items: center; gap: 2px) to replace the inner div, export them from ParticipatedGroupDetail.styled, import them into ParticipatedGroupDetail.tsx, and update the JSX to use <ColumnContainer> and <IconRow> in place of the two inline-styled divs (keeping MetaTopRow unchanged).
343-355: 단일 필드 접근을 위한 불필요한 IIFE
isHost하나를 구조분해하기 위해 IIFE를 사용하고 있습니다. 직접 접근으로 충분합니다.♻️ 간소화 제안
- {data && (() => { - const { isHost } = data; - return ( - <GroupActionBottomSheet - isOpen={isBottomSheetOpen} - isGroupOwner={isHost} - onClose={handleCloseBottomSheet} - onDeleteGroup={handleDeleteGroup} - onLeaveGroup={handleLeaveGroup} - onReportGroup={handleReportGroup} - /> - ); - })()} + {data && ( + <GroupActionBottomSheet + isOpen={isBottomSheetOpen} + isGroupOwner={data.isHost} + onClose={handleCloseBottomSheet} + onDeleteGroup={handleDeleteGroup} + onLeaveGroup={handleLeaveGroup} + onReportGroup={handleReportGroup} + /> + )}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/groupDetail/ParticipatedGroupDetail.tsx` around lines 343 - 355, Remove the unnecessary IIFE used to destructure isHost for a single-field access; directly pass data.isHost into the GroupActionBottomSheet prop. Locate the JSX block that currently does {data && (() => { const { isHost } = data; return (<GroupActionBottomSheet ... isGroupOwner={isHost} ... />); })()} and replace it with a direct conditional render that uses data.isHost while keeping the existing props isOpen={isBottomSheetOpen}, onClose={handleCloseBottomSheet}, onDeleteGroup={handleDeleteGroup}, onLeaveGroup={handleLeaveGroup}, and onReportGroup={handleReportGroup}.
76-81: 최소 로딩 시간 패턴: API 에러 시에도 500ms 대기API 호출이 즉시 실패하는 경우(네트워크 오류 등)에도 사용자가 500ms 동안 스켈레톤을 보게 됩니다. PR 목적에 따라 의도된 동작일 수 있지만, 에러 상황에서는 빠른 피드백이 더 나을 수 있습니다. 필요 시 에러 발생 시
minLoadingTime을 취소하는 패턴도 고려해 보세요.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/groupDetail/ParticipatedGroupDetail.tsx` around lines 76 - 81, The current pattern always waits the minLoadingTime Promise (minLoadingTime) even when getRoomPlaying(parseInt(roomId)) rejects, which delays error feedback; change the flow so you await getRoomPlaying first (using getRoomPlaying/roomId) and only if it resolves do you await minLoadingTime (or use Promise.allSettled/Promise.race and if the API rejects immediately bypass the 500ms wait), then proceed with response handling — this ensures errors from getRoomPlaying are surfaced immediately while still enforcing the minimum skeleton display on successful responses.
248-306: JSX 내 IIFE 패턴 대신 컴포넌트 추출 또는 상위 레벨 구조분해 권장
(() => { ... })()패턴이 3곳(Line 251, 318, 343)에서 반복 사용되고 있습니다. 이 패턴은 JSX 가독성을 저하시키며, 데이터 구조분해를 위한 목적이라면 더 간결한 방법이 있습니다.대안 1:
data가드 이후 상위 레벨에서 구조분해const data = roomData?.data; // data가 존재할 때 사용할 값들을 미리 추출 const roomName = data?.roomName; const isPublic = data?.isPublic; // ... etc대안 2: 각 섹션을 별도 컴포넌트로 추출하여
data를 props로 전달두 번째 방안이 컴포넌트 크기를 줄이고 관심사 분리에도 도움이 됩니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/groupDetail/ParticipatedGroupDetail.tsx` around lines 248 - 306, The JSX uses repeated IIFE patterns to destructure and render `data` (seen around the block that references roomName, isPublic, roomDescription, progressStartDate, progressEndDate, memberCount, category), which hurts readability; replace each IIFE by first guarding and destructuring `data` at the top of the render (or component body) — e.g., after `if (loading || !data) return <BannerSkeleton />;` or `const { roomName, isPublic, roomDescription, progressStartDate, progressEndDate, memberCount, category } = data;` — then return the JSX directly, or better, extract repeated sections into small components (e.g., GroupHeader/MetaMembers/TagRow) that accept the relevant props (use existing handlers/helpers like handleMembersClick, formatDate, TagGenre, ClickableMeta, GroupTitle) to remove the IIFE usage.
201-205:roomData?.data.isbn에서 optional chaining 누락
roomData?.data.isbn은roomData가null이 아니지만data가undefined인 경우(이론적으로 API 응답 구조가 변경될 때) TypeError가 발생할 수 있습니다. 실제로 이 핸들러는data가 존재할 때만 렌더링되는 컴포넌트에서 호출되므로 현재는 안전하지만, 방어적으로roomData?.data?.isbn으로 변경하는 것이 좋습니다.const handleBookSectionClick = () => { - if (roomData?.data.isbn) { - navigate(`/search/book/${roomData.data.isbn}`); + if (roomData?.data?.isbn) { + navigate(`/search/book/${roomData.data.isbn}`); } };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/groupDetail/ParticipatedGroupDetail.tsx` around lines 201 - 205, The handler handleBookSectionClick uses roomData?.data.isbn which can throw if roomData exists but data is undefined; update the guard to check the nested property safely (use roomData?.data?.isbn) before calling navigate and keep the same navigation path `/search/book/${...}` so the function only navigates when an ISBN string is present.
ho0010
approved these changes
Feb 24, 2026
Collaborator
ho0010
left a comment
ho0010
left a comment
There was a problem hiding this comment.
스켈레톤 UI 하나하나 확인해서 추가하시는게 힘들었을 것 같은데 정말 고생 많으셨습니다!
코드래빗 리뷰 사항도 반영해주시면 좋을 것 같습니다!
- getMyProfile()(로그인 사용자)과 getOtherFeed(userId)(URL 기준) 혼용으로 데이터 불일치 발생 - getOtherProfile(userId)로 교체하여 피드·프로필 모두 URL userId 기준으로 통일 - isFollowing 수동 주입 제거, API 응답값 그대로 사용
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/pages/feed/MyFeedPage.tsx (1)
37-55:⚠️ Potential issue | 🟡 Minor오류 경로에서도 최소 로딩 시간을 보장하세요.
Promise.all이 하나라도 실패하면 즉시 reject되어 500ms 대기가 스킵됩니다. 에러가 빠르게 나는 경우 스켈레톤이 깜빡일 수 있으니, 최소 지연을 finally에서 보장하거나 타이머 기반으로 처리해 주세요.
🔧 수정 제안 (오류 시에도 최소 로딩 시간 보장)
- const minLoadingTime = new Promise(resolve => setTimeout(resolve, 500)); - const [feedResponse, profileResponse] = (await Promise.all([ - getOtherFeed(Number(userId)), - getOtherProfile(Number(userId)), - minLoadingTime, - ])) as [ - Awaited<ReturnType<typeof getOtherFeed>>, - Awaited<ReturnType<typeof getOtherProfile>>, - void, - ]; + const minLoadingMs = 500; + const startedAt = Date.now(); + const [feedResponse, profileResponse] = await Promise.all([ + getOtherFeed(Number(userId)), + getOtherProfile(Number(userId)), + ]); @@ - } finally { + } finally { + const elapsed = Date.now() - startedAt; + if (elapsed < minLoadingMs) { + await new Promise(resolve => setTimeout(resolve, minLoadingMs - elapsed)); + } setLoading(false); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/feed/MyFeedPage.tsx` around lines 37 - 55, The current Promise.all includes minLoadingTime so a rejection from getOtherFeed/getOtherProfile can skip the 500ms wait; ensure the 500ms delay always applies by awaiting minLoadingTime in the finally block instead of inside Promise.all (or use Promise.allSettled for the API calls and keep minLoadingTime separate). Concretely: keep calling getOtherFeed and getOtherProfile as before, but remove minLoadingTime from the Promise.all, handle successes/errors the same (setFeedData/setProfileData or setError), and then in the finally block await minLoadingTime before calling setLoading(false) so the skeleton UI always shows at least 500ms; reference functions/variables: minLoadingTime, getOtherFeed, getOtherProfile, setFeedData, setProfileData, setError, setLoading.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/pages/feed/MyFeedPage.tsx`:
- Around line 37-55: The current Promise.all includes minLoadingTime so a
rejection from getOtherFeed/getOtherProfile can skip the 500ms wait; ensure the
500ms delay always applies by awaiting minLoadingTime in the finally block
instead of inside Promise.all (or use Promise.allSettled for the API calls and
keep minLoadingTime separate). Concretely: keep calling getOtherFeed and
getOtherProfile as before, but remove minLoadingTime from the Promise.all,
handle successes/errors the same (setFeedData/setProfileData or setError), and
then in the finally block await minLoadingTime before calling setLoading(false)
so the skeleton UI always shows at least 500ms; reference functions/variables:
minLoadingTime, getOtherFeed, getOtherProfile, setFeedData, setProfileData,
setError, setLoading.
ℹ️ Review info
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
src/pages/feed/MyFeedPage.tsx
- 에러 발생 시 Header가 없어 브라우저 뒤로가기 외 탈출 수단 없음 - 에러 상태에서도 TitleHeader 렌더링하여 뒤로가기 버튼 제공 - 모바일 환경에서 사용자가 화면에 갇히는 UX 문제 해결
- 로딩 중 RecordTabs·RecordFilters 클릭 시 2차 요청 발생 → stale 데이터 노출 가능 - FixedSection에 pointerEvents: none 적용으로 로딩 중 인터랙션 차단 - AbortController 없이 단순한 방어 처리로 Race Condition 방지
- MenuItemSkeleton이 어디에도 사용되지 않아 dead code로 제거 - 프로필 img에 alt 없어 스크린 리더가 이미지 목적 인식 불가 - profile.nickname 기반 alt 텍스트 추가로 접근성 개선
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/pages/mypage/Mypage.tsx (1)
44-46:⚠️ Potential issue | 🟠 MajorAPI 실패 시 스켈레톤이 무한 표시됩니다 — 에러 상태 처리가 필요합니다
catch블록에서 에러를 로그만 하고 상태를 변경하지 않아,getMyProfile()호출이 실패하면:
finally에서loading이false가 되고profile은 계속null유지결과적으로 108번 라인의
loading || !profile조건이true로 남아 스켈레톤이 영구 표시되며, 사용자는 오류 발생 여부를 알 수 없고 복구 방법도 없습니다.🛠️ 에러 상태 추가 제안
const [profile, setProfile] = useState<GetMyProfileResponse['data'] | null>(null); const [loading, setLoading] = useState(true); + const [error, setError] = useState(false); ... } catch (error) { console.error('프로필 정보 로드 실패:', error); + setError(true); } finally {- {loading || !profile ? ( + {loading ? ( <ProfileSkeletonContainer> ... </ProfileSkeletonContainer> + ) : error || !profile ? ( + <div>프로필을 불러오지 못했습니다.</div> // 실제 에러 UI로 교체 ) : ( <UserProfile>Also applies to: 108-108
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/mypage/Mypage.tsx` around lines 44 - 46, The catch block currently only logs errors so when getMyProfile() fails loading becomes false but profile stays null, leaving the UI stuck on the skeleton; add an error state (e.g., const [error, setError] = useState<Error|null>(null)) and in the catch setError(error) (and optionally setProfile(null) explicitly), then update the render condition from loading || !profile to loading || (!profile && !error) and render an error UI when error is truthy so users see a recoverable error instead of the infinite skeleton; reference getMyProfile, loading, profile, and the catch/finally block to locate changes.
♻️ Duplicate comments (2)
src/pages/mypage/Mypage.tsx (1)
122-122:alt속성이 정상적으로 추가되었습니다이전 리뷰에서 지적된
alt속성 누락이profile.nickname기반으로 수정되었습니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/mypage/Mypage.tsx` at line 122, The img tag now includes an alt based on profile.nickname, but make it robust against missing/empty nicknames: update the img element that uses profile.profileImageUrl and profile.nickname (e.g., <img src={profile.profileImageUrl} alt={`${profile.nickname} 프로필`} />) to provide a safe fallback like `${profile.nickname || '사용자'} 프로필` (or use optional chaining and a default) so the alt never becomes "undefined 프로필".src/pages/groupDetail/ParticipatedGroupDetail.tsx (1)
216-226: 에러 화면에 뒤로가기 제공 — 이전 리뷰 반영 확인.
TitleHeader를 통해 에러 상태에서도 사용자가 뒤로 갈 수 있게 되었습니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/groupDetail/ParticipatedGroupDetail.tsx` around lines 216 - 226, The error UI must allow back navigation: in the error-return branch ensure the TitleHeader remains rendered with leftIcon and onLeftClick={handleBackClick} (as shown) so users can go back when an error occurs; verify handleBackClick is defined in the component and that ParticipatedWrapper and ErrorContainer are used to wrap the TitleHeader and error message exactly as in the diff to preserve layout and accessibility.
🧹 Nitpick comments (8)
src/pages/mypage/Mypage.tsx (1)
42-43:profile변수명이 외부 state를 가립니다
const [profile] = await Promise.all(...)구문이 31번 라인의profilestate 변수를 섀도잉합니다. 기능상 문제는 없으나 가독성을 저해할 수 있습니다.♻️ 변수명 변경 제안
- const [profile] = await Promise.all([getMyProfile(), minLoadingTime]); - setProfile(profile); + const [fetchedProfile] = await Promise.all([getMyProfile(), minLoadingTime]); + setProfile(fetchedProfile);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/mypage/Mypage.tsx` around lines 42 - 43, The local const destructured as "profile" from Promise.all shadows the component state variable "profile"; rename the local result (e.g., fetchedProfile or apiProfile) to avoid shadowing and then pass that to setProfile. Specifically, change the destructured variable returned by Promise.all (from getMyProfile() and minLoadingTime) to a distinct name and call setProfile(fetchedName) so getMyProfile, minLoadingTime, and setProfile are used without shadowing the state variable.src/pages/groupDetail/GroupDetail.tsx (2)
56-56:isLoading초기값이false로 설정되어 있습니다.
ParticipatedGroupDetail에서는loading이true로 초기화되어 마운트 직후부터 스켈레톤이 표시되지만, 여기서는false로 시작합니다.useEffect가 실행되기 전 첫 렌더에서isLoading=false,roomData=null상태가 되므로isLoading || !roomData가드 덕분에 스켈레톤이 표시되긴 하지만, 의미적으로 초기 상태는 로딩 중이므로true로 초기화하는 것이 두 파일 간 일관성과 의도 표현 면에서 더 낫습니다.♻️ 제안
- const [isLoading, setIsLoading] = useState(false); + const [isLoading, setIsLoading] = useState(true);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/groupDetail/GroupDetail.tsx` at line 56, The initial value of the isLoading state in GroupDetail is currently false; change its initialization so it starts true to reflect that the component is loading on mount (aligning with ParticipatedGroupDetail). Update the useState call that creates isLoading and setIsLoading to use true as the initial value, and ensure any logic in the component (including the useEffect that fetches roomData and calls setIsLoading) still sets isLoading to false after data load.
337-340: 삼항 연산자 대신||사용으로 간결하게 표현 가능합니다.♻️ 제안
<BookCover - src={roomData.bookImageUrl ? roomData.bookImageUrl : bookCoverLargeImg} + src={roomData.bookImageUrl || bookCoverLargeImg} alt={roomData.bookTitle} />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/groupDetail/GroupDetail.tsx` around lines 337 - 340, Replace the ternary used for the BookCover src prop with the simpler logical OR pattern: change the expression that currently checks roomData.bookImageUrl ? roomData.bookImageUrl : bookCoverLargeImg to use roomData.bookImageUrl || bookCoverLargeImg so BookCover (component) receives the fallback image when bookImageUrl is falsy; update the src prop in the BookCover JSX accordingly.src/pages/groupDetail/ParticipatedGroupDetail.tsx (4)
257-315: IIFE 패턴 대신 early return 또는 별도 컴포넌트 추출을 고려해 주세요.현재 렌더링 블록들(Lines 260-314, 327-349, 352-364)에서 IIFE
(() => { ... })()패턴이 반복적으로 사용되고 있습니다. 동작에는 문제가 없지만, JSX 내부의 IIFE는 가독성을 떨어뜨립니다.로딩/데이터 없음 상태를 early return으로 처리하면 이후
data가 항상 존재한다고 보장할 수 있어 IIFE 없이도 깔끔한 렌더링이 가능합니다.♻️ early return 패턴 예시
+ if (loading || !data) { + return ( + <ParticipatedWrapper> + <TopBackground genre=""> + <Header> + <IconButton src={leftArrow} onClick={handleBackButton} /> + </Header> + <BannerSkeleton /> + </TopBackground> + <GroupBookSectionSkeleton /> + <RecordSectionSkeleton /> + <CommentSectionSkeleton /> + <HotTopicSectionSkeleton /> + </ParticipatedWrapper> + ); + } // 이후 data가 항상 non-null이므로 IIFE 불필요🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/groupDetail/ParticipatedGroupDetail.tsx` around lines 257 - 315, The JSX currently uses an IIFE in ParticipatedGroupDetail to conditionally render when loading or data is missing; replace the IIFE blocks with an early-return check (e.g., if (loading || !data) return <BannerSkeleton />) at the top of the ParticipatedGroupDetail render body so the remaining JSX can assume data exists, and/or extract the large JSX fragment into a small child component (e.g., GroupDetailContent or similar) that receives props like roomName, isPublic, roomDescription, progressStartDate, progressEndDate, memberCount, category and uses handleMembersClick; update references to IconButton, MetaInfo, TagRow, etc., to render inside that component to remove the inline IIFE and improve readability.
291-303: 인라인 스타일 사용을 styled-component로 전환하는 것을 권장합니다.Lines 291, 293에서
style={{ display: 'flex', ... }}가 직접 사용되고 있습니다. 이 파일의 나머지 코드는 styled-components를 사용하고 있으므로, 일관성을 위해 styled-component로 추출하는 것이 좋습니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/groupDetail/ParticipatedGroupDetail.tsx` around lines 291 - 303, Replace the inline-styled container and span with new styled-components to match the file's pattern: create a styled wrapper (e.g., MetaColumn or MetaWrapper) for the outer div currently using style={{ display: 'flex', flexDirection: 'column', gap: '12px' }} and a styled span (e.g., MetaMemberRow) for the inner span that wraps MetaMember and MetaTotalMember; then swap the inline div/span with those styled components and keep existing elements like MetaTopRow, IconButton, MetaChevron, MetaMember, and MetaTotalMember unchanged.
352-364:isHost접근을 위한 IIFE는 과합니다.
data.isHost를 직접 참조하면 IIFE 없이 간결하게 작성할 수 있습니다.♻️ 제안
- {data && (() => { - const { isHost } = data; - return ( - <GroupActionBottomSheet - isOpen={isBottomSheetOpen} - isGroupOwner={isHost} - onClose={handleCloseBottomSheet} - onDeleteGroup={handleDeleteGroup} - onLeaveGroup={handleLeaveGroup} - onReportGroup={handleReportGroup} - /> - ); - })()} + {data && ( + <GroupActionBottomSheet + isOpen={isBottomSheetOpen} + isGroupOwner={data.isHost} + onClose={handleCloseBottomSheet} + onDeleteGroup={handleDeleteGroup} + onLeaveGroup={handleLeaveGroup} + onReportGroup={handleReportGroup} + /> + )}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/groupDetail/ParticipatedGroupDetail.tsx` around lines 352 - 364, The IIFE around rendering GroupActionBottomSheet is unnecessary; remove the self-invoking function and pass data.isHost directly. Locate the JSX that currently does {data && (() => { const { isHost } = data; return (<GroupActionBottomSheet ... isGroupOwner={isHost} .../>); })()} and simplify it to use data && <GroupActionBottomSheet isOpen={isBottomSheetOpen} isGroupOwner={data.isHost} onClose={handleCloseBottomSheet} onDeleteGroup={handleDeleteGroup} onLeaveGroup={handleLeaveGroup} onReportGroup={handleReportGroup} /> so you keep the same props (isBottomSheetOpen, handleCloseBottomSheet, handleDeleteGroup, handleLeaveGroup, handleReportGroup) but without the IIFE.
67-69: 두 개의 뒤로가기 핸들러가 존재하여 혼란을 줄 수 있습니다.
handleBackClick(Line 67,navigate(-1))은 에러 화면에서,handleBackButton(Line 110,navigate('/group'))은 정상 헤더에서 사용됩니다. 동작이 다르지만 이름이 비슷하여 유지보수 시 혼란을 줄 수 있습니다. 하나로 통일하거나, 역할이 다른 경우 이름을 명확히 구분해 주세요 (예:handleNavigateBackvshandleNavigateToGroup).Also applies to: 110-112
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/groupDetail/ParticipatedGroupDetail.tsx` around lines 67 - 69, There are two similar back handlers (handleBackClick which calls navigate(-1) and handleBackButton which calls navigate('/group')) that cause ambiguity; either unify behavior into a single handler or rename them to clearly reflect different intents (e.g., handleNavigateBack for navigate(-1) and handleNavigateToGroup for navigate('/group')). Update all usages of handleBackClick and handleBackButton in the component to the new names (or replace with the unified handler) so callers and tests unambiguously reflect whether they perform a history back or a fixed navigation to '/group'.src/pages/memory/Memory.tsx (1)
336-340: 인라인 스타일 대신 Styled Component 활용 권장 (선택적 개선)
ScrollableSection이 이미display: flex; flex-direction: column을 제공하므로, 내부div의 같은 속성 선언은 중복입니다.gap값도 인라인 스타일보다 styled component로 관리하면 실제MemoryContent내부의 레코드 목록 간격과의 정합성을 코드 레벨에서 확인하기 쉬워집니다.♻️ 개선 예시
- <div style={{ display: 'flex', flexDirection: 'column', gap: '40px' }}> - {Array.from({ length: 3 }).map((_, i) => ( - <RecordItemSkeleton key={i} /> - ))} - </div> + {Array.from({ length: 3 }).map((_, i) => ( + <RecordItemSkeleton key={i} /> + ))}그리고
ScrollableSection에gap을 추가하거나(공유 스타일이므로 영향 범위 확인 필요), 별도 래퍼 컴포넌트(SkeletonList등)를MemoryContent.styled.ts에 정의하는 방법도 있습니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/memory/Memory.tsx` around lines 336 - 340, The inline style on the wrapper div for the three RecordItemSkeletons duplicates styles provided by ScrollableSection and should be replaced with a styled component; remove the inline style usage around RecordItemSkeleton and either add a shared gap to ScrollableSection or create a new styled wrapper (e.g., SkeletonList) in MemoryContent.styled.ts that sets display:flex, flex-direction:column and gap:40px, then use that component in Memory.tsx to render the skeletons.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/pages/groupDetail/GroupDetail.tsx`:
- Around line 132-134: The error branch in the GroupDetail component returns a
bare div (if (error) { return <div>...);) leaving no back navigation; update
this branch to render the same header/back control used elsewhere (e.g., include
TitleHeader or Header as used in ParticipatedGroupDetail) so users can navigate
away from the error screen, preserving the existing error message display and
any props/callbacks required by TitleHeader/Header.
In `@src/pages/memory/Memory.tsx`:
- Around line 127-128: The current logic starts minLoadingTime alongside
getMemoryPosts using Promise.all, so if getMemoryPosts rejects the Promise.all
rejects immediately and the finally runs before minLoadingTime completes; move
the minLoadingTime creation outside and before the try block (create const
minLoadingTime = new Promise(resolve => setTimeout(resolve, 500));), then await
getMemoryPosts(params) inside try/catch to handle success/failure, and in the
finally always await minLoadingTime so the skeleton shows for at least 500ms;
update the code references around minLoadingTime, getMemoryPosts, Promise.all
and the surrounding try/finally accordingly.
---
Outside diff comments:
In `@src/pages/mypage/Mypage.tsx`:
- Around line 44-46: The catch block currently only logs errors so when
getMyProfile() fails loading becomes false but profile stays null, leaving the
UI stuck on the skeleton; add an error state (e.g., const [error, setError] =
useState<Error|null>(null)) and in the catch setError(error) (and optionally
setProfile(null) explicitly), then update the render condition from loading ||
!profile to loading || (!profile && !error) and render an error UI when error is
truthy so users see a recoverable error instead of the infinite skeleton;
reference getMyProfile, loading, profile, and the catch/finally block to locate
changes.
---
Duplicate comments:
In `@src/pages/groupDetail/ParticipatedGroupDetail.tsx`:
- Around line 216-226: The error UI must allow back navigation: in the
error-return branch ensure the TitleHeader remains rendered with leftIcon and
onLeftClick={handleBackClick} (as shown) so users can go back when an error
occurs; verify handleBackClick is defined in the component and that
ParticipatedWrapper and ErrorContainer are used to wrap the TitleHeader and
error message exactly as in the diff to preserve layout and accessibility.
In `@src/pages/mypage/Mypage.tsx`:
- Line 122: The img tag now includes an alt based on profile.nickname, but make
it robust against missing/empty nicknames: update the img element that uses
profile.profileImageUrl and profile.nickname (e.g., <img
src={profile.profileImageUrl} alt={`${profile.nickname} 프로필`} />) to provide a
safe fallback like `${profile.nickname || '사용자'} 프로필` (or use optional chaining
and a default) so the alt never becomes "undefined 프로필".
---
Nitpick comments:
In `@src/pages/groupDetail/GroupDetail.tsx`:
- Line 56: The initial value of the isLoading state in GroupDetail is currently
false; change its initialization so it starts true to reflect that the component
is loading on mount (aligning with ParticipatedGroupDetail). Update the useState
call that creates isLoading and setIsLoading to use true as the initial value,
and ensure any logic in the component (including the useEffect that fetches
roomData and calls setIsLoading) still sets isLoading to false after data load.
- Around line 337-340: Replace the ternary used for the BookCover src prop with
the simpler logical OR pattern: change the expression that currently checks
roomData.bookImageUrl ? roomData.bookImageUrl : bookCoverLargeImg to use
roomData.bookImageUrl || bookCoverLargeImg so BookCover (component) receives the
fallback image when bookImageUrl is falsy; update the src prop in the BookCover
JSX accordingly.
In `@src/pages/groupDetail/ParticipatedGroupDetail.tsx`:
- Around line 257-315: The JSX currently uses an IIFE in ParticipatedGroupDetail
to conditionally render when loading or data is missing; replace the IIFE blocks
with an early-return check (e.g., if (loading || !data) return <BannerSkeleton
/>) at the top of the ParticipatedGroupDetail render body so the remaining JSX
can assume data exists, and/or extract the large JSX fragment into a small child
component (e.g., GroupDetailContent or similar) that receives props like
roomName, isPublic, roomDescription, progressStartDate, progressEndDate,
memberCount, category and uses handleMembersClick; update references to
IconButton, MetaInfo, TagRow, etc., to render inside that component to remove
the inline IIFE and improve readability.
- Around line 291-303: Replace the inline-styled container and span with new
styled-components to match the file's pattern: create a styled wrapper (e.g.,
MetaColumn or MetaWrapper) for the outer div currently using style={{ display:
'flex', flexDirection: 'column', gap: '12px' }} and a styled span (e.g.,
MetaMemberRow) for the inner span that wraps MetaMember and MetaTotalMember;
then swap the inline div/span with those styled components and keep existing
elements like MetaTopRow, IconButton, MetaChevron, MetaMember, and
MetaTotalMember unchanged.
- Around line 352-364: The IIFE around rendering GroupActionBottomSheet is
unnecessary; remove the self-invoking function and pass data.isHost directly.
Locate the JSX that currently does {data && (() => { const { isHost } = data;
return (<GroupActionBottomSheet ... isGroupOwner={isHost} .../>); })()} and
simplify it to use data && <GroupActionBottomSheet isOpen={isBottomSheetOpen}
isGroupOwner={data.isHost} onClose={handleCloseBottomSheet}
onDeleteGroup={handleDeleteGroup} onLeaveGroup={handleLeaveGroup}
onReportGroup={handleReportGroup} /> so you keep the same props
(isBottomSheetOpen, handleCloseBottomSheet, handleDeleteGroup, handleLeaveGroup,
handleReportGroup) but without the IIFE.
- Around line 67-69: There are two similar back handlers (handleBackClick which
calls navigate(-1) and handleBackButton which calls navigate('/group')) that
cause ambiguity; either unify behavior into a single handler or rename them to
clearly reflect different intents (e.g., handleNavigateBack for navigate(-1) and
handleNavigateToGroup for navigate('/group')). Update all usages of
handleBackClick and handleBackButton in the component to the new names (or
replace with the unified handler) so callers and tests unambiguously reflect
whether they perform a history back or a fixed navigation to '/group'.
In `@src/pages/memory/Memory.tsx`:
- Around line 336-340: The inline style on the wrapper div for the three
RecordItemSkeletons duplicates styles provided by ScrollableSection and should
be replaced with a styled component; remove the inline style usage around
RecordItemSkeleton and either add a shared gap to ScrollableSection or create a
new styled wrapper (e.g., SkeletonList) in MemoryContent.styled.ts that sets
display:flex, flex-direction:column and gap:40px, then use that component in
Memory.tsx to render the skeletons.
In `@src/pages/mypage/Mypage.tsx`:
- Around line 42-43: The local const destructured as "profile" from Promise.all
shadows the component state variable "profile"; rename the local result (e.g.,
fetchedProfile or apiProfile) to avoid shadowing and then pass that to
setProfile. Specifically, change the destructured variable returned by
Promise.all (from getMyProfile() and minLoadingTime) to a distinct name and call
setProfile(fetchedName) so getMyProfile, minLoadingTime, and setProfile are used
without shadowing the state variable.
ℹ️ Review info
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
src/pages/groupDetail/GroupDetail.tsxsrc/pages/groupDetail/ParticipatedGroupDetail.tsxsrc/pages/memory/Memory.tsxsrc/pages/mypage/Mypage.styled.tssrc/pages/mypage/Mypage.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- src/pages/mypage/Mypage.styled.ts
Comment on lines
+127
to
+128
| const minLoadingTime = new Promise(resolve => setTimeout(resolve, 500)); | ||
| const [response] = await Promise.all([getMemoryPosts(params), minLoadingTime]); |
There was a problem hiding this comment.
에러 발생 시 최소 로딩 시간(500ms) 미보장 — 스켈레톤 순간 깜빡임 가능
getMemoryPosts가 빠르게 실패(throw)하면 Promise.all은 즉시 reject되고, finally가 바로 실행되어 스켈레톤이 500ms 미만으로만 표시될 수 있습니다. 성공 경로에서는 최소 로딩 시간이 보장되지만, 네트워크 오류 등 실패 경로에서는 보장되지 않습니다.
에러 경로에서도 일관된 최소 로딩 시간이 필요하다면, minLoadingTime을 try/catch 외부에서 병렬로 시작하고 finally에서 await하는 방식을 고려할 수 있습니다.
💡 최소 로딩 시간 보장 개선 예시 (선택 사항)
- setLoading(true);
+ setLoading(true);
+ const minLoadingTime = new Promise(resolve => setTimeout(resolve, 500));
try {
// ...
- const minLoadingTime = new Promise(resolve => setTimeout(resolve, 500));
- const [response] = await Promise.all([getMemoryPosts(params), minLoadingTime]);
+ const [response] = await Promise.all([getMemoryPosts(params), minLoadingTime]);
// ...
} catch (error) {
// ...
+ await minLoadingTime;
} finally {
setLoading(false);
}📝 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.
Suggested change
| const minLoadingTime = new Promise(resolve => setTimeout(resolve, 500)); | |
| const [response] = await Promise.all([getMemoryPosts(params), minLoadingTime]); | |
| setLoading(true); | |
| const minLoadingTime = new Promise(resolve => setTimeout(resolve, 500)); | |
| try { | |
| const [response] = await Promise.all([getMemoryPosts(params), minLoadingTime]); | |
| // ... rest of try block | |
| } catch (error) { | |
| // ... error handling | |
| await minLoadingTime; | |
| } finally { | |
| setLoading(false); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/pages/memory/Memory.tsx` around lines 127 - 128, The current logic starts
minLoadingTime alongside getMemoryPosts using Promise.all, so if getMemoryPosts
rejects the Promise.all rejects immediately and the finally runs before
minLoadingTime completes; move the minLoadingTime creation outside and before
the try block (create const minLoadingTime = new Promise(resolve =>
setTimeout(resolve, 500));), then await getMemoryPosts(params) inside try/catch
to handle success/failure, and in the finally always await minLoadingTime so the
skeleton shows for at least 500ms; update the code references around
minLoadingTime, getMemoryPosts, Promise.all and the surrounding try/finally
accordingly.
- A는 isLoading || 조건을 JSX 내부 삼항으로 분산 처리 → 가독성 저하 - B는 함수 상단 early return으로 skeleton 반환 후 data non-null 보장 - GroupDetail: BannerSkeleton/BookSkeleton 분기 제거, buttonProps·handlers 단순화
- 에러 발생 시 <div>만 반환해 사용자가 브라우저 뒤로가기 외 탈출 불가 - ParticipatedGroupDetail 에러 상태 TitleHeader 추가와 동일한 이슈 - Wrapper + Header + IconButton 구조로 에러 화면에도 뒤로가기 제공
heeeeyong
approved these changes
Feb 24, 2026
Collaborator
heeeeyong
left a comment
heeeeyong
left a comment
There was a problem hiding this comment.
최소 로딩 시간 확보해서 구현한 점 아주 좋은 것 같습니다~
모든 화면마다 추가하느라 너무 고생많으셨습니다 ㅠㅠ
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
3 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.
📝 작업 내용
서비스 전반에 걸쳐 일관된 로딩 UX를 제공하기 위해 Skeleton UI 시스템을 구축하고, 모든 주요 페이지에 적용했습니다. 기존의 텍스트 기반 로딩 메시지나 스피너 대신, 실제 컴포넌트 구조와 동일한 Skeleton UI를 표시하여 사용자에게 더 나은 로딩 경험을 제공합니다.
전체 적용 페이지 (총 13개 페이지)
1️⃣ 피드 관련 페이지 (5개)
2️⃣ 모임 관련 페이지 (4개)
3️⃣ 검색 관련 페이지 (3개)
4️⃣ 기타 페이지 (3개)
주요 변경사항
1. Skeleton UI 시스템 구축
Base Skeleton 컴포넌트 (
src/shared/ui/Skeleton/base/)Skeleton.Box: 사각형 스켈레톤Skeleton.Circle: 원형 스켈레톤 (프로필 이미지용)Skeleton.Text: 텍스트 스켈레톤 (단일/다중 라인 지원)폴더 구조 개선
2. Skeleton 컴포넌트
피드 관련
FeedPostSkeleton- 피드 게시글ProfileSkeleton- 프로필 영역OtherFeedSkeleton- 다른 사용자 피드TotalBarSkeleton- 전체 개수 표시 바모임 관련
GroupDetailSkeleton- 모임 상세 전체BannerSkeleton- 모임 배너BookSkeleton- 책 정보GroupBookSectionSkeleton- 모임 책 섹션RecordSectionSkeleton- 기록 섹션CommentSectionSkeleton- 댓글 섹션HotTopicSectionSkeleton- 핫토픽 섹션기타
MessageListSkeleton- 오늘의 한마디 메시지 목록검색 관련
RecordItemSkeleton- Memory 페이지 기록 아이템BookDetailSkeleton- 책 상세 페이지 배너 섹션BookItemSkeleton- 책 검색 결과 아이템MostSearchedBooksSkeleton- 인기 검색 도서 섹션RecentSearchTabsSkeleton- 최근 검색어 탭모임 관련
GroupCardSkeleton- 모임 카드 (type: main/search/modal, recommend 옵션 지원)MemberListSkeleton- 독서메이트 목록피드 관련
UserProfileItemSkeleton- 팔로워/팔로잉 목록 아이템 (type: followerlist/followlist)3. 기술적 개선사항
로딩 상태의 깜빡임 방지 → 시각적 안정성 확보
사용자가 로딩 상태를 충분히 인지할 수 있는 시간 제공
Feed 페이지에서는 최초 1초로 더 길게 적용 (깜빡임 완전 차단)
컴포넌트 구조 완벽 매칭
조건부 렌더링 최적화
GroupCardSkeleton: type(main/search/modal), isRecommendUserProfileItemSkeleton: type(followerlist/followlist)Summary by CodeRabbit