Merge refactor-skeleton โ refacgtor#311
Conversation
- LoadingSpinner ๋์ ์ฝํ ์ธ ๊ตฌ์กฐ๋ฅผ ๋ณด์ฌ์ฃผ๋ Skeleton UI ๋์ - shared/ui/Skeleton์ ์ฌ์ฌ์ฉ ๊ฐ๋ฅํ ์ปดํฌ๋ํธ ๊ตฌํ (Box/Circle/Text) - ํผ๋ ํ์ด์ง ๋ก๋ฉ ์ํ์ ์ค์ผ๋ ํค ์ ์ฉ์ผ๋ก UX ๊ฐ์
THIP-405: Skeleton UI ๊ณตํต ์ปดํฌ๋ํธ ๊ตฌํ
- ๊ธฐ์กด ๋น 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 ์ ๊ณต
- getMyProfile()(๋ก๊ทธ์ธ ์ฌ์ฉ์)๊ณผ getOtherFeed(userId)(URL ๊ธฐ์ค) ํผ์ฉ์ผ๋ก ๋ฐ์ดํฐ ๋ถ์ผ์น ๋ฐ์ - getOtherProfile(userId)๋ก ๊ต์ฒดํ์ฌ ํผ๋ยทํ๋กํ ๋ชจ๋ URL userId ๊ธฐ์ค์ผ๋ก ํต์ผ - isFollowing ์๋ ์ฃผ์ ์ ๊ฑฐ, API ์๋ต๊ฐ ๊ทธ๋๋ก ์ฌ์ฉ
- ์๋ฌ ๋ฐ์ ์ Header๊ฐ ์์ด ๋ธ๋ผ์ฐ์ ๋ค๋ก๊ฐ๊ธฐ ์ธ ํ์ถ ์๋จ ์์ - ์๋ฌ ์ํ์์๋ TitleHeader ๋ ๋๋งํ์ฌ ๋ค๋ก๊ฐ๊ธฐ ๋ฒํผ ์ ๊ณต - ๋ชจ๋ฐ์ผ ํ๊ฒฝ์์ ์ฌ์ฉ์๊ฐ ํ๋ฉด์ ๊ฐํ๋ UX ๋ฌธ์ ํด๊ฒฐ
- ๋ก๋ฉ ์ค RecordTabsยทRecordFilters ํด๋ฆญ ์ 2์ฐจ ์์ฒญ ๋ฐ์ โ stale ๋ฐ์ดํฐ ๋ ธ์ถ ๊ฐ๋ฅ - FixedSection์ pointerEvents: none ์ ์ฉ์ผ๋ก ๋ก๋ฉ ์ค ์ธํฐ๋์ ์ฐจ๋จ - AbortController ์์ด ๋จ์ํ ๋ฐฉ์ด ์ฒ๋ฆฌ๋ก Race Condition ๋ฐฉ์ง
- MenuItemSkeleton์ด ์ด๋์๋ ์ฌ์ฉ๋์ง ์์ dead code๋ก ์ ๊ฑฐ - ํ๋กํ img์ alt ์์ด ์คํฌ๋ฆฐ ๋ฆฌ๋๊ฐ ์ด๋ฏธ์ง ๋ชฉ์ ์ธ์ ๋ถ๊ฐ - profile.nickname ๊ธฐ๋ฐ alt ํ ์คํธ ์ถ๊ฐ๋ก ์ ๊ทผ์ฑ ๊ฐ์
- A๋ isLoading || ์กฐ๊ฑด์ JSX ๋ด๋ถ ์ผํญ์ผ๋ก ๋ถ์ฐ ์ฒ๋ฆฌ โ ๊ฐ๋ ์ฑ ์ ํ - B๋ ํจ์ ์๋จ early return์ผ๋ก skeleton ๋ฐํ ํ data non-null ๋ณด์ฅ - GroupDetail: BannerSkeleton/BookSkeleton ๋ถ๊ธฐ ์ ๊ฑฐ, buttonPropsยทhandlers ๋จ์ํ
- ์๋ฌ ๋ฐ์ ์ <div>๋ง ๋ฐํํด ์ฌ์ฉ์๊ฐ ๋ธ๋ผ์ฐ์ ๋ค๋ก๊ฐ๊ธฐ ์ธ ํ์ถ ๋ถ๊ฐ - ParticipatedGroupDetail ์๋ฌ ์ํ TitleHeader ์ถ๊ฐ์ ๋์ผํ ์ด์ - Wrapper + Header + IconButton ๊ตฌ์กฐ๋ก ์๋ฌ ํ๋ฉด์๋ ๋ค๋ก๊ฐ๊ธฐ ์ ๊ณต
refactor-skeleton-global: ์ฃผ์ ํ์ด์ง ์ ์ญ Skeleton UI ์ ์ฉ ๋ฐ ๋ก๋ฉ ๊ฒฝํ ๊ฐ์
|
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 (5)
Walkthrough์ ์ญ์ ์ผ๋ก ์ฌ์ฌ์ฉ ๊ฐ๋ฅํ Skeleton ์ปดํฌ๋ํธ๊ตฐ์ ์ถ๊ฐํ๊ณ , ์ฌ๋ฌ ํ์ด์ง/์ปดํฌ๋ํธ์ ๊ธฐ์กด ์คํผ๋/ํ ์คํธ ๋ก๋๋ฅผ ์ค์ผ๋ ํค UI๋ก ๊ต์ฒดํ๋ฉฐ ํญ/์ด๊ธฐ ๋ก๋ฉ์ 500ms ์ต์ ์ง์ฐ์ Promise.all๋ก ๋ณ๋ ฌ ์ ์ฉํฉ๋๋ค. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Page as PageComponent
participant API as RemoteAPI
participant Delay as MinLoading(500ms)
participant Skeleton as SkeletonUI
participant Content as FinalContent
User->>Page: ์ง์
๋๋ ํญ ์ ํ
Page->>Page: isLoading = true
Page->>Skeleton: ์ค์ผ๋ ํค ๋ ๋๋ง (์ด๊ธฐ ๋ทฐ)
par ๋ณ๋ ฌ ์คํ
Page->>API: ๋ฐ์ดํฐ ์์ฒญ (fetch)
Page->>Delay: 500ms ํ์ด๋จธ ์์
end
API-->>Page: ๋ฐ์ดํฐ ์๋ต
Delay-->>Page: 500ms ๋ง๋ฃ
Page->>Page: isLoading = false
Page->>Content: ์ค์ ์ฝํ
์ธ ๋ ๋๋ง
Content-->>User: ํ๋ฉด ํ์
Estimated code review effort๐ฏ 4 (Complex) | โฑ๏ธ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
๐ฅ Pre-merge checks | โ 1 | โ 2โ Failed checks (1 warning, 1 inconclusive)
โ Passed checks (1 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 |
There was a problem hiding this comment.
Actionable comments posted: 16
Caution
Some comments are outside the diff and canโt be posted inline due to platform limitations.
โ ๏ธ Outside diff range comments (5)
src/pages/feed/MyFeedPage.tsx (1)
75-77:โ ๏ธ Potential issue | ๐ Major์๋ฌ ์ํ์์ ๋น ํ๋ฉด(
<></>) ๋ฐํ โ ์ฌ์ฉ์ ํผ๋๋ฐฑ ๋ฐ ๋ค๋ก๊ฐ๊ธฐ ์๋จ ์์์ค๋ฅ ๋ฐ์ ์ ๋น ํ๋๊ทธ๋จผํธ๋ฅผ ๋ฐํํ๋ฉด ์ฌ์ฉ์์๊ฒ ์๋ฌด๋ฐ ์๋ด ์์ด ๋น ํ๋ฉด๋ง ํ์๋ฉ๋๋ค. ๋ค๋ก๊ฐ๊ธฐ ๋ฒํผ์ด๋ ์ค๋ฅ ๋ฉ์์ง ์์ด๋ ์ฌ์ฉ์๊ฐ ํ์ด์ง๋ฅผ ๋ฒ์ด๋ ๋ฐฉ๋ฒ์ด ์์ต๋๋ค. PR ์ปค๋ฐ ๋ฉ์์ง์์๋
GroupDetail/ParticipatedGroupDetail์ ์๋ฌ ์ํ์ ๋ค๋ก๊ฐ๊ธฐ ๋ค๋น๊ฒ์ด์ ์ ์ถ๊ฐํ๋ค๊ณ ์ธ๊ธํ๊ณ ์์ด, ๋์ผํ ํจํด์ด ์ฌ๊ธฐ์๋ ์ ์ฉ๋์ด์ผ ํฉ๋๋ค.๐ ์์ ์ ์
- if (error) { - return <></>; - } + if (error) { + return ( + <Container> + <TitleHeader + leftIcon={<img src={leftArrow} alt="๋ค๋ก๊ฐ๊ธฐ" />} + onLeftClick={handleBackClick} + /> + <div style={{ padding: '20px', textAlign: 'center', color: 'gray' }}> + ์ฌ์ฉ์ ์ ๋ณด๋ฅผ ๋ถ๋ฌ์ค๋๋ฐ ์คํจํ์ต๋๋ค. + </div> + <NavBar src={writefab} path="/post/create" /> + </Container> + ); + }๐ค 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 75 - 77, The current MyFeedPage returns an empty fragment on error (if (error) return <></>), leaving users stranded; replace that with the same error UI + back navigation used in GroupDetail/ParticipatedGroupDetail: render a user-friendly error message and a visible "Back" control that triggers the page navigation (e.g., the same BackButton or useNavigate/navigation callback used elsewhere) and preserve any error details for logging or debugging; update the error branch inside the MyFeedPage component to mirror the error-handling pattern from GroupDetail/ParticipatedGroupDetail instead of returning an empty fragment.src/pages/feed/FollowerListPage.tsx (1)
61-70:โ ๏ธ Potential issue | ๐ Major
responsenull ์ฒดํฌ ์์๊ฐ ์๋ชป๋์์ต๋๋ค โ ์ด๋ฏธ ๋ฐ์ดํฐ๋ฅผ ์ ๊ทผํ ์ดํ์ ์ฒดํฌํฉ๋๋ค.
response.data๊ฐ lines 62, 64์์ ๋จผ์ ์ ๊ทผ๋ ๋ค lines 67-70์์ null ์ฒดํฌ๊ฐ ์ํ๋ฉ๋๋ค. ๋ง์ฝresponse๋response.data๊ฐ falsy๋ผ๋ฉด line 62์์ ์ด๋ฏธ ๋ฐํ์ ์ค๋ฅ๊ฐ ๋ฐ์ํฉ๋๋ค. null ์ฒดํฌ๋ฅผ ๋ฐ์ดํฐ ์ ๊ทผ ์ด์ ์ผ๋ก ์ด๋ํ๊ฑฐ๋, ๋ถํ์ํ๋ค๋ฉด ์ ๊ฑฐํด์ผ ํฉ๋๋ค.๐ ์์ ์ ์
+ if (!response || !response.data) { + setError('API ์๋ต์ด ์์ต๋๋ค.'); + return; + } + let userData: FollowData[] = []; if (type === 'followerlist') { userData = (response.data as { followers: FollowData[] })?.followers || []; } else { userData = (response.data as { followings: FollowData[] })?.followings || []; } - if (!response || !response.data) { - setError('API ์๋ต์ด ์์ต๋๋ค.'); - return; - } -๐ค Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/feed/FollowerListPage.tsx` around lines 61 - 70, Move the null-check for response/response.data before any access to response.data to avoid runtime errors: check if response and response.data are truthy (e.g., if (!response || !response.data) { setError('API ์๋ต์ด ์์ต๋๋ค.'); return; }) before computing userData, then populate userData using the existing type switch that references (response.data as { followers: FollowData[] }) or followings; alternatively replace direct access with safe optional chaining when assigning userData (e.g., response?.data?.followers) but ensure setError/return still runs when data is missing; update the logic around the variables response, userData, type and setError in FollowerListPage.tsx accordingly.src/pages/groupSearch/GroupSearch.tsx (1)
50-59:โ ๏ธ Potential issue | ๐ Major์ต๊ทผ ๊ฒ์์ด๋ฅผ ์ด๊ธฐ ๋ ๋์์ ์ค๋ณต ํธ์ถํ๊ณ ์์ต๋๋ค.
Line 50-53๊ณผ Line 56-58์ด ๋ง์ดํธ ์์ ์ ๋์์ ์คํ๋์ด
fetchRecentSearches()๊ฐ 2ํ ํธ์ถ๋ฉ๋๋ค(searchStatus์ด๊ธฐ๊ฐ์ดidle). ์ด๊ธฐ ์์ฒญ ์ค๋ณต๊ณผ ๋ก๋ฉ ์ํ ๊น๋นก์์ด ๋ฐ์ํ ์ ์์ต๋๋ค.๐ง ์ ์ ์์ ์
- 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, The useEffect hooks call fetchRecentSearches() twice on mount because searchStatus starts as 'idle'โfix by consolidating the logic: keep a single useEffect (the one with empty deps or the one watching searchStatus) and add a guard so fetchRecentSearches() is invoked only when searchStatus === 'idle' (or remove the effect that watches searchStatus entirely and rely on the mount effect with the status check); update references to useEffect, fetchRecentSearches, and searchStatus accordingly to avoid duplicate initial requests and loading flicker.src/pages/groupDetail/GroupDetail.tsx (1)
55-57:โ ๏ธ Potential issue | ๐ก Minor
isLoading์ด๊ธฐ๊ฐ์ดfalse๋ก ์ค์ ๋์ดParticipatedGroupDetail๊ณผ ๋ถ์ผ์นํฉ๋๋ค.
ParticipatedGroupDetail.tsx์์๋isLoading์ดtrue๋ก ์ด๊ธฐํ๋์ง๋ง, ์ฌ๊ธฐ์๋false์ ๋๋ค.!roomData๊ฐ๋ ๋๋ถ์ ์ค์ผ๋ ํค์ด ํ์๋๊ธด ํ์ง๋ง,setIsLoading(true)๊ฐ ์คํ๋๊ธฐ ์ ์ฒซ ๋ ๋๋ง์์isLoading์ดfalse์ธ ์ํ๊ฐ ์กด์ฌํฉ๋๋ค. ์ผ๊ด์ฑ์ ์ํด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` around lines 55 - 57, The isLoading state in GroupDetail (useState for isLoading) is initialized to false which disagrees with ParticipatedGroupDetail; change the initial value to true so the first render reflects loading semantics (update the useState<boolean> call that declares isLoading and setIsLoading in GroupDetail.tsx to use true) to keep behavior consistent with ParticipatedGroupDetail and avoid a non-loading initial render before setIsLoading(true) runs.src/pages/groupDetail/ParticipatedGroupDetail.tsx (1)
56-63:โ ๏ธ Potential issue | ๐ก Minor
isLoading์ ์ด๊ธฐ๊ฐ์ดtrue๋ก ์ค์ ๋์ด ์์ด, ์ฒซ ๋ ๋๋ง ์ ์ค์ผ๋ ํค์ด ๋ฐ๋ก ํ์๋ฉ๋๋ค.์ด ๋ถ๋ถ์ ์ฌ๋ฐ๋ฅธ ์ ๊ทผ์ ๋๋ค.
GroupDetail.tsx์์๋isLoading์ ์ด๊ธฐ๊ฐ์ดfalse๋ก ์ค์ ๋์ด ์์ด (Line 56)!roomData๊ฐ๋์๋ง ์์กดํ๊ฒ ๋ฉ๋๋ค. ๋ ํ์ด์ง ๊ฐ ์ด๊ธฐ๊ฐ์ด ๋ค๋ฅธ ์ ์ด ์๋๋ ๊ฒ์ธ์ง ํ์ธํด ์ฃผ์ธ์ โGroupDetail.tsx์์๋ ์ฒซ ๋ ๋๋ง ์ ์ค์ผ๋ ํค ๋์ ๋น ํ๋ฉด์ด ์๊ฐ์ ์ผ๋ก ๋ํ๋ ์ ์์ต๋๋ค.๐ค 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 56 - 63, The ParticipatedGroupDetail component initializes loading as true which immediately shows the skeleton on first render; make the initial loading value consistent with GroupDetail.tsx by setting the loading state (const [loading, setLoading]) to false (or align both components' intent) and rely on the !roomData guard for initial empty-render behavior, updating setLoading(true/false) only around your async fetch logic (functions referenced: ParticipatedGroupDetail, setLoading, roomData) so both pages behave consistently.
โป๏ธ Duplicate comments (3)
src/components/search/MostSearchedBooks.tsx (1)
30-34: ์ต์ ๋ก๋ฉ ๋ณด์ฅ ๋ก์ง ์ด์๊ฐ ๋ฐ๋ณต๋ฉ๋๋ค.์ด ๊ตฌ๊ฐ๋ API ์คํจ ์
Promise.all์ด ์ฆ์ reject๋์ด ์ค์ผ๋ ํค ์ต์ ๋ ธ์ถ ์๊ฐ์ด ๋ณด์ฅ๋์ง ์์ต๋๋ค.๐ค Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/search/MostSearchedBooks.tsx` around lines 30 - 34, The Promise.all usage with getMostSearchedBooks and minLoadingTime can reject immediately when the API fails, breaking the guaranteed skeleton display; change the logic to await both promises regardless of API failure (e.g., use Promise.allSettled([getMostSearchedBooks(), minLoadingTime]) and then extract the fulfilled value from the first result or fall back to safe empty data), or wrap getMostSearchedBooks() with a catch that returns a sentinel/fallback so Promise.all always resolves; update references in this component (minLoadingTime, getMostSearchedBooks) accordingly.src/pages/feed/OtherFeedPage.tsx (1)
37-46: ์ต์ ๋ก๋ฉ ๋ณด์ฅ ๋ก์ง ์ด์๊ฐ ๋์ผํฉ๋๋ค.์ฌ๊ธฐ๋
Promise.allfail-fast ํน์ฑ ๋๋ฌธ์ ์คํจ ์ 500ms ๋ณด์ฅ์ด ๊นจ์ง ์ ์์ต๋๋ค. ๋์ผ ํจํด์ผ๋ก ๋ณด์ ํด ์ฃผ์ธ์.๐ค 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 current Promise.all with getOtherFeed, getOtherProfile and minLoadingTime can fail-fast and skip the 500ms guarantee; wrap the API calls so the timing promise always resolves independentlyโe.g., use Promise.allSettled or attach .catch handlers to getOtherFeed/getOtherProfile before awaitingโthen after awaiting, check results and throw or handle errors and extract values into feedResponse/profileResponse; update the block referencing getOtherFeed, getOtherProfile, minLoadingTime, feedResponse and profileResponse accordingly.src/pages/today-words/TodayWords.tsx (1)
94-101: ์ต์ ๋ก๋ฉ ๋ณด์ฅ ๋ก์ง ์ด์๊ฐ ๋์ผํ๊ฒ ๋ฐ๋ณต๋ฉ๋๋ค.
Promise.all๊ธฐ๋ฐ์ด๋ผ API ์คํจ ์ ์ต์ 500ms ๋ ธ์ถ์ด ๋ณด์ฅ๋์ง ์์ต๋๋ค. ์์์ ์ง์ ํ ํจํด๊ณผ ๋์ผํ๊ฒ ์ ๋ฆฌํ๋ ๊ฒ์ ๊ถ์ฅํฉ๋๋ค.๐ค 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, The current Promise.all usage with minLoadingTime plus getDailyGreeting means if getDailyGreeting rejects the 500ms delay may be skipped; change to ensure minLoadingTime is always awaited by either awaiting minLoadingTime in a finally block or by converting the API call into a non-rejecting promise and then using Promise.all/Promise.allSettled. Concretely, wrap getDailyGreeting(roomId: parseInt(roomId), cursor: cursor || undefined) into a promise that catches errors (so it never rejects) or use Promise.allSettled for [getDailyGreeting(...), minLoadingTime] and then handle the settled result for the API call (check for rejection and process accordingly); update references in TodayWords to use the new safe API result handling and keep isRefresh logic the same.
๐งน Nitpick comments (16)
src/shared/ui/Skeleton/base/Skeleton.styled.ts (2)
54-56:SkeletonTextLine์์height๋ณด๊ฐ ์ค๋ณต ์ ์ธ
SkeletonBase์์ ์ด๋ฏธheightprop ๋ณด๊ฐ ๋ก์ง์ ์ฒ๋ฆฌํฉ๋๋ค.SkeletonTextLine์ด ์ด๋ฅผ ์ฌ์ ์ธํ๋ ์ ์ผํ ์ด์ ๋ ๊ธฐ๋ณธ๊ฐ์20px์์16px๋ก ๋ณ๊ฒฝํ๊ธฐ ์ํจ์ ๋๋ค. ์ด ํจํด์ ํผ๋์ค๋ฌ์ธ ์ ์์ผ๋ฉฐ, ํฅํSkeletonBase์ height ๋ก์ง์ด ๋ณ๊ฒฝ๋ ๊ฒฝ์ฐ ๋ ๊ณณ์ ๋ชจ๋ ์์ ํด์ผ ํฉ๋๋ค.โป๏ธ ๊ธฐ๋ณธ๊ฐ๋ง ๋ช ์์ ์ผ๋ก ๋ถ๋ฆฌํ๋ ๋ฐฉ์ ์ ์
-export const SkeletonTextLine = styled(SkeletonBase)` - height: ${({ height }) => (typeof height === 'number' ? `${height}px` : height || '16px')}; - border-radius: 4px; -`; +export const SkeletonTextLine = styled(SkeletonBase).attrs<SkeletonBaseProps>(({ height }) => ({ + height: height ?? 16, +}))` + border-radius: 4px; +`;๐ค Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/shared/ui/Skeleton/base/Skeleton.styled.ts` around lines 54 - 56, SkeletonTextLine duplicates the height interpolation already handled by SkeletonBase; remove the redundant height handling from SkeletonTextLine and instead rely on SkeletonBase's logic while only overriding the default value (from 20px to 16px) via a single source of truthโe.g., pass a default prop or theme/variable used by SkeletonBaseโso update SkeletonTextLine to stop re-declaring height and supply the desired default height through SkeletonBase's prop/default mechanism (reference SkeletonTextLine and SkeletonBase and the height prop).
22-36:display: inline-block์ผ๋ก ์ธํ ๋น(้) flex ์ปจํ ์ด๋์์์ ์๊ธฐ์น ์์ ๋ ์ด์์ ๊ฐ๋ฅ์ฑ
SkeletonBase๊ฐ flex/grid๊ฐ ์๋ ์ผ๋ฐ ๋ธ๋ก ์ปจํ ์ด๋์ ์์์ผ๋ก ๋ฐฐ์น๋ ๊ฒฝ์ฐ,inline-blockํน์ฑ ๋๋ฌธ์ ์ฌ๋ฌ ์ค์ผ๋ ํค ์์๊ฐ ์ธ๋ก๋ก ์์ด์ง ์๊ณ ๊ฐ๋ก๋ก ๋๋ํ ํ๋ฅผ ์ ์์ต๋๋ค. flex ์ปจํ ์คํธ ๋ด์์๋ ์ธ๋ถ ๋์คํ๋ ์ด ์ ํ์ด ์ฌ์ ์๋๋ฏ๋ก ๋ฌธ์ ๊ฐ ์์ง๋ง, ์ฝ๋๋ฒ ์ด์ค ์ ๋ฐ์ ๊ฑธ์ณ ๋น flex ์ปจํ ์คํธ์์๋ ์ฌ์ฉ ๊ฐ๋ฅ์ฑ์ด ์๋ค๋ฉดdisplay: block์ด ๋ ์์ธก ๊ฐ๋ฅํ ๊ธฐ๋ณธ๊ฐ์ ๋๋ค.โป๏ธ ์ ์: block์ผ๋ก ๋ณ๊ฒฝ
export const SkeletonBase = styled.div<SkeletonBaseProps>` - display: inline-block; + display: block; width: ${({ width }) => (typeof width === 'number' ? `${width}px` : width || '100%')};๐ค Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/shared/ui/Skeleton/base/Skeleton.styled.ts` around lines 22 - 36, The SkeletonBase styled component currently uses display: inline-block which can cause multiple skeletons to flow horizontally in non-flex/grid parents; update SkeletonBase to use display: block so it behaves as a predictable full-width block element in plain containers, keeping the existing width/height/borderRadius logic and animations intact (locate SkeletonBase in src/shared/ui/Skeleton/base/Skeleton.styled.ts and replace the display rule from inline-block to block).src/shared/ui/Skeleton/search/RecentSearchTabsSkeleton.tsx (1)
4-15: ์ค์ผ๋ ํค ๋ด ํ๋์ฝ๋ฉ๋ ํ์ดํ ํ ์คํธ โ ์ค์ ์ปดํฌ๋ํธ์ ํ ์คํธ ๋๊ธฐํ ํ์
Title์ "์ต๊ทผ ๊ฒ์์ด"๊ฐ ๋ฆฌํฐ๋ด๋ก ํ๋์ฝ๋ฉ๋์ด ์์ต๋๋ค. ์ค์ RecentSearchTabs์ปดํฌ๋ํธ์ ํ์ดํ ํ ์คํธ๊ฐ ๋ณ๊ฒฝ๋๋ฉด ์ด ์ค์ผ๋ ํค๋ ํจ๊ป ์๋์ผ๋ก ์ ๋ฐ์ดํธํด์ผ ํฉ๋๋ค. ํ์ดํ๋ ์ค์ผ๋ ํค์ผ๋ก ๋์ฒดํ๊ฑฐ๋, ์ค์ ์ปดํฌ๋ํธ์์ ์์๋ฅผ ๊ณต์ ํ๋ ๋ฐฉ์์ ๊ณ ๋ คํ์ธ์.๐ค Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/shared/ui/Skeleton/search/RecentSearchTabsSkeleton.tsx` around lines 4 - 15, RecentSearchTabsSkeleton currently hardcodes the Title text ("์ต๊ทผ ๊ฒ์์ด"), causing drift if RecentSearchTabs' title changes; update RecentSearchTabsSkeleton to avoid literal text by either rendering the same shared constant used by RecentSearchTabs (import the title constant) or rendering a Skeleton placeholder for the title (replace Title's literal children with a Skeleton.Box/Skeleton.Text), targeting the RecentSearchTabsSkeleton component and the Title element so the skeleton stays in sync with the real component.src/shared/ui/Skeleton/searchBook/BookItemSkeleton.tsx (1)
18-23: ์คํ์ด์ฑ์ ์ํ ์ธ๋ผ์ธ ์คํ์ผdiv์ฌ์ฉ โ styled-component ํจํด๊ณผ ๋ถ์ผ์น์ธ ๋ฒ์งธ ์ค์ผ๋ ํค ํ ์คํธ ๋ผ์ธ์ ์๋จ ์ฌ๋ฐฑ์ ์ํด bare
<div style={{ marginTop: '8px' }}>๋ฅผ ์ฌ์ฉํฉ๋๋ค. ๋์ผ ์ปดํฌ๋ํธ ๋ด ๋ค๋ฅธ ๋ ์ด์์์ด styled-component๋ฅผ ํตํด ์ฒ๋ฆฌ๋๋ ๊ฒ๊ณผ ์ผ๊ด์ฑ์ด ์์ต๋๋ค.Skeleton.Text์margin๋๋styleprop์ ์ง์ ์ ๋ฌํ๊ฑฐ๋,BookInfo๋ด๋ถ ์คํ์ผ๋ก ์ฒ๋ฆฌํ๋ ๋ฐฉ์์ ๊ถ์ฅํฉ๋๋ค.โป๏ธ ์ ์
- <div style={{ marginTop: '8px' }}> - <Skeleton.Text width={150} height={12} /> - </div> + <Skeleton.Text width={150} height={12} style={{ marginTop: '8px' }} />๋จ,
Skeleton.Text๊ฐstyleprop์ ์ง์ํ์ง ์๋ ๊ฒฝ์ฐ,SkeletonProps์style?: React.CSSProperties๋ฅผ ์ถ๊ฐํ๊ฑฐ๋ ๋ํผ styled-component๋ฅผ ์ฌ์ฉํ์ธ์.๐ค Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/shared/ui/Skeleton/searchBook/BookItemSkeleton.tsx` around lines 18 - 23, ํ์ฌ BookItemSkeleton.tsx์์ BookInfo ๋ด๋ถ์ bare <div style={{ marginTop: '8px' }}>๋ก ์ธ๋ผ์ธ ์คํ์ผ์ ์ฌ์ฉํ๊ณ ์์ด styled-component ํจํด๊ณผ ๋ถ์ผ์นํฉ๋๋ค; BookInfo ์์ ์ธ ๋ฒ์งธ Skeleton.Text ์๋จ ์ฌ๋ฐฑ์ ์ํด inline div๋ฅผ ์ ๊ฑฐํ๊ณ ๋์ Skeleton.Text์ margin/top ์คํ์ผ์ ์ ๋ฌํ๊ฑฐ๋(๋ง์ฝ Skeleton.Text๊ฐ style prop์ ์ง์ํ์ง ์์ผ๋ฉด SkeletonProps์ style?: React.CSSProperties๋ฅผ ์ถ๊ฐ) ๋๋ BookInfo ๋ด๋ถ์ ๋ํผ styled-component(์: InfoGap ๋๋ StyledSkeletonWrap)๋ฅผ ๋ง๋ค์ด margin-top์ ์ ์ฉํ๋๋ก ๋ณ๊ฒฝํ์ธ์; ์ฐธ์กฐํ ์๋ณ์๋ BookItemSkeleton.tsx, BookInfo, Skeleton.Text์ ๋๋ค.src/shared/ui/Skeleton/base/Skeleton.tsx (1)
40-44:Skeleton.Box์ ๊ธฐ๋ณธSkeleton์ ์ค๋ณต
Skeleton(base)๊ณผSkeleton.Box๋ ์๋ก ๋ค๋ฅธ styled ์ปดํฌ๋ํธ(SkeletonBasevsSkeletonBox)๋ฅผ ์ฌ์ฉํ์ง๋ง,SkeletonBox๋SkeletonBase์ ๋น ํ์ฅ(styled(SkeletonBase)```)์ผ๋ก ์ ์๋์ด ์์ด ์ค์ง์ ์ผ๋ก ๋์ผํฉ๋๋ค. ๋ API ํ๋ฉด ๊ฐ์ ์๋ฏธ์ ๊ตฌ๋ถ์ด ์์ผ๋ฏ๋ก, ์ธ๋ถ ์๋น์๊ฐ ๋ ์ค ์ด๋ ๊ฒ์ ์ฌ์ฉํด์ผ ํ๋์ง ํผ๋์ค๋ฌ์ธ ์ ์์ต๋๋ค.Skeleton.Box๋ฅผ ์ ์ผํ ๋ช ์์ ์ง์ฌ๊ฐํ ์ค์ผ๋ ํค API๋ก ์ฌ์ฉํ๊ณ ๊ธฐ๋ณธSkeleton` export๋ ๋ด๋ถ ์ ์ฉ์ผ๋ก ์ ํํ๋ ๊ฒ์ ๊ณ ๋ คํ์ธ์.๐ค Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/shared/ui/Skeleton/base/Skeleton.tsx` around lines 40 - 44, The base and box APIs are redundant: SkeletonBox is just a styled(SkeletonBase) empty extension and Skeleton.Box duplicates functionality; make the rectangular API explicit by keeping Skeleton.Box (Skeleton.Box component) as the public export and restrict the base Skeleton (Skeleton and SkeletonBase) to internal use onlyโremove or stop exporting the top-level Skeleton component, keep SkeletonBox/Skeleton.Box as the single public rectangle API, update any export lists and consume-sites to import Skeleton.Box instead of Skeleton, and add a deprecation note/alias if needed to preserve backwards compatibility.src/shared/ui/Skeleton/memory/RecordItemSkeleton.tsx (1)
12-36: ์ธ๋ผ์ธ ์คํ์ผ ์ผ๋ถ๋ styled ์ปดํฌ๋ํธ๋ก ๋ถ๋ฆฌํ๋ฉด ์ ์ง๋ณด์๊ฐ ๋ ์ฌ์์ง๋๋ค.ํ์ฌ ๋ฐ๋ณต๋๋ ์ธ๋ผ์ธ ์คํ์ผ(
div3๊ณณ)์ ์ถํ ์ค์ผ๋ ํค ๋์์ธ ๋ณ๊ฒฝ ์ ์์ ํฌ์ธํธ๋ฅผ ๋๋ฆฝ๋๋ค. ๊ฒฝ๋ ๋ํผ๋ก ์ถ์ถํด ๋๋ฉด ์ผ๊ด์ฑ ๊ด๋ฆฌ๊ฐ ๋ ํธํด์ง๋๋ค.๐ค Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/shared/ui/Skeleton/memory/RecordItemSkeleton.tsx` around lines 12 - 36, The three repeated inline divs inside the RecordItemSkeleton (the ones wrapping Skeleton.Box + Skeleton.Text in ActionSection and the circle wrapper in UserSection) should be replaced with a lightweight styled component to remove duplicated inline styles; create a styled component (e.g., IconRow or InlineRow) and use it in place of the divs in Container/UserSection/ActionSection so styles like display:flex, gap, alignItems and marginRight are centralized and applied consistently across the component.src/shared/ui/Skeleton/group/HotTopicSectionSkeleton.tsx (1)
15-22: ์ธ๋ผ์ธ ์คํ์ผ์ styled-component๋ก ๋ถ๋ฆฌํ๋ ๊ฒ์ ๊ถ์ฅํฉ๋๋ค.๋ด๋ถ ๋ํผ
div๋ค์ ์ธ๋ผ์ธ ์คํ์ผ์ด ์ฌ์ฉ๋๊ณ ์์ผ๋, ํด๋น ํ์ผ์ ๋๋จธ์ง ๋ถ๋ถ์GroupDetailSkeleton.styled๊ธฐ๋ฐ์ styled-component๋ฅผ ํ์ฉํฉ๋๋ค. ์ธ๋ผ์ธ ์คํ์ผ ๊ฐ์ฒด๋ ๋งค ๋ ๋๋ง๋ค ์๋ก ์์ฑ๋๊ณ ์ฝ๋ ์ผ๊ด์ฑ๋ ๋ฎ์์ง๋๋ค.โป๏ธ ๋ฆฌํฉํฐ๋ง ์ ์
HotTopicSectionSkeleton.styled.ts(๋๋GroupDetailSkeleton.styled.ts)์ ์๋ ์คํ์ผ์ ์ถ๊ฐํ๊ณ ์ธ๋ผ์ธ ์คํ์ผ์ ๊ต์ฒดํฉ๋๋ค:+export const HotTopicContentPadding = styled.div` + padding: 16px 0; +`; + +export const HotTopicContentTitle = styled.div` + margin-bottom: 16px; +`; + +export const HotTopicBoxList = styled.div` + display: flex; + flex-direction: column; + gap: 8px; +`;- <div style={{ padding: '16px 0' }}> - <div style={{ marginBottom: '16px' }}> - <Skeleton.Text width="80%" height={16} /> - </div> - <div style={{ display: 'flex', flexDirection: 'column', gap: '8px' }}> - <Skeleton.Box width="100%" height={40} borderRadius={8} /> - <Skeleton.Box width="100%" height={40} borderRadius={8} /> - </div> - </div> + <HotTopicContentPadding> + <HotTopicContentTitle> + <Skeleton.Text width="80%" height={16} /> + </HotTopicContentTitle> + <HotTopicBoxList> + <Skeleton.Box width="100%" height={40} borderRadius={8} /> + <Skeleton.Box width="100%" height={40} borderRadius={8} /> + </HotTopicBoxList> + </HotTopicContentPadding>๐ค 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 - 22, HotTopicSectionSkeleton currently uses inline style objects on the outer wrapper and two inner divs; extract those into styled-components (e.g., create StyledHotTopicWrapper, StyledTitleWrapper, StyledListWrapper in HotTopicSectionSkeleton.styled.ts or reuse GroupDetailSkeleton.styled) and replace the inline divs in the HotTopicSectionSkeleton component with these styled components, preserving the same CSS (padding: 16px 0; margin-bottom: 16px; and display:flex; flex-direction:column; gap:8px;) and keep existing Skeleton children and props unchanged.src/shared/ui/Skeleton/members/MemberListSkeleton.tsx (1)
16-18:Skeleton.Circle์ฃผ๋ณ์ ๋ถํ์ํ ๋ํผ<div>์ ๊ฑฐ๋ฅผ ๊ถ์ฅํฉ๋๋ค.
ProfileSection์ ์ด๋ฏธdisplay: flex; align-items: center; gap: 8px๋ ์ด์์์ ๊ฐ์ถ๊ณ ์์ดSkeleton.Circle์ ์ง์ ๋ฐฐ์นํ ์ ์์ต๋๋ค. ๋น<div>๋ ๋ถํ์ํ DOM ๋ ธ๋๋ฅผ ์ถ๊ฐํฉ๋๋ค.โป๏ธ ๋ฆฌํฉํฐ๋ง ์ ์
<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 the MemberListSkeleton component: because ProfileSection already applies display:flex, align-items:center and gap:8px, place Skeleton.Circle directly as a child (remove the extra div node) so the DOM is simpler and layout remains unchanged; update the JSX in MemberListSkeleton (where Skeleton.Circle is rendered) to render Skeleton.Circle inline and verify styling via ProfileSection still centers and gaps correctly.src/shared/ui/Skeleton/group/BannerSkeleton.tsx (1)
15-20:marginTop์ธ๋ผ์ธ ์คํ์ผ์ styled-component๋ก ๋ถ๋ฆฌํ๋ ๊ฒ์ ๊ถ์ฅํฉ๋๋ค.
HotTopicSectionSkeleton๊ณผ ๋์ผํ๊ฒ, ๋ด๋ถ ๋ํผdiv์ ์ธ๋ผ์ธ ์คํ์ผ์ด ์ฌ์ฉ๋๊ณ ์์ต๋๋ค.GroupDetailSkeleton.styled.ts์ ๊ด๋ จ ์คํ์ผ์ด ์ด๋ฏธ ์กด์ฌํ๋ฏ๋ก(MetaSkeletonRow,TagSkeletonRow๋ฑ) ์ผ๊ด์ฑ์ ์ํด ์คํ์ผ์ styled-component๋ก ๋ถ๋ฆฌํ๋ ๊ฒ์ด ์ข์ต๋๋ค.๐ค 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, The inline marginTop in BannerSkeleton.tsx should be moved to a styled-component in GroupDetailSkeleton.styled.ts for consistency; create a new styled export (e.g., BannerSkeletonWrapper or reuse an existing pattern like MetaSkeletonRow/TagSkeletonRow), apply the margin-top: 20px (and the inner 8px spacer as another styled div or apply a margin-top on a new InnerWrapper), export and import that styled into BannerSkeleton.tsx, then replace the two inline-styled divs with the new styled components so no inline styles remain.src/pages/feed/MyFeedPage.tsx (1)
38-46:Promise.all๊ฒฐ๊ณผ์ ๋ํ ์๋ ํ์ ์บ์คํธ๊ฐ ์ทจ์ฝํฉ๋๋ค.
Promise.all์ ์ธ ๋ฒ์งธ ์ธ์(minLoadingTime)๊ฐ ํ์ ์ถ๋ก ์ ๋ฐฉํดํ์ฌ ์ ์ฒด ๊ฒฐ๊ณผ๋ฅผ ์๋์ผ๋ก ์บ์คํ ํด์ผ ํ๋ ๊ตฌ์กฐ๊ฐ ๋์์ต๋๋ค. ๋ฐฐ์ด ์์๊ฐ ๋ณ๊ฒฝ๋ ๊ฒฝ์ฐ ์บ์คํธ๊ฐ ๋ฐํ์ ๋ฒ๊ทธ๋ฅผ ์กฐ์ฉํ ํ์ฉํฉ๋๋ค.โป๏ธ ๋ฆฌํฉํฐ๋ง ์ ์
- const [feedResponse, profileResponse] = (await Promise.all([ - getOtherFeed(Number(userId)), - getOtherProfile(Number(userId)), - minLoadingTime, - ])) as [ - Awaited<ReturnType<typeof getOtherFeed>>, - Awaited<ReturnType<typeof getOtherProfile>>, - void, - ]; + const [feedResponse, profileResponse] = await Promise.all([ + getOtherFeed(Number(userId)), + getOtherProfile(Number(userId)), + ] as const); + await minLoadingTime;๐ค 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 38 - 46, The current Promise.all includes minLoadingTime in the same array which forces a manual cast and risks silent runtime bugs if array order changes; change the code to await the two network calls together and await minLoadingTime separately: call Promise.all([getOtherFeed(Number(userId)), getOtherProfile(Number(userId))]) to get [feedResponse, profileResponse] (no manual cast), then await minLoadingTime on its own (e.g., await minLoadingTime) before continuing; update references to feedResponse and profileResponse accordingly.src/pages/mypage/Mypage.tsx (1)
109-118: ์ค๋ณต๋ ํ๋กํ ์ค์ผ๋ ํค ๋งํฌ์ ์ ๊ณต์ฉ ์ปดํฌ๋ํธ ์ฌ์ฌ์ฉ์ ๊ถ์ฅํฉ๋๋ค.ํ์ฌ ๊ตฌ์กฐ๋
src/shared/ui/Skeleton/feed/ProfileSkeleton.tsx์ ์ญํ ์ด ๊ฒน์ณ ์ ์ง๋ณด์ ํฌ์ธํธ๊ฐ ๋์ด๋ฉ๋๋ค. ๊ณต์ฉ ์ค์ผ๋ ํค์ผ๋ก ์นํํ๋ฉด ์คํ์ผ ๋ณ๊ฒฝ ์ ์ผ๊ด์ฑ์ด ์ข์์ง๋๋ค.๐ค 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 109 - 118, The markup duplicated in Mypage.tsx (ProfileSkeletonContainer / ProfileSkeletonLeft / ProfileSkeletonText with Skeleton.Circle/Text/Box) should be replaced by the shared ProfileSkeleton component from src/shared/ui/Skeleton/feed/ProfileSkeleton.tsx; locate the block that renders ProfileSkeletonContainer and swap it for the shared <ProfileSkeleton /> (or pass the existing size/variant props if ProfileSkeleton supports them) so styling and behavior come from the single source of truth and the local skeleton subcomponents (ProfileSkeletonContainer, ProfileSkeletonLeft, ProfileSkeletonText, Skeleton.Circle, Skeleton.Text, Skeleton.Box) are removed from this file.src/pages/groupDetail/ParticipatedGroupDetail.tsx (3)
80-86:minLoadingTime์resolveํ์ ์ด ๋ช ์๋์ง ์์์ต๋๋ค.
new Promise(resolve => setTimeout(resolve, 500))์์resolve์ ํ์ ์ด ์ถ๋ก ์ผ๋กPromise<unknown>์ด ๋ฉ๋๋ค. ๊ธฐ๋ฅ์ ๋ฌธ์ ๋ ์์ง๋ง, ์ฌ๋ฌ ๊ณณ์์ ๋ฐ๋ณต๋๋ ํจํด์ด๋ฏ๋ก ์ ํธ ํจ์๋ก ์ถ์ถํ๋ฉด ์ฝ๋ ์ค๋ณต์ ์ค์ผ ์ ์์ต๋๋ค.โป๏ธ ์ ํธ ํจ์ ์ถ์ถ ์ ์
// e.g. src/utils/delay.ts export const delay = (ms: number): Promise<void> => new Promise(resolve => setTimeout(resolve, ms));๊ทธ ํ ๊ฐ ํ์ผ์์:
-const minLoadingTime = new Promise(resolve => setTimeout(resolve, 500)); +const minLoadingTime = delay(500);๐ค 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 80 - 86, Replace the inline new Promise pattern used as minLoadingTime with a reusable typed utility (e.g., export const delay = (ms: number): Promise<void>) and use it in the ParticipatedGroupDetail component when awaiting getRoomPlaying; specifically, remove new Promise(resolve => setTimeout(resolve, 500)) and call await delay(500) (or include delay(500) in Promise.all alongside getRoomPlaying(parseInt(roomId))) so the resolve type is explicit (Promise<void>) and the pattern is centralized for reuse.
106-108:useEffect์์กด์ฑ ๋ฐฐ์ด์navigate๊ฐ ๋๋ฝ๋์ด ์์ต๋๋ค.
react-router-dom์navigate๋ ์์ ์ ์ธ ์ฐธ์กฐ์ด๋ฏ๋ก ์ค์ ๋ฒ๊ทธ๊ฐ ๋์ง ์์ง๋ง,eslint-plugin-react-hooks์์ ๊ฒฝ๊ณ ๊ฐ ๋ฐ์ํ ์ ์์ต๋๋ค. ์ด ํ์ผ ๋ด์์navigate๋ฅผ ์ฌ์ฉํ๋useEffect์ ์์กด์ฑ์ ์ถ๊ฐํ๋ ๊ฒ์ ๊ณ ๋ คํด ์ฃผ์ธ์.๐ค 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 106 - 108, The useEffect that calls fetchRoomDetail currently lists [roomId] but omits the navigate function, which can trigger eslint-plugin-react-hooks warnings; update the dependency array for the useEffect that calls fetchRoomDetail to include navigate (i.e., [roomId, navigate]) so the hook lists all referenced symbols (fetchRoomDetail, navigate, roomId) and keeps hooks linting satisfied.
67-69: ๋ ๊ฐ์ ๋ค๋ก๊ฐ๊ธฐ ํธ๋ค๋ฌ๊ฐ ์กด์ฌํฉ๋๋ค โ ์๋ํ ๊ฒ์ธ์ง ํ์ธ ํ์.
handleBackClick์navigate(-1)(์๋ฌ ์ํ์์ ์ฌ์ฉ),handleBackButton์navigate('/group')(๋ก๋ฉ/๋ฉ์ธ ์ฝํ ์ธ ์์ ์ฌ์ฉ)์ผ๋ก ๋์์ด ๋ค๋ฆ ๋๋ค. ์๋ฌ ์ ๋ธ๋ผ์ฐ์ ํ์คํ ๋ฆฌ ๋ค๋ก๊ฐ๊ธฐ, ์ ์ ์ ๊ทธ๋ฃน ๋ชฉ๋ก ์ด๋์ด ์๋๋ ์ค๊ณ๋ผ๋ฉด ๊ด์ฐฎ์ง๋ง, ์ผ๊ด์ฑ ์ธก๋ฉด์์ ํ์ธํด ์ฃผ์ธ์.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 different back handlersโhandleBackClick (calls navigate(-1)) and handleBackButton (calls navigate('/group'))โwhich may be unintended; decide on the intended UX and make them consistent: either consolidate into a single handler (e.g., unify both to navigate('/group') or to navigate(-1)) and replace usages, or keep both but rename them to clarify intent (e.g., handleBackToHistory for navigate(-1) and handleBackToGroupList for navigate('/group')) and ensure error and normal render paths call the correct one; update any onClick references where handleBackClick or handleBackButton are used so the code reflects the chosen behavior and add a short comment near the handler(s) explaining when each should be used.src/shared/ui/Skeleton/group/GroupDetailSkeleton.styled.ts (1)
4-14:SkeletonWrapper๊ฐParticipatedWrapper/Wrapper์ ๊ฑฐ์ ๋์ผํ ์คํ์ผ์ ๊ฐ์ง๊ณ ์์ต๋๋ค.
ParticipatedGroupDetail.styled.ts์ParticipatedWrapper์GroupDetail.styled.ts์Wrapper๋ชจ๋ ๋์ผํ ๋ ์ด์์ ํจํด(flex column, min/max-width, margin auto, background)์ ์ฌ์ฉํฉ๋๋ค. ๊ณตํต ๋ํผ ์ปดํฌ๋ํธ๋ก ์ถ์ถํ๋ฉด ์ค๋ณต์ ์ค์ผ ์ ์์ต๋๋ค.๐ค 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 4 - 14, Duplicate layout styles exist across SkeletonWrapper, ParticipatedWrapper, and Wrapper; extract a shared styled-component (e.g., CommonWrapper or BaseLayout) that contains the shared rules (display:flex; position:relative; flex-direction:column; align-items:center; min-width:320px; max-width:767px; min-height:100vh; margin:0 auto; background-color:${colors.black.main}); replace SkeletonWrapper, ParticipatedWrapper, and Wrapper to extend or wrap this new CommonWrapper (via styled(CommonWrapper) or by composition) and keep any unique styles in each specific component so duplication is removed but unique overrides remain.src/pages/searchBook/SearchBook.tsx (1)
82-87:minLoadingTimeํจํด์ด ๋ฐ๋ณต์ ์ผ๋ก ์ฌ์ฉ๋๊ณ ์์ต๋๋ค.์ด ํ์ผ ๋ด์์ ๋์ผํ
new Promise(resolve => setTimeout(resolve, 500))ํจํด์ด ๋ ๋ฒ, ํ๋ก์ ํธ ์ ์ฒด์์๋ ๋ค์ ์ฌ์ฉ๋ฉ๋๋ค.ParticipatedGroupDetail.tsx๋ฆฌ๋ทฐ์์ ์ธ๊ธํ ๋๋ก ๊ณตํต ์ ํธ๋ก ์ถ์ถ์ ๊ถ์ฅํฉ๋๋ค.Also applies to: 120-124
๐ค 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 82 - 87, Extract the repeated new Promise(resolve => setTimeout(resolve, 500)) into a shared utility (e.g., sleep or delay) and replace local minLoadingTime usages with that util; specifically create a util function (sleep/msDelay) and import it into SearchBook.tsx, then change the Promise.all call that currently awaits getBookDetail(isbn), getRecruitingRooms(isbn), minLoadingTime to use sleep(500) (and update the other occurrence around lines 120-124 similarly), and also replace the same pattern in ParticipatedGroupDetail.tsx and other files to use the new utility to avoid duplication.
๐ค 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/components/feed/MyFeed.tsx`:
- Around line 33-35: The current conditional in the MyFeed component (the if in
MyFeed that checks `loading || !profileData`) keeps the skeleton shown forever
when the profile API fails; change the logic so the skeleton is shown only while
`loading` is true (e.g., `if (loading) return <OtherFeedSkeleton .../>`), and
handle the `!profileData` case separately (render an error/fallback UI or an
empty-state with retry) once loading is false. Update the fetch handler in
MyFeed to set an explicit error or status flag (e.g., `profileError` or
`profileStatus`) so you can distinguish loading vs failed vs empty states and
render accordingly.
In `@src/pages/feed/FeedDetailPage.tsx`:
- Around line 69-74: The current Promise.all([...getFeedDetail, getComments,
minLoadingTime]) causes the 500ms minLoadingTime to be skipped if any API
rejects; extract minLoadingTime (const minLoadingTime = new Promise(resolve =>
setTimeout(resolve, 500))) outside Promise.all and ensure you always await it
even on error โ e.g., await Promise.all([getFeedDetail(...), getComments(...)])
inside a try/catch (or use Promise.allSettled for both API calls) and in a
finally block await minLoadingTime before updating state or hiding the skeleton;
update references to feedResponse/commentsResponse accordingly to handle
settled/rejected results.
In `@src/pages/feed/FollowerListPage.tsx`:
- Line 132: The TitleHeader usage is missing an alt on the leftArrow image:
update the leftIcon prop where TitleHeader is rendered (the JSX line using
leftArrow, handleBackClick, and title) to include an alt attribute on the <img>
(e.g., alt="๋ค๋ก๊ฐ๊ธฐ" to match MyFeedPage's back icon) so the image is accessible.
In `@src/pages/groupDetail/GroupDetail.tsx`:
- Around line 132-141: The error block in GroupDetail.tsx returns plain div text
without a color, which can be invisible against the dark background; update the
error rendering to set an explicit text color (e.g., white or var(--color-red))
or reuse the existing ErrorContainer styled-component used in
ParticipatedGroupDetail so the message inside Wrapper/Header/IconButton (with
leftArrow and handleBackButton) is visible; replace the inline div with the
styled ErrorContainer (or add a color style) to ensure consistent error styling
similar to SearchBook.tsx and ParticipatedGroupDetail.
- Around line 314-316: The displayed date range in GroupDetail (MetaDate showing
roomData.progressStartDate ~ roomData.progressEndDate) uses raw API strings and
is inconsistent with ParticipatedGroupDetail which uses formatDate to convert
'-' to '.'; update GroupDetail to pass roomData.progressStartDate and
roomData.progressEndDate through the same formatDate utility (or the shared date
formatting helper used by ParticipatedGroupDetail) before rendering inside
MetaDate so both pages show the same formatted dates.
In `@src/pages/groupMembers/GroupMembers.tsx`:
- Around line 34-38: The current Promise.all between
getRoomMembers(parseInt(currentRoomId)) and minLoadingTime can short-circuit on
API failure and skip the 500ms delay; change the pattern so you await
getRoomMembers (or its try/catch) and always await minLoadingTime in a
.finally() (or use try { await getRoomMembers(...) } catch (err) {
store/propagate err } finally { await minLoadingTime }) to guarantee the minimum
loading time; ensure you reference getRoomMembers and minLoadingTime and rethrow
or handle the error after the finally so existing error handling/upstream
behavior (response variable usage) remains correct.
In `@src/pages/groupSearch/GroupSearch.tsx`:
- Around line 359-366: The delete callback (handleDelete) may throw because
deleteRecentSearch can throw on failure; wrap the async call in a try/catch
inside handleDelete, locate the handler using handleDelete and
deleteRecentSearch (and recentSearches/fetchRecentSearches) to add error
handling, await deleteRecentSearch inside try, keep the existing res.isSuccess
flow to call fetchRecentSearches on success, and in catch log the error and/or
show a user-facing notification so the rejection is not unhandled.
In `@src/pages/memory/Memory.tsx`:
- Around line 318-342: When rendering the loading skeleton, pointerEvents:
'none' still allows keyboard focus and Enter/Space to trigger handleTabChange
and change activeTab (causing duplicate loads); add the inert attribute to the
FixedSection wrapper when loading is true so all focus and keyboard interactions
are blocked (i.e., render <FixedSection inert ...> alongside the existing
style), and if FixedSection is a custom component ensure it forwards the inert
prop to the underlying DOM element (or explicitly set inert on that DOM node) so
RecordTabs, handleTabChange and RecordFilters cannot receive focus or fire
events during loading.
In `@src/pages/mypage/Mypage.tsx`:
- Around line 108-134: Currently the conditional `loading || !profile` causes
the skeleton to render even when loading is finished but profile fetch failed;
update the render logic in Mypage (around the JSX using ProfileSkeletonContainer
and UserProfile) to explicitly branch: if (loading) show the skeleton, else if
(profile) show the UserProfile block (using profile.profileImageUrl,
profile.nickname, profile.aliasColor, profile.aliasName and handleEditClick),
else show a failure/fallback UI (e.g., an error message or empty state) so the
skeleton is not shown indefinitely when profile is null and loading is false.
In `@src/pages/mypage/SavePage.tsx`:
- Around line 128-133: The current Promise.all call can reject early and bypass
minLoadingTime; change the flow so API calls and the minimum loading timer are
awaited independently: use Promise.allSettled([getSavedFeedsInMy(null),
getSavedBooksInMy()]) to collect both results without throwing, then await the
existing minLoadingTime Promise (minLoadingTime) before proceeding to process
the settled results (inspect the .status and .value/.reason for
feedsResponse/booksResponse) so the skeleton is guaranteed to show at least
500ms even if one request fails.
In `@src/pages/searchBook/SearchBook.tsx`:
- Around line 257-265: TopBackground is rendered with
bookImgUrl={bookDetail?.imageUrl || ''} even when bookDetail is null, causing an
empty url() CSS and unwanted requests; change the render logic so TopBackground
is only mounted when bookDetail is available (e.g., move <TopBackground ... />
inside the branch where !isLoading && bookDetail is truthy) or alternatively
update TopBackground to accept an optional prop and skip applying the background
when bookImgUrl is falsy; refer to TopBackground, bookDetail, bookImgUrl,
BookDetailSkeleton and isLoading to locate the relevant JSX and implement the
conditional rendering or guard.
- Around line 244-255: The error guard renders when error is truthy, so the '||
"์ฑ
์ ๋ณด๋ฅผ ์ฐพ์ ์ ์์ต๋๋ค."' fallback is dead code; change the JSX inside the if (error)
block to render the actual error only (e.g., replace {error || '์ฑ
์ ๋ณด๋ฅผ ์ฐพ์ ์
์์ต๋๋ค.'} with {error} or String(error)) so the Wrapper/Header/IconButton
(leftArrow, handleBackButton) block shows the real error; alternatively, if you
intended to show the fallback when no error exists, adjust the surrounding
condition (remove if (error) and render conditional content) instead.
In `@src/shared/ui/Skeleton/base/Skeleton.tsx`:
- Around line 62-85: The component currently renders an empty
SkeletonTextWrapper when lines is 0 or negative; update the guard in
Skeleton.Text so that values <= 1 render a single SkeletonTextLine instead of
the wrapper. Specifically, change the conditional that checks lines (currently
if (lines === 1)) to if (lines <= 1) and keep using
width/height/className/lastLineWidth appropriately when returning the single
SkeletonTextLine; this avoids rendering an empty SkeletonTextWrapper for lines
<= 0.
In `@src/shared/ui/Skeleton/group/GroupBookSectionSkeleton.tsx`:
- Around line 7-17: GroupBookSectionSkeleton currently sets only
cursor:'default' which still allows event propagation; update the component so
the container (StyledGroupBookSection) also applies pointerEvents:'none' when
rendering the skeleton (same behavior as BookItemSkeleton) to fully block clicks
and pointer events; locate GroupBookSectionSkeleton and add pointerEvents:'none'
to the inline style or the styled component variant used for skeleton rendering
so parent click handlers wonโt fire.
In `@src/shared/ui/Skeleton/README.md`:
- Around line 25-50: Add blank lines before and after each Markdown table to
satisfy markdownlint MD058: insert an empty line immediately above and below the
table under the "### Skeleton (๊ธฐ๋ณธ)" section (the
width/height/borderRadius/className table), do the same for the "###
Skeleton.Circle" table, and for the "### Skeleton.Text" table; ensure each table
block is separated from surrounding text by a single blank line so the tables
for Skeleton (๊ธฐ๋ณธ), Skeleton.Circle, and Skeleton.Text are each preceded and
followed by an empty line.
In `@src/shared/ui/Skeleton/searchBook/BookDetailSkeleton.tsx`:
- Around line 31-40: The skeleton buttons (RecruitingGroupButton,
WritePostButton, SaveButton) still receive keyboard focus because only
pointerEvents is disabled; update these components in BookDetailSkeleton so they
are non-interactive to keyboard users by adding disabled (or tabIndex={-1}) to
each button instance and remove any conflicting interactive propsโe.g., set
<RecruitingGroupButton disabled style=...>, <WritePostButton disabled ...>, and
ensure <SaveButton already disabled> includes tabIndex={-1} or likewise; keep
the visual skeleton styles but make their ARIA/DOM state explicitly
non-interactive.
---
Outside diff comments:
In `@src/pages/feed/FollowerListPage.tsx`:
- Around line 61-70: Move the null-check for response/response.data before any
access to response.data to avoid runtime errors: check if response and
response.data are truthy (e.g., if (!response || !response.data) { setError('API
์๋ต์ด ์์ต๋๋ค.'); return; }) before computing userData, then populate userData using
the existing type switch that references (response.data as { followers:
FollowData[] }) or followings; alternatively replace direct access with safe
optional chaining when assigning userData (e.g., response?.data?.followers) but
ensure setError/return still runs when data is missing; update the logic around
the variables response, userData, type and setError in FollowerListPage.tsx
accordingly.
In `@src/pages/feed/MyFeedPage.tsx`:
- Around line 75-77: The current MyFeedPage returns an empty fragment on error
(if (error) return <></>), leaving users stranded; replace that with the same
error UI + back navigation used in GroupDetail/ParticipatedGroupDetail: render a
user-friendly error message and a visible "Back" control that triggers the page
navigation (e.g., the same BackButton or useNavigate/navigation callback used
elsewhere) and preserve any error details for logging or debugging; update the
error branch inside the MyFeedPage component to mirror the error-handling
pattern from GroupDetail/ParticipatedGroupDetail instead of returning an empty
fragment.
In `@src/pages/groupDetail/GroupDetail.tsx`:
- Around line 55-57: The isLoading state in GroupDetail (useState for isLoading)
is initialized to false which disagrees with ParticipatedGroupDetail; change the
initial value to true so the first render reflects loading semantics (update the
useState<boolean> call that declares isLoading and setIsLoading in
GroupDetail.tsx to use true) to keep behavior consistent with
ParticipatedGroupDetail and avoid a non-loading initial render before
setIsLoading(true) runs.
In `@src/pages/groupDetail/ParticipatedGroupDetail.tsx`:
- Around line 56-63: The ParticipatedGroupDetail component initializes loading
as true which immediately shows the skeleton on first render; make the initial
loading value consistent with GroupDetail.tsx by setting the loading state
(const [loading, setLoading]) to false (or align both components' intent) and
rely on the !roomData guard for initial empty-render behavior, updating
setLoading(true/false) only around your async fetch logic (functions referenced:
ParticipatedGroupDetail, setLoading, roomData) so both pages behave
consistently.
In `@src/pages/groupSearch/GroupSearch.tsx`:
- Around line 50-59: The useEffect hooks call fetchRecentSearches() twice on
mount because searchStatus starts as 'idle'โfix by consolidating the logic: keep
a single useEffect (the one with empty deps or the one watching searchStatus)
and add a guard so fetchRecentSearches() is invoked only when searchStatus ===
'idle' (or remove the effect that watches searchStatus entirely and rely on the
mount effect with the status check); update references to useEffect,
fetchRecentSearches, and searchStatus accordingly to avoid duplicate initial
requests and loading flicker.
---
Duplicate comments:
In `@src/components/search/MostSearchedBooks.tsx`:
- Around line 30-34: The Promise.all usage with getMostSearchedBooks and
minLoadingTime can reject immediately when the API fails, breaking the
guaranteed skeleton display; change the logic to await both promises regardless
of API failure (e.g., use Promise.allSettled([getMostSearchedBooks(),
minLoadingTime]) and then extract the fulfilled value from the first result or
fall back to safe empty data), or wrap getMostSearchedBooks() with a catch that
returns a sentinel/fallback so Promise.all always resolves; update references in
this component (minLoadingTime, getMostSearchedBooks) accordingly.
In `@src/pages/feed/OtherFeedPage.tsx`:
- Around line 37-46: The current Promise.all with getOtherFeed, getOtherProfile
and minLoadingTime can fail-fast and skip the 500ms guarantee; wrap the API
calls so the timing promise always resolves independentlyโe.g., use
Promise.allSettled or attach .catch handlers to getOtherFeed/getOtherProfile
before awaitingโthen after awaiting, check results and throw or handle errors
and extract values into feedResponse/profileResponse; update the block
referencing getOtherFeed, getOtherProfile, minLoadingTime, feedResponse and
profileResponse accordingly.
In `@src/pages/today-words/TodayWords.tsx`:
- Around line 94-101: The current Promise.all usage with minLoadingTime plus
getDailyGreeting means if getDailyGreeting rejects the 500ms delay may be
skipped; change to ensure minLoadingTime is always awaited by either awaiting
minLoadingTime in a finally block or by converting the API call into a
non-rejecting promise and then using Promise.all/Promise.allSettled. Concretely,
wrap getDailyGreeting(roomId: parseInt(roomId), cursor: cursor || undefined)
into a promise that catches errors (so it never rejects) or use
Promise.allSettled for [getDailyGreeting(...), minLoadingTime] and then handle
the settled result for the API call (check for rejection and process
accordingly); update references in TodayWords to use the new safe API result
handling and keep isRefresh logic the same.
---
Nitpick comments:
In `@src/pages/feed/MyFeedPage.tsx`:
- Around line 38-46: The current Promise.all includes minLoadingTime in the same
array which forces a manual cast and risks silent runtime bugs if array order
changes; change the code to await the two network calls together and await
minLoadingTime separately: call Promise.all([getOtherFeed(Number(userId)),
getOtherProfile(Number(userId))]) to get [feedResponse, profileResponse] (no
manual cast), then await minLoadingTime on its own (e.g., await minLoadingTime)
before continuing; update references to feedResponse and profileResponse
accordingly.
In `@src/pages/groupDetail/ParticipatedGroupDetail.tsx`:
- Around line 80-86: Replace the inline new Promise pattern used as
minLoadingTime with a reusable typed utility (e.g., export const delay = (ms:
number): Promise<void>) and use it in the ParticipatedGroupDetail component when
awaiting getRoomPlaying; specifically, remove new Promise(resolve =>
setTimeout(resolve, 500)) and call await delay(500) (or include delay(500) in
Promise.all alongside getRoomPlaying(parseInt(roomId))) so the resolve type is
explicit (Promise<void>) and the pattern is centralized for reuse.
- Around line 106-108: The useEffect that calls fetchRoomDetail currently lists
[roomId] but omits the navigate function, which can trigger
eslint-plugin-react-hooks warnings; update the dependency array for the
useEffect that calls fetchRoomDetail to include navigate (i.e., [roomId,
navigate]) so the hook lists all referenced symbols (fetchRoomDetail, navigate,
roomId) and keeps hooks linting satisfied.
- Around line 67-69: There are two different back handlersโhandleBackClick
(calls navigate(-1)) and handleBackButton (calls navigate('/group'))โwhich may
be unintended; decide on the intended UX and make them consistent: either
consolidate into a single handler (e.g., unify both to navigate('/group') or to
navigate(-1)) and replace usages, or keep both but rename them to clarify intent
(e.g., handleBackToHistory for navigate(-1) and handleBackToGroupList for
navigate('/group')) and ensure error and normal render paths call the correct
one; update any onClick references where handleBackClick or handleBackButton are
used so the code reflects the chosen behavior and add a short comment near the
handler(s) explaining when each should be used.
In `@src/pages/mypage/Mypage.tsx`:
- Around line 109-118: The markup duplicated in Mypage.tsx
(ProfileSkeletonContainer / ProfileSkeletonLeft / ProfileSkeletonText with
Skeleton.Circle/Text/Box) should be replaced by the shared ProfileSkeleton
component from src/shared/ui/Skeleton/feed/ProfileSkeleton.tsx; locate the block
that renders ProfileSkeletonContainer and swap it for the shared
<ProfileSkeleton /> (or pass the existing size/variant props if ProfileSkeleton
supports them) so styling and behavior come from the single source of truth and
the local skeleton subcomponents (ProfileSkeletonContainer, ProfileSkeletonLeft,
ProfileSkeletonText, Skeleton.Circle, Skeleton.Text, Skeleton.Box) are removed
from this file.
In `@src/pages/searchBook/SearchBook.tsx`:
- Around line 82-87: Extract the repeated new Promise(resolve =>
setTimeout(resolve, 500)) into a shared utility (e.g., sleep or delay) and
replace local minLoadingTime usages with that util; specifically create a util
function (sleep/msDelay) and import it into SearchBook.tsx, then change the
Promise.all call that currently awaits getBookDetail(isbn),
getRecruitingRooms(isbn), minLoadingTime to use sleep(500) (and update the other
occurrence around lines 120-124 similarly), and also replace the same pattern in
ParticipatedGroupDetail.tsx and other files to use the new utility to avoid
duplication.
In `@src/shared/ui/Skeleton/base/Skeleton.styled.ts`:
- Around line 54-56: SkeletonTextLine duplicates the height interpolation
already handled by SkeletonBase; remove the redundant height handling from
SkeletonTextLine and instead rely on SkeletonBase's logic while only overriding
the default value (from 20px to 16px) via a single source of truthโe.g., pass a
default prop or theme/variable used by SkeletonBaseโso update SkeletonTextLine
to stop re-declaring height and supply the desired default height through
SkeletonBase's prop/default mechanism (reference SkeletonTextLine and
SkeletonBase and the height prop).
- Around line 22-36: The SkeletonBase styled component currently uses display:
inline-block which can cause multiple skeletons to flow horizontally in
non-flex/grid parents; update SkeletonBase to use display: block so it behaves
as a predictable full-width block element in plain containers, keeping the
existing width/height/borderRadius logic and animations intact (locate
SkeletonBase in src/shared/ui/Skeleton/base/Skeleton.styled.ts and replace the
display rule from inline-block to block).
In `@src/shared/ui/Skeleton/base/Skeleton.tsx`:
- Around line 40-44: The base and box APIs are redundant: SkeletonBox is just a
styled(SkeletonBase) empty extension and Skeleton.Box duplicates functionality;
make the rectangular API explicit by keeping Skeleton.Box (Skeleton.Box
component) as the public export and restrict the base Skeleton (Skeleton and
SkeletonBase) to internal use onlyโremove or stop exporting the top-level
Skeleton component, keep SkeletonBox/Skeleton.Box as the single public rectangle
API, update any export lists and consume-sites to import Skeleton.Box instead of
Skeleton, and add a deprecation note/alias if needed to preserve backwards
compatibility.
In `@src/shared/ui/Skeleton/group/BannerSkeleton.tsx`:
- Around line 15-20: The inline marginTop in BannerSkeleton.tsx should be moved
to a styled-component in GroupDetailSkeleton.styled.ts for consistency; create a
new styled export (e.g., BannerSkeletonWrapper or reuse an existing pattern like
MetaSkeletonRow/TagSkeletonRow), apply the margin-top: 20px (and the inner 8px
spacer as another styled div or apply a margin-top on a new InnerWrapper),
export and import that styled into BannerSkeleton.tsx, then replace the two
inline-styled divs with the new styled components so no inline styles remain.
In `@src/shared/ui/Skeleton/group/GroupDetailSkeleton.styled.ts`:
- Around line 4-14: Duplicate layout styles exist across SkeletonWrapper,
ParticipatedWrapper, and Wrapper; extract a shared styled-component (e.g.,
CommonWrapper or BaseLayout) that contains the shared rules (display:flex;
position:relative; flex-direction:column; align-items:center; min-width:320px;
max-width:767px; min-height:100vh; margin:0 auto;
background-color:${colors.black.main}); replace SkeletonWrapper,
ParticipatedWrapper, and Wrapper to extend or wrap this new CommonWrapper (via
styled(CommonWrapper) or by composition) and keep any unique styles in each
specific component so duplication is removed but unique overrides remain.
In `@src/shared/ui/Skeleton/group/HotTopicSectionSkeleton.tsx`:
- Around line 15-22: HotTopicSectionSkeleton currently uses inline style objects
on the outer wrapper and two inner divs; extract those into styled-components
(e.g., create StyledHotTopicWrapper, StyledTitleWrapper, StyledListWrapper in
HotTopicSectionSkeleton.styled.ts or reuse GroupDetailSkeleton.styled) and
replace the inline divs in the HotTopicSectionSkeleton component with these
styled components, preserving the same CSS (padding: 16px 0; margin-bottom:
16px; and display:flex; flex-direction:column; gap:8px;) and keep existing
Skeleton children and props unchanged.
In `@src/shared/ui/Skeleton/members/MemberListSkeleton.tsx`:
- Around line 16-18: Remove the unnecessary wrapper <div> around Skeleton.Circle
in the MemberListSkeleton component: because ProfileSection already applies
display:flex, align-items:center and gap:8px, place Skeleton.Circle directly as
a child (remove the extra div node) so the DOM is simpler and layout remains
unchanged; update the JSX in MemberListSkeleton (where Skeleton.Circle is
rendered) to render Skeleton.Circle inline and verify styling via ProfileSection
still centers and gaps correctly.
In `@src/shared/ui/Skeleton/memory/RecordItemSkeleton.tsx`:
- Around line 12-36: The three repeated inline divs inside the
RecordItemSkeleton (the ones wrapping Skeleton.Box + Skeleton.Text in
ActionSection and the circle wrapper in UserSection) should be replaced with a
lightweight styled component to remove duplicated inline styles; create a styled
component (e.g., IconRow or InlineRow) and use it in place of the divs in
Container/UserSection/ActionSection so styles like display:flex, gap, alignItems
and marginRight are centralized and applied consistently across the component.
In `@src/shared/ui/Skeleton/search/RecentSearchTabsSkeleton.tsx`:
- Around line 4-15: RecentSearchTabsSkeleton currently hardcodes the Title text
("์ต๊ทผ ๊ฒ์์ด"), causing drift if RecentSearchTabs' title changes; update
RecentSearchTabsSkeleton to avoid literal text by either rendering the same
shared constant used by RecentSearchTabs (import the title constant) or
rendering a Skeleton placeholder for the title (replace Title's literal children
with a Skeleton.Box/Skeleton.Text), targeting the RecentSearchTabsSkeleton
component and the Title element so the skeleton stays in sync with the real
component.
In `@src/shared/ui/Skeleton/searchBook/BookItemSkeleton.tsx`:
- Around line 18-23: ํ์ฌ BookItemSkeleton.tsx์์ BookInfo ๋ด๋ถ์ bare <div style={{
marginTop: '8px' }}>๋ก ์ธ๋ผ์ธ ์คํ์ผ์ ์ฌ์ฉํ๊ณ ์์ด styled-component ํจํด๊ณผ ๋ถ์ผ์นํฉ๋๋ค; BookInfo ์์
์ธ ๋ฒ์งธ Skeleton.Text ์๋จ ์ฌ๋ฐฑ์ ์ํด inline div๋ฅผ ์ ๊ฑฐํ๊ณ ๋์ Skeleton.Text์ margin/top ์คํ์ผ์
์ ๋ฌํ๊ฑฐ๋(๋ง์ฝ Skeleton.Text๊ฐ style prop์ ์ง์ํ์ง ์์ผ๋ฉด SkeletonProps์ style?:
React.CSSProperties๋ฅผ ์ถ๊ฐ) ๋๋ BookInfo ๋ด๋ถ์ ๋ํผ styled-component(์: InfoGap ๋๋
StyledSkeletonWrap)๋ฅผ ๋ง๋ค์ด margin-top์ ์ ์ฉํ๋๋ก ๋ณ๊ฒฝํ์ธ์; ์ฐธ์กฐํ ์๋ณ์๋ BookItemSkeleton.tsx,
BookInfo, Skeleton.Text์
๋๋ค.
โน๏ธ 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 (61)
src/components/feed/MyFeed.tsxsrc/components/search/MostSearchedBooks.tsxsrc/pages/feed/Feed.styled.tssrc/pages/feed/Feed.tsxsrc/pages/feed/FeedDetailPage.styled.tssrc/pages/feed/FeedDetailPage.tsxsrc/pages/feed/FollowerListPage.tsxsrc/pages/feed/MyFeedPage.tsxsrc/pages/feed/OtherFeedPage.tsxsrc/pages/groupDetail/GroupDetail.tsxsrc/pages/groupDetail/ParticipatedGroupDetail.tsxsrc/pages/groupMembers/GroupMembers.tsxsrc/pages/groupSearch/GroupSearch.tsxsrc/pages/memory/Memory.tsxsrc/pages/mypage/Mypage.styled.tssrc/pages/mypage/Mypage.tsxsrc/pages/mypage/SavePage.styled.tssrc/pages/mypage/SavePage.tsxsrc/pages/search/Search.tsxsrc/pages/searchBook/SearchBook.tsxsrc/pages/today-words/TodayWords.tsxsrc/shared/ui/Skeleton/README.mdsrc/shared/ui/Skeleton/base/Skeleton.styled.tssrc/shared/ui/Skeleton/base/Skeleton.tsxsrc/shared/ui/Skeleton/base/index.tssrc/shared/ui/Skeleton/feed/FeedPostSkeleton.styled.tssrc/shared/ui/Skeleton/feed/FeedPostSkeleton.tsxsrc/shared/ui/Skeleton/feed/OtherFeedSkeleton.styled.tssrc/shared/ui/Skeleton/feed/OtherFeedSkeleton.tsxsrc/shared/ui/Skeleton/feed/ProfileSkeleton.styled.tssrc/shared/ui/Skeleton/feed/ProfileSkeleton.tsxsrc/shared/ui/Skeleton/feed/TotalBarSkeleton.styled.tssrc/shared/ui/Skeleton/feed/TotalBarSkeleton.tsxsrc/shared/ui/Skeleton/feed/UserProfileItemSkeleton.tsxsrc/shared/ui/Skeleton/feed/index.tssrc/shared/ui/Skeleton/group/BannerSkeleton.tsxsrc/shared/ui/Skeleton/group/BookSkeleton.tsxsrc/shared/ui/Skeleton/group/CommentSectionSkeleton.tsxsrc/shared/ui/Skeleton/group/GroupBookSectionSkeleton.tsxsrc/shared/ui/Skeleton/group/GroupCardSkeleton.tsxsrc/shared/ui/Skeleton/group/GroupDetailSkeleton.styled.tssrc/shared/ui/Skeleton/group/GroupDetailSkeleton.tsxsrc/shared/ui/Skeleton/group/HotTopicSectionSkeleton.tsxsrc/shared/ui/Skeleton/group/RecordSectionSkeleton.tsxsrc/shared/ui/Skeleton/group/index.tssrc/shared/ui/Skeleton/index.tssrc/shared/ui/Skeleton/members/MemberListSkeleton.tsxsrc/shared/ui/Skeleton/members/index.tssrc/shared/ui/Skeleton/memory/RecordItemSkeleton.tsxsrc/shared/ui/Skeleton/memory/index.tssrc/shared/ui/Skeleton/search/MostSearchedBooksSkeleton.tsxsrc/shared/ui/Skeleton/search/RecentSearchTabsSkeleton.tsxsrc/shared/ui/Skeleton/search/index.tssrc/shared/ui/Skeleton/searchBook/BookDetailSkeleton.tsxsrc/shared/ui/Skeleton/searchBook/BookItemSkeleton.tsxsrc/shared/ui/Skeleton/searchBook/index.tssrc/shared/ui/Skeleton/todaywords/DateDividerSkeleton.tsxsrc/shared/ui/Skeleton/todaywords/MessageItemSkeleton.tsxsrc/shared/ui/Skeleton/todaywords/MessageListSkeleton.styled.tssrc/shared/ui/Skeleton/todaywords/MessageListSkeleton.tsxsrc/shared/ui/Skeleton/todaywords/index.ts
| const minLoadingTime = new Promise(resolve => setTimeout(resolve, 500)); | ||
| const [feedResponse, commentsResponse] = await Promise.all([ | ||
| getFeedDetail(Number(feedId)), | ||
| getComments(Number(feedId), { postType: 'FEED' }), | ||
| minLoadingTime, | ||
| ]); |
There was a problem hiding this comment.
๐งฉ Analysis chain
๐ Script executed:
# FeedDetailPage.tsx ํ์ผ ํ์ธ
fd "FeedDetailPage.tsx" --type fRepository: THIP-TextHip/THIP-Web
Length of output: 98
๐ Script executed:
# ํ์ผ ํฌ๊ธฐ ํ์ธ
wc -l src/pages/feed/FeedDetailPage.tsxRepository: THIP-TextHip/THIP-Web
Length of output: 102
๐ Script executed:
# FeedDetailPage.tsx ํ์ผ ๋ด์ฉ ํ์ธ
cat -n src/pages/feed/FeedDetailPage.tsxRepository: THIP-TextHip/THIP-Web
Length of output: 8364
์คํจ ๊ฒฝ๋ก์์ ์ต์ ๋ก๋ฉ ์๊ฐ์ด ๋ณด์ฅ๋์ง ์์ต๋๋ค.
69-74์ค์ Promise.all ๋ด๋ถ์ minLoadingTime์ ํฌํจํ๋ฉด, API ์ค ํ๋๊ฐ ์คํจํ ๋ Promise.all์ด ์ฆ์ ๊ฑฐ๋ถ๋์ด 500ms๋ฅผ ๊ธฐ๋ค๋ฆฌ์ง ์์ต๋๋ค. ์๋ฌ ์ผ์ด์ค์์๋ ์ต์ 500ms ๋์ skeleton์ ํ์ํ๋ ค๋ฉด minLoadingTime์ Promise.all ์ธ๋ถ๋ก ๋ถ๋ฆฌํด์ผ ํฉ๋๋ค.
์์ ์์
const minLoadingTime = new Promise(resolve => setTimeout(resolve, 500));
- const [feedResponse, commentsResponse] = await Promise.all([
+ const [feedResponse, commentsResponse] = await Promise.all([
getFeedDetail(Number(feedId)),
getComments(Number(feedId), { postType: 'FEED' }),
- minLoadingTime,
- ]);
+ ]);
+ await minLoadingTime;๐ค Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/pages/feed/FeedDetailPage.tsx` around lines 69 - 74, The current
Promise.all([...getFeedDetail, getComments, minLoadingTime]) causes the 500ms
minLoadingTime to be skipped if any API rejects; extract minLoadingTime (const
minLoadingTime = new Promise(resolve => setTimeout(resolve, 500))) outside
Promise.all and ensure you always await it even on error โ e.g., await
Promise.all([getFeedDetail(...), getComments(...)]) inside a try/catch (or use
Promise.allSettled for both API calls) and in a finally block await
minLoadingTime before updating state or hiding the skeleton; update references
to feedResponse/commentsResponse accordingly to handle settled/rejected results.
There was a problem hiding this comment.
๐งฉ Analysis chain
๐ Script executed:
#!/bin/bash
# Fetch the commit details to see what changes were made
gh api repos/THIP-TextHip/THIP-Web/commits/20856eb6d329bed7f2639945492053acf678f755 --jq '.commit.message, .files[] | select(.filename == "src/pages/feed/FeedDetailPage.tsx") | .patch'
Length of output: 57
๐ Script executed:
#!/bin/bash
# Fetch the commit message
echo "=== Commit Message ==="
gh api repos/THIP-TextHip/THIP-Web/commits/20856eb6d329bed7f2639945492053acf678f755 --jq '.commit.message'
echo -e "\n=== Changes to FeedDetailPage.tsx ==="
# Fetch the patch for FeedDetailPage.tsx
gh api repos/THIP-TextHip/THIP-Web/commits/20856eb6d329bed7f2639945492053acf678f755 --jq '.files[] | select(.filename == "src/pages/feed/FeedDetailPage.tsx") | .patch'
Length of output: 499
@ljh130334 ์ปค๋ฐ ํ์ธํ์ต๋๋ค! ์ฑ๊ณต ๊ฒฝ๋ก์ ๋ํ ๊ฐ์ ์ ์ ๋์์ง๋ง, ์๋ฌ ๊ฒฝ๋ก์์ ์ฌ์ ํ ์ต์ ๋ก๋ฉ ์๊ฐ์ด ๋ณด์ฅ๋์ง ์์ต๋๋ค.
ํ์ฌ ์ฝ๋ ํ๋ฆ์ ๋ณด๋ฉด:
- โ
์ฑ๊ณต ์: Promise.all ์๋ฃ โ
await minLoadingTime๋๊ธฐ โ ๋ฐ์ดํฐ ์ค์ - โ ์คํจ ์: Promise.all ์ฆ์ reject โ catch ๋ธ๋ก์ผ๋ก ์ ํ (69์ค์
await minLoadingTime์ ๊ฑด๋๋)
API๊ฐ 100ms๋ง์ ์คํจํ๋ฉด skeleton์ด 100ms๋ง ํ์๋๊ณ ๋ฐ๋ก ์๋ฌ ์ํ๋ก ์ ํ๋ฉ๋๋ค.
์์ ํ ํด๊ฒฐ ๋ฐฉ๋ฒ:
try {
setLoading(true);
const minLoadingTime = new Promise(resolve => setTimeout(resolve, 500));
const [feedResponse, commentsResponse] = await Promise.all([
getFeedDetail(Number(feedId)),
getComments(Number(feedId), { postType: 'FEED' }),
]);
await minLoadingTime;
setFeedData(feedResponse.data);
setCommentList(commentsResponse.data.commentList);
setError(null);
} catch (err) {
+ await minLoadingTime;
console.error('ํผ๋ ์์ธ ์ ๋ณด ๋๋ ๋๊ธ ๋ก๋ ์คํจ:', err);
setError('ํผ๋ ์ ๋ณด๋ฅผ ๋ถ๋ฌ์ค๋๋ฐ ์คํจํ์ต๋๋ค.');
} finally {
setLoading(false);
}์ด๋ ๊ฒ ํ๋ฉด ์๋ฌ๊ฐ ๋ฐ์ํด๋ ์ต์ 500ms ๋์ skeleton์ ํ์ํ ์ ์์ต๋๋ค.
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/groupSearch/GroupSearch.tsx (1)
50-59:โ ๏ธ Potential issue | ๐ก Minor๋ง์ดํธ ์ ์ต๊ทผ ๊ฒ์์ด ์กฐํ๊ฐ ์ค๋ณต ํธ์ถ๋ฉ๋๋ค.
Line [51]๊ณผ Line [56]์์ ์ด๊ธฐ ๋ ๋ ์
fetchRecentSearches๊ฐ 2๋ฒ ์คํ๋ฉ๋๋ค. ๋ถํ์ํ ๋คํธ์ํฌ ํธ์ถ/์ํ ๊ฒฝํฉ์ ์ค์ด๊ธฐ ์ํด ํ ๊ฒฝ๋ก๋ก ํตํฉํ๋ ๊ฒ ์ข์ต๋๋ค.๐ง ์ ์ ์์ ์
- useEffect(() => { - fetchRecentSearches(); - // eslint-disable-next-line react-hooks/exhaustive-deps - }, []); - useEffect(() => { if (searchStatus === 'idle') { fetchRecentSearches(); } }, [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, fetchRecentSearches is being invoked twice on mount because there's a useEffect with an empty dependency array calling fetchRecentSearches and a second useEffect that calls fetchRecentSearches when searchStatus === 'idle', causing duplicate network calls; consolidate into a single control path by removing the unconditional mount call and relying on the searchStatus-driven effect (or vice versa) โ update the effects around fetchRecentSearches and searchStatus so only one effect (either the empty-deps useEffect or the [searchStatus] useEffect that checks for 'idle') triggers the initial fetch to eliminate the duplicate call.
โป๏ธ Duplicate comments (3)
src/pages/searchBook/SearchBook.tsx (2)
247-258:if (error)๋ธ๋ก ๋ด๋ถ์ ํด๋ฐฑ ๋ฌธ์์ด์ด ๋๋ฌ ๋ถ๊ฐ๋ฅํ dead code์ ๋๋ค.Line 254์์
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 247 - 258, The JSX inside the if (error) branch contains a dead fallback string ("|| '์ฑ ์ ๋ณด๋ฅผ ์ฐพ์ ์ ์์ต๋๋ค.'") because the clause only runs when error is truthy; update the rendering in SearchBook.tsx to remove the unreachable fallback or change the condition/render logic so the fallback is shown when error is falsyโe.g., either remove "|| '์ฑ ์ ๋ณด๋ฅผ ์ฐพ์ ์ ์์ต๋๋ค.'" from the block that returns Wrapper/Header/IconButton (using symbols error, Wrapper, Header, IconButton, leftArrow, handleBackButton), or replace the top-level conditional with a ternary (error ? error : '์ฑ ์ ๋ณด๋ฅผ ์ฐพ์ ์ ์์ต๋๋ค.') so the fallback can be reached.
260-262:bookDetail์ดnull์ผ ๋TopBackground๊ฐ ๋น ๋ฌธ์์ด URL๋ก ๋ ๋๋ง๋ฉ๋๋ค.
bookDetail?.imageUrl || ''๋ ๋ก๋ฉ ์ค์ ๋นurl("")CSS๋ฅผ ์์ฑํ์ฌ ๋ถํ์ํ ๋คํธ์ํฌ ์์ฒญ์ด๋ ์ฝ์ ๊ฒฝ๊ณ ๋ฅผ ์ ๋ฐํ ์ ์์ต๋๋ค.โป๏ธ ์ ์: ์กฐ๊ฑด๋ถ ๋ ๋๋ง
- <TopBackground bookImgUrl={bookDetail?.imageUrl || ''} /> + {bookDetail && <TopBackground bookImgUrl={bookDetail.imageUrl} />}๐ค 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 260 - 262, The TopBackground is currently rendered with bookImgUrl={bookDetail?.imageUrl || ''}, causing an empty url("") when bookDetail is null; change this to conditionally render TopBackground only when bookDetail?.imageUrl is truthy (use a guard around the TopBackground component) so you pass a real image URL (bookDetail.imageUrl) or avoid rendering it entirely when absent; update the JSX near Wrapper/TopBackground references to use this conditional rendering pattern to prevent empty CSS urls and unnecessary network/console noise.src/pages/groupSearch/GroupSearch.tsx (1)
380-387:โ ๏ธ Potential issue | ๐ก Minor์ต๊ทผ ๊ฒ์์ด ์ญ์ ์ฝ๋ฐฑ์ ์์ธ ์ฒ๋ฆฌ๊ฐ ํ์ํฉ๋๋ค.
Line [383]์
deleteRecentSearch๊ฐ throwํ ๊ฒฝ์ฐ ํ์ฌ ํธ๋ค๋ฌ์์ unhandled rejection์ด ๋ฐ์ํ ์ ์์ต๋๋ค.๐ง ์ ์ ์์ ์
handleDelete={async (term: string) => { const x = recentSearches.find(i => i.searchTerm === term); if (!x) return; - const res = await deleteRecentSearch(x.recentSearchId); - if (res.isSuccess) { - await fetchRecentSearches(); + try { + const res = await deleteRecentSearch(x.recentSearchId); + if (res.isSuccess) { + await fetchRecentSearches(); + } + } catch (error) { + console.error('์ต๊ทผ ๊ฒ์์ด ์ญ์ ์ค๋ฅ:', error); } }}๐ค 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 380 - 387, The handleDelete callback currently calls deleteRecentSearch asynchronously without catching exceptions; wrap the async body of handleDelete in a try/catch so any thrown error from deleteRecentSearch (or subsequent await fetchRecentSearches) is caught and handled (e.g., log via console/error logger and optionally surface a user-facing error), ensure you still return/exit early if recentSearches.find(...) returns undefined, and keep calling fetchRecentSearches only on successful delete responses (res.isSuccess) inside the try block to avoid unhandled promise rejections.
๐งน Nitpick comments (1)
src/pages/searchBook/SearchBook.tsx (1)
84-89:minLoadingTime์ ๋ช ์์ Promise<void>ํ์ ์ ์ง์ ํ์ธ์.
new Promise(resolve => setTimeout(resolve, 500))๋ TypeScript๊ฐPromise<unknown>์ผ๋ก ์ถ๋ก ํฉ๋๋ค. ์๋๋ฅผ ๋ช ํํ ํ๊ณPromise.all์ ๋ฐํ ํ์ ์ถ๋ก ์ ๊น๋ํ๊ฒ ์ ์งํ๋ ค๋ฉดPromise<void>๋ฅผ ๋ช ์ํ๋ ๊ฒ์ด ์ข์ต๋๋ค.โป๏ธ ์ ์: ๋ช ์์ ํ์ ์ด๋ ธํ ์ด์
- const minLoadingTime = new Promise(resolve => setTimeout(resolve, 500)); + const minLoadingTime = new Promise<void>(resolve => setTimeout(resolve, 500));(Line 120 ๋์ผํ๊ฒ ์ ์ฉ)
- const minLoadingTime = new Promise(resolve => setTimeout(resolve, 500)); + const minLoadingTime = new Promise<void>(resolve => setTimeout(resolve, 500));Also applies to: 120-124
๐ค 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 84 - 89, The temporary loading timer promise minLoadingTime is inferred as Promise<unknown>; update its declaration to explicitly type it as Promise<void> (e.g., const minLoadingTime: Promise<void> = new Promise(resolve => setTimeout(resolve, 500))); do the same for the other instance around lines 120-124 so Promise.all([getBookDetail(isbn), getRecruitingRooms(isbn), minLoadingTime]) keeps correct type inference for getBookDetail and getRecruitingRooms.
๐ค 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/groupSearch/GroupSearch.tsx`:
- Around line 50-59: fetchRecentSearches is being invoked twice on mount because
there's a useEffect with an empty dependency array calling fetchRecentSearches
and a second useEffect that calls fetchRecentSearches when searchStatus ===
'idle', causing duplicate network calls; consolidate into a single control path
by removing the unconditional mount call and relying on the searchStatus-driven
effect (or vice versa) โ update the effects around fetchRecentSearches and
searchStatus so only one effect (either the empty-deps useEffect or the
[searchStatus] useEffect that checks for 'idle') triggers the initial fetch to
eliminate the duplicate call.
---
Duplicate comments:
In `@src/pages/groupSearch/GroupSearch.tsx`:
- Around line 380-387: The handleDelete callback currently calls
deleteRecentSearch asynchronously without catching exceptions; wrap the async
body of handleDelete in a try/catch so any thrown error from deleteRecentSearch
(or subsequent await fetchRecentSearches) is caught and handled (e.g., log via
console/error logger and optionally surface a user-facing error), ensure you
still return/exit early if recentSearches.find(...) returns undefined, and keep
calling fetchRecentSearches only on successful delete responses (res.isSuccess)
inside the try block to avoid unhandled promise rejections.
In `@src/pages/searchBook/SearchBook.tsx`:
- Around line 247-258: The JSX inside the if (error) branch contains a dead
fallback string ("|| '์ฑ
์ ๋ณด๋ฅผ ์ฐพ์ ์ ์์ต๋๋ค.'") because the clause only runs when
error is truthy; update the rendering in SearchBook.tsx to remove the
unreachable fallback or change the condition/render logic so the fallback is
shown when error is falsyโe.g., either remove "|| '์ฑ
์ ๋ณด๋ฅผ ์ฐพ์ ์ ์์ต๋๋ค.'" from the
block that returns Wrapper/Header/IconButton (using symbols error, Wrapper,
Header, IconButton, leftArrow, handleBackButton), or replace the top-level
conditional with a ternary (error ? error : '์ฑ
์ ๋ณด๋ฅผ ์ฐพ์ ์ ์์ต๋๋ค.') so the fallback
can be reached.
- Around line 260-262: The TopBackground is currently rendered with
bookImgUrl={bookDetail?.imageUrl || ''}, causing an empty url("") when
bookDetail is null; change this to conditionally render TopBackground only when
bookDetail?.imageUrl is truthy (use a guard around the TopBackground component)
so you pass a real image URL (bookDetail.imageUrl) or avoid rendering it
entirely when absent; update the JSX near Wrapper/TopBackground references to
use this conditional rendering pattern to prevent empty CSS urls and unnecessary
network/console noise.
---
Nitpick comments:
In `@src/pages/searchBook/SearchBook.tsx`:
- Around line 84-89: The temporary loading timer promise minLoadingTime is
inferred as Promise<unknown>; update its declaration to explicitly type it as
Promise<void> (e.g., const minLoadingTime: Promise<void> = new Promise(resolve
=> setTimeout(resolve, 500))); do the same for the other instance around lines
120-124 so Promise.all([getBookDetail(isbn), getRecruitingRooms(isbn),
minLoadingTime]) keeps correct type inference for getBookDetail and
getRecruitingRooms.
โน๏ธ 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 (2)
src/pages/groupSearch/GroupSearch.tsxsrc/pages/searchBook/SearchBook.tsx
- Promise.all ๋ด๋ถ ํฌํจ ์ API ์คํจํ๋ฉด ์ฆ์ reject๋์ด 500ms ๋ฏธ๋ณด์ฅ - minLoadingTime์ Promise.all ์ธ๋ถ๋ก ๋ถ๋ฆฌํ์ฌ ์ฑ๊ณต/์คํจ ๋ฌด๊ดํ๊ฒ ๋ณด์ฅ - 10๊ฐ ํ์ผ ๋์ผ ํจํด ์ผ๊ด ์์
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/groupDetail/GroupDetail.tsx (1)
56-56:โ ๏ธ Potential issue | ๐ก Minor
isJoining์ดnull์ธ ์ฑ๋ก ํ ํ๋ ์ ๋ ๋๋ง๋์ด ๋ฒํผ ํ ์คํธ๊ฐ ์๊ฐ์ ์ผ๋ก '์ฐธ์ฌํ๊ธฐ'๋ก ํ์๋ฉ๋๋ค.
isJoining์useEffect๋กroomData๋ณํ์ ๋ฐ์ํด ์ค์ ๋์ง๋ง, React์ ํจ๊ณผ๋ ํ์ธํธ ์ดํ์ ์คํ๋ฉ๋๋ค. ๋ฐ๋ผ์isLoading || !roomData๊ฐ๋๊ฐ ํด์ ๋ ์งํ ํ ํ๋ ์ ๋์isJoining === null์ธ ์ํ๋ก ๋ ๋๋ง๋์ดbuttonProps๊ฐ ์ค์ ๋ก ์ฐธ์ฌ ์ค์ธ ์ฌ์ฉ์์๊ฒ๋'์ฐธ์ฌํ๊ธฐ'๋ฅผ ๋ฐํํ๋ ์๊ฐ์ด ์กด์ฌํฉ๋๋ค.
isJoining์ fetch ์๋ฃ ์์ ์ ๋ฐ๋ก ์ค์ ํ๋ฉด ์ด ๋ฌธ์ ๋ฅผ ์ ๊ฑฐํ ์ ์์ต๋๋ค.๐ ์ ์ ์์
- useEffect(() => { - if (roomData) { - setIsJoining(roomData.isJoining); - } - }, [roomData]);
fetchRoomDetail๋ด๋ถ์์ ์ง์ ์ค์ :if (response.isSuccess) { setRoomData(response.data); + setIsJoining(response.data.isJoining); } else {Also applies to: 126-130, 287-292
๐ค 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 transient render showing '์ฐธ์ฌํ๊ธฐ' happens because isJoining starts as null and is set in a useEffect after paint; to fix, initialize isJoining to a non-null default and set it immediately when room data is fetched: change the useState for isJoining to a non-null default (e.g., false) and move the logic that determines whether the current user is joining into fetchRoomDetail so it calls setIsJoining(...) right after roomData is retrieved; remove or stop relying on the separate useEffect that sets isJoining based on roomData and ensure buttonProps reads the updated isJoining set by fetchRoomDetail.
โป๏ธ Duplicate comments (3)
src/pages/searchBook/SearchBook.tsx (2)
247-258: Line 254:error๊ฐ๋ ๋ด๋ถ ํด๋ฐฑ ๋ฌธ์์ด์ dead code์ ๋๋ค.๐ค 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 247 - 258, The current if (error) block renders {error || '์ฑ ์ ๋ณด๋ฅผ ์ฐพ์ ์ ์์ต๋๋ค.'}, making the fallback string dead code; change the rendering logic so the fallback displays when error is falsy. For example, compute a message const message = error || '์ฑ ์ ๋ณด๋ฅผ ์ฐพ์ ์ ์์ต๋๋ค.' (or use null-coalescing) and render {message} inside the Wrapper, or invert the condition to if (!error) show the fallback and if (error) show the error; update the JSX that uses Wrapper, Header, IconButton, leftArrow and handleBackButton to render message accordingly.
260-262:TopBackground๊ฐ ๋ก๋ฉ ์ค์ ๋น ๋ฌธ์์ด URL๋ก ๋ ๋๋ง๋ฉ๋๋ค.๐ค 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 260 - 262, TopBackground is rendered with an empty string while bookDetail is loading; change to render TopBackground only when a valid image URL exists by wrapping the component in a conditional that checks bookDetail?.imageUrl (or pass undefined/null instead of ''), e.g., guard the TopBackground render in the return so TopBackground(bookImgUrl={bookDetail.imageUrl}) only mounts when bookDetail?.imageUrl is truthy; refer to TopBackground and bookDetail usages in SearchBook component and the surrounding Wrapper.src/pages/groupSearch/GroupSearch.tsx (1)
380-387:โ ๏ธ Potential issue | ๐ก Minor์ต๊ทผ ๊ฒ์์ด ์ญ์ ์ฝ๋ฐฑ์ ์์ธ ์ฒ๋ฆฌ๊ฐ ํ์ํฉ๋๋ค.
Line 383์
deleteRecentSearch๋ ์คํจ ์ throw ๊ฐ๋ฅํ๋ฐ, ํ์ฌhandleDelete์์try/catch๊ฐ ์์ด unhandled rejection์ด ๋ฐ์ํ ์ ์์ต๋๋ค.๐ง ์ ์ ์์ ์
handleDelete={async (term: string) => { const x = recentSearches.find(i => i.searchTerm === term); if (!x) return; - const res = await deleteRecentSearch(x.recentSearchId); - if (res.isSuccess) { - await fetchRecentSearches(); + try { + const res = await deleteRecentSearch(x.recentSearchId); + if (res.isSuccess) { + await fetchRecentSearches(); + } + } catch (error) { + console.error('์ต๊ทผ ๊ฒ์์ด ์ญ์ ์ค๋ฅ:', error); } }}๐ค 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 380 - 387, The handleDelete callback risks unhandled rejections because deleteRecentSearch can throw; wrap the async work in a try/catch inside the handleDelete passed to the component (the arrow function referencing recentSearches, deleteRecentSearch, fetchRecentSearches and recentSearchId), call await deleteRecentSearch(x.recentSearchId) inside the try, on success call await fetchRecentSearches(), and in the catch log the error and/or surface a user-facing message (e.g., showToast or processLogger) so failures are handled gracefully.
๐งน Nitpick comments (4)
src/pages/searchBook/SearchBook.tsx (1)
120-124: ๋จ์ผ ์์Promise.all์ ๋ถํ์ํฉ๋๋ค.
getFeedsByIsbnํ๋๋ง ๊ฐ์ธ๋Promise.all๊ณผ ๋ฐฐ์ด ๊ตฌ์กฐ ๋ถํดconst [res]๋ ์๋ฏธ ์๋ ๋ณต์ก๋๋ฅผ ์ถ๊ฐํฉ๋๋ค. ๋จ์ํawait์ผ๋ก ์ถฉ๋ถํฉ๋๋ค.โป๏ธ ์ ์: ๋จ์ await์ผ๋ก ๋ณ๊ฒฝ
- const [res] = await Promise.all([ - getFeedsByIsbn(isbn, toFeedSort(selectedFilter), null), - ]); + const res = await getFeedsByIsbn(isbn, toFeedSort(selectedFilter), null);๐ค 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 120 - 124, The code uses Promise.all with a single element and array destructuring (const [res] = await Promise.all([ getFeedsByIsbn(isbn, toFeedSort(selectedFilter), null) ])), which is unnecessary; replace it with a direct await call to getFeedsByIsbn (e.g., const res = await getFeedsByIsbn(isbn, toFeedSort(selectedFilter), null)) and keep the existing minLoadingTime await, ensuring references to minLoadingTime, getFeedsByIsbn, toFeedSort, selectedFilter, and isbn are preserved and no array destructuring is used.src/pages/groupSearch/GroupSearch.tsx (1)
93-105: ์ต์ ์ง์ฐ ๋๊ธฐ ๋ก์ง์Promise.allํ ๋ฒ์ผ๋ก ํต์ผํด๋ ์ข๊ฒ ์ต๋๋ค.ํ์ฌ ๋์์ ๋ง์ง๋ง(
await minLoadingTime๋ณ๋ ๋๊ธฐ),fetchRecentSearches์ ํจํด์ ํต์ผํ๋ฉด ์๋ ํ์ ์ด ๋ ์ฌ์์ง๋๋ค.๐ง ์ ์ ์์ ์
try { const isFinalized = status === 'searched'; const minLoadingTime = new Promise(resolve => setTimeout(resolve, 500)); - const [res] = await Promise.all([ + const [res] = await Promise.all([ getSearchRooms( term.trim(), sortKey, undefined, isFinalized, categoryParam, isAllCategory, ), + minLoadingTime, ]); - await minLoadingTime; if (res.isSuccess) {๐ค 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 93 - 105, Unify the minimum loading delay with the fetch by including the minLoadingTime promise in the same Promise.all call instead of awaiting it separately: replace the current separate minLoadingTime creation + awaiting with Promise.all([ getSearchRooms(...), minLoadingTime ]) and destructure the getSearchRooms result (e.g., const [res] = await Promise.all([...]) or const [res] = ... depending on order) so the logic in getSearchRooms, minLoadingTime, and the subsequent if (res.isSuccess) remains unchanged (mirror the fetchRecentSearches pattern).src/pages/groupDetail/ParticipatedGroupDetail.tsx (1)
82-86:Promise.all๋ฐฐ์ด์minLoadingTime์ ํฌํจํ์ธ์.ํ์ฌ ์ฝ๋์์
Promise.all์getRoomPlayingํ๋๋ง ํฌํจํ๊ณ ,minLoadingTime์ ๋ณ๋๋กawaitํฉ๋๋ค. ๋์์ ๋์ผํ์ง๋ง,Promise.all์ ์๋๊ฐ ๋ถ๋ช ํํฉ๋๋ค.minLoadingTime์ ๋ฐฐ์ด ์์ ํจ๊ป ๋ฃ๋ ๊ฒ์ด ๋ณ๋ ฌ ์คํ ํจํด์ ๊ด์ฉ์ ์ธ ํํ์ ๋๋ค.โป๏ธ ์ ์ ์์
- const minLoadingTime = new Promise(resolve => setTimeout(resolve, 500)); - const [response] = await Promise.all([ - getRoomPlaying(parseInt(roomId)), - ]); - await minLoadingTime; + const [response] = await Promise.all([ + getRoomPlaying(parseInt(roomId)), + new Promise(resolve => setTimeout(resolve, 500)), + ]);๐ค 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 82 - 86, The current code creates minLoadingTime and awaits it separately after awaiting Promise.all([getRoomPlaying(parseInt(roomId))]); change this to run both in parallel by moving minLoadingTime into the Promise.all call so Promise.all([ getRoomPlaying(parseInt(roomId)), minLoadingTime ]) is awaited together (and remove the separate await minLoadingTime); this keeps the existing response extraction from the array returned by Promise.all while ensuring the artificial minimum loading delay runs in parallel with getRoomPlaying.src/pages/groupDetail/GroupDetail.tsx (1)
93-97:Promise.all์ ๋จ์ผ ์์๋ง ํฌํจ๋์ดminLoadingTime์ด ๋ฐฐ์ด ๋ฐ์์ ๋ฐ๋กawait๋๊ณ ์์ต๋๋ค.
minLoadingTime์ ํ์ด๋จธ ์์ ํPromise.all๊ณผ ๋ณ๊ฐ๋กawait๋๋ฏ๋ก ๋์์ ์ฌ๋ฐ๋ฅด์ง๋ง,Promise.all์ด ๋จ์ผ ์์๋ง ๊ฐ์ธ๊ณ ์์ด ์๋๊ฐ ๋ชจํธํฉ๋๋ค.minLoadingTime์ ๋ฐฐ์ด ์์ ํฌํจ์ํค๋ ๊ฒ์ด ๊ด์ฉ์ ์ด๋ฉฐ ๊ฐ๋ ์ฑ๋ ๋์ต๋๋ค.โป๏ธ ์ ์ ๋ฆฌํฉํฐ
- const minLoadingTime = new Promise(resolve => setTimeout(resolve, 500)); - const [response] = await Promise.all([ - getRoomDetail(Number(roomId)), - ]); - await minLoadingTime; + const [response] = await Promise.all([ + getRoomDetail(Number(roomId)), + new Promise(resolve => setTimeout(resolve, 500)), + ]);๐ค 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 93 - 97, The code currently awaits minLoadingTime separately, which is confusing because Promise.all only contains getRoomDetail; update the await to include minLoadingTime inside the Promise.all call so both promises are awaited together (e.g., call Promise.all([ getRoomDetail(Number(roomId)), minLoadingTime ]) and adjust destructuring to capture getRoomDetail's result (keep const [response] = ...) and remove the separate await minLoadingTime; reference: minLoadingTime, getRoomDetail, roomId, response.
๐ค 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/components/search/MostSearchedBooks.tsx`:
- Around line 30-35: The current fail-fast Promise.all can skip awaiting
minLoadingTime when getMostSearchedBooks() rejects; change the logic so
minLoadingTime is always awaited before you update loading state or render
error: start minLoadingTime as now, run the fetch using Promise.allSettled (or
wrap getMostSearchedBooks() into a promise that never rejects), then await
minLoadingTime before handling results; in practice replace Promise.all([...])
with Promise.allSettled([getMostSearchedBooks()]) (referencing
getMostSearchedBooks, minLoadingTime and setIsLoading in MostSearchedBooks.tsx),
then after await minLoadingTime inspect the settled result (response.status ===
'fulfilled' ? response.value : handle error) and only then call
setIsLoading(false) or set error state so the 500ms skeleton is always honored.
In `@src/pages/groupDetail/ParticipatedGroupDetail.tsx`:
- Around line 67-69: The back-navigation behavior is inconsistent:
handleBackClick uses navigate(-1) while handleBackButton uses navigate('/group')
and their names are confusing; pick the intended behavior (either always go to
'/group' or always go back one step), replace the other handler so both error
and normal/loading states call the same function (e.g., unify into a single
handler name like handleBack or handleNavigateBack), update all onLeftClick
usages (including the error-state onLeftClick) to use that unified handler, and
remove the redundant handler to avoid duplicate implementations.
---
Outside diff comments:
In `@src/pages/groupDetail/GroupDetail.tsx`:
- Line 56: The transient render showing '์ฐธ์ฌํ๊ธฐ' happens because isJoining starts
as null and is set in a useEffect after paint; to fix, initialize isJoining to a
non-null default and set it immediately when room data is fetched: change the
useState for isJoining to a non-null default (e.g., false) and move the logic
that determines whether the current user is joining into fetchRoomDetail so it
calls setIsJoining(...) right after roomData is retrieved; remove or stop
relying on the separate useEffect that sets isJoining based on roomData and
ensure buttonProps reads the updated isJoining set by fetchRoomDetail.
---
Duplicate comments:
In `@src/pages/groupSearch/GroupSearch.tsx`:
- Around line 380-387: The handleDelete callback risks unhandled rejections
because deleteRecentSearch can throw; wrap the async work in a try/catch inside
the handleDelete passed to the component (the arrow function referencing
recentSearches, deleteRecentSearch, fetchRecentSearches and recentSearchId),
call await deleteRecentSearch(x.recentSearchId) inside the try, on success call
await fetchRecentSearches(), and in the catch log the error and/or surface a
user-facing message (e.g., showToast or processLogger) so failures are handled
gracefully.
In `@src/pages/searchBook/SearchBook.tsx`:
- Around line 247-258: The current if (error) block renders {error || '์ฑ
์ ๋ณด๋ฅผ ์ฐพ์
์ ์์ต๋๋ค.'}, making the fallback string dead code; change the rendering logic so
the fallback displays when error is falsy. For example, compute a message const
message = error || '์ฑ
์ ๋ณด๋ฅผ ์ฐพ์ ์ ์์ต๋๋ค.' (or use null-coalescing) and render
{message} inside the Wrapper, or invert the condition to if (!error) show the
fallback and if (error) show the error; update the JSX that uses Wrapper,
Header, IconButton, leftArrow and handleBackButton to render message
accordingly.
- Around line 260-262: TopBackground is rendered with an empty string while
bookDetail is loading; change to render TopBackground only when a valid image
URL exists by wrapping the component in a conditional that checks
bookDetail?.imageUrl (or pass undefined/null instead of ''), e.g., guard the
TopBackground render in the return so
TopBackground(bookImgUrl={bookDetail.imageUrl}) only mounts when
bookDetail?.imageUrl is truthy; refer to TopBackground and bookDetail usages in
SearchBook component and the surrounding Wrapper.
---
Nitpick comments:
In `@src/pages/groupDetail/GroupDetail.tsx`:
- Around line 93-97: The code currently awaits minLoadingTime separately, which
is confusing because Promise.all only contains getRoomDetail; update the await
to include minLoadingTime inside the Promise.all call so both promises are
awaited together (e.g., call Promise.all([ getRoomDetail(Number(roomId)),
minLoadingTime ]) and adjust destructuring to capture getRoomDetail's result
(keep const [response] = ...) and remove the separate await minLoadingTime;
reference: minLoadingTime, getRoomDetail, roomId, response.
In `@src/pages/groupDetail/ParticipatedGroupDetail.tsx`:
- Around line 82-86: The current code creates minLoadingTime and awaits it
separately after awaiting Promise.all([getRoomPlaying(parseInt(roomId))]);
change this to run both in parallel by moving minLoadingTime into the
Promise.all call so Promise.all([ getRoomPlaying(parseInt(roomId)),
minLoadingTime ]) is awaited together (and remove the separate await
minLoadingTime); this keeps the existing response extraction from the array
returned by Promise.all while ensuring the artificial minimum loading delay runs
in parallel with getRoomPlaying.
In `@src/pages/groupSearch/GroupSearch.tsx`:
- Around line 93-105: Unify the minimum loading delay with the fetch by
including the minLoadingTime promise in the same Promise.all call instead of
awaiting it separately: replace the current separate minLoadingTime creation +
awaiting with Promise.all([ getSearchRooms(...), minLoadingTime ]) and
destructure the getSearchRooms result (e.g., const [res] = await
Promise.all([...]) or const [res] = ... depending on order) so the logic in
getSearchRooms, minLoadingTime, and the subsequent if (res.isSuccess) remains
unchanged (mirror the fetchRecentSearches pattern).
In `@src/pages/searchBook/SearchBook.tsx`:
- Around line 120-124: The code uses Promise.all with a single element and array
destructuring (const [res] = await Promise.all([ getFeedsByIsbn(isbn,
toFeedSort(selectedFilter), null) ])), which is unnecessary; replace it with a
direct await call to getFeedsByIsbn (e.g., const res = await
getFeedsByIsbn(isbn, toFeedSort(selectedFilter), null)) and keep the existing
minLoadingTime await, ensuring references to minLoadingTime, getFeedsByIsbn,
toFeedSort, selectedFilter, and isbn are preserved and no array destructuring is
used.
โน๏ธ 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 (13)
src/components/feed/MyFeed.tsxsrc/components/search/MostSearchedBooks.tsxsrc/pages/feed/FeedDetailPage.tsxsrc/pages/feed/FollowerListPage.tsxsrc/pages/feed/MyFeedPage.tsxsrc/pages/feed/OtherFeedPage.tsxsrc/pages/groupDetail/GroupDetail.tsxsrc/pages/groupDetail/ParticipatedGroupDetail.tsxsrc/pages/groupMembers/GroupMembers.tsxsrc/pages/groupSearch/GroupSearch.tsxsrc/pages/mypage/SavePage.tsxsrc/pages/search/Search.tsxsrc/pages/searchBook/SearchBook.tsx
๐ง Files skipped from review as they are similar to previous changes (6)
- src/pages/feed/FeedDetailPage.tsx
- src/pages/mypage/SavePage.tsx
- src/pages/groupMembers/GroupMembers.tsx
- src/pages/search/Search.tsx
- src/pages/feed/MyFeedPage.tsx
- src/pages/feed/FollowerListPage.tsx
| const minLoadingTime = new Promise(resolve => setTimeout(resolve, 500)); | ||
| const [response] = await Promise.all([ | ||
| getMostSearchedBooks(), | ||
| ]); | ||
| await minLoadingTime; | ||
|
|
There was a problem hiding this comment.
๐งฉ Analysis chain
๐ Web query:
Promise.all reject immediately vs Promise.allSettled wait for all promises
๐ก Result:
-
Promise.all(iterable): returns a promise that fulfills only if all input promises fulfill, but rejects as soon as the first one rejects (fail-fast). The rejection reason is from that first rejection. (mdn2.netlify.app)- Important: it does not cancel the other promisesโthose async operations may keep running; you just stop waiting for their results via that
Promise.allcall. (javascript.info)
- Important: it does not cancel the other promisesโthose async operations may keep running; you just stop waiting for their results via that
-
Promise.allSettled(iterable): returns a promise that waits until every input promise settles (fulfilled or rejected), then fulfills with an array like[{status:"fulfilled", value}, {status:"rejected", reason}, ...]in the original order. (developer.mozilla.org)
Citations:
- 1: https://mdn2.netlify.app/en-US/docs/Web/JavaScript/Reference/Global_Objects/Promise/all?utm_source=openai
- 2: https://javascript.info/task/promise-all-failure?utm_source=openai
- 3: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Promise/allSettled?utm_source=openai
๐ Script executed:
cd /repo && cat -n src/components/search/MostSearchedBooks.tsx | head -80Repository: THIP-TextHip/THIP-Web
Length of output: 120
๐ Script executed:
git ls-files | grep -i "MostSearchedBooks.tsx"Repository: THIP-TextHip/THIP-Web
Length of output: 108
๐ Script executed:
git ls-files | grep -E "search|components" | head -20Repository: THIP-TextHip/THIP-Web
Length of output: 914
๐ Script executed:
cat -n src/components/search/MostSearchedBooks.tsxRepository: THIP-TextHip/THIP-Web
Length of output: 3329
๐ Script executed:
git ls-files | grep -i "getMostSearchedBooks"Repository: THIP-TextHip/THIP-Web
Length of output: 102
๐ Script executed:
cat -n src/api/books/getMostSearchedBooks.ts 2>/dev/null || find . -name "*getMostSearchedBooks*" -type fRepository: THIP-TextHip/THIP-Web
Length of output: 932
์คํจ ๊ฒฝ๋ก์์ 500ms ์ต์ ๋ก๋ฉ ์๊ฐ์ด ๋ณด์ฅ๋์ง ์์ต๋๋ค.
Promise.all์ ์ฒซ ๋ฒ์งธ Promise ๊ฑฐ๋ถ ์ ์ฆ์ ์คํจํ๋ฏ๋ก(fail-fast), getMostSearchedBooks()์ด ์๋ฌ๋ฅผ throwํ๋ฉด Line 34์ await minLoadingTime์ ๋๋ฌํ์ง ์๊ณ catch ๋ธ๋ก์ผ๋ก ์ด๋ํฉ๋๋ค. ๊ฒฐ๊ณผ์ ์ผ๋ก setIsLoading(false)๊ฐ ์ง์ฐ ์์ด ์คํ๋์ด ์ค์ผ๋ ํค์ด 500ms ๋ฏธ๋ง์ผ๋ก ํ์๋๊ณ , ์ฆ์ ์๋ฌ ๋ฉ์์ง๋ก ์ ํ๋์ด ํ๋ฉด ๊น๋นก์์ด ๋ฐ์ํฉ๋๋ค.
๐ง ์ ์ ์์ ์
- const minLoadingTime = new Promise(resolve => setTimeout(resolve, 500));
- const [response] = await Promise.all([
- getMostSearchedBooks(),
- ]);
- await minLoadingTime;
+ const minLoadingTime = new Promise<void>(resolve => setTimeout(resolve, 500));
+ const [responseResult] = await Promise.allSettled([getMostSearchedBooks()]);
+ await minLoadingTime;
+
+ if (responseResult.status === 'rejected') {
+ throw responseResult.reason;
+ }
+ const response = responseResult.value;๐ 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 minLoadingTime = new Promise(resolve => setTimeout(resolve, 500)); | |
| const [response] = await Promise.all([ | |
| getMostSearchedBooks(), | |
| ]); | |
| await minLoadingTime; | |
| const minLoadingTime = new Promise<void>(resolve => setTimeout(resolve, 500)); | |
| const [responseResult] = await Promise.allSettled([getMostSearchedBooks()]); | |
| await minLoadingTime; | |
| if (responseResult.status === 'rejected') { | |
| throw responseResult.reason; | |
| } | |
| const response = responseResult.value; |
๐ค Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/search/MostSearchedBooks.tsx` around lines 30 - 35, The
current fail-fast Promise.all can skip awaiting minLoadingTime when
getMostSearchedBooks() rejects; change the logic so minLoadingTime is always
awaited before you update loading state or render error: start minLoadingTime as
now, run the fetch using Promise.allSettled (or wrap getMostSearchedBooks() into
a promise that never rejects), then await minLoadingTime before handling
results; in practice replace Promise.all([...]) with
Promise.allSettled([getMostSearchedBooks()]) (referencing getMostSearchedBooks,
minLoadingTime and setIsLoading in MostSearchedBooks.tsx), then after await
minLoadingTime inspect the settled result (response.status === 'fulfilled' ?
response.value : handle error) and only then call setIsLoading(false) or set
error state so the 500ms skeleton is always honored.
| const handleBackClick = () => { | ||
| navigate(-1); | ||
| }; |
There was a problem hiding this comment.
์๋ฌ/๋ก๋ฉ ์ํ์ ๋ค๋ก๊ฐ๊ธฐ ๋์ ๋ถ์ผ์น๋ฅผ ํ์ธํ์ธ์.
handleBackClick(์๋ฌ ์ํ)์ navigate(-1)์ ์ฌ์ฉํ๊ณ , handleBackButton(๋ก๋ฉยท์ ์ ์ํ)์ navigate('/group')๋ก ์ด๋ํฉ๋๋ค. ๊ฐ์ ๋ค๋ก๊ฐ๊ธฐ ๋ฒํผ์ด ์ํ์ ๋ฐ๋ผ ๋ค๋ฅธ ๋ชฉ์ ์ง๋ก ์ด๋ํ๋ฉด ์ฌ์ฉ์ ๊ฒฝํ์ด ์ผ๊ด๋์ง ์์ ์ ์์ต๋๋ค.
- ์๋ฌ ์ํ์์๋
/group์ผ๋ก ์ด๋ํ๋ ๊ฒ์ด ์๋๋ผ๋ฉดhandleBackClick์handleBackButton์ผ๋ก ๋์ฒดํ๊ณ ์ค๋ณต ํธ๋ค๋ฌ๋ฅผ ์ ๊ฑฐํ์ธ์. navigate(-1)์ด ์๋์ ์ด๋ผ๋ฉด ๋ก๋ฉ ์ํ์ ๋ค๋ก๊ฐ๊ธฐ๋ ๋์ผํ๊ฒ ๋ง์ถ๋ ๊ฒ์ด ์ข์ต๋๋ค.
์ถ๊ฐ๋ก, ๋ ํธ๋ค๋ฌ์ ์ด๋ฆ(handleBackClick vs handleBackButton)์ด ์๋ฏธ์ ๊ตฌ๋ถ์ด ๋ถ๋ช
ํํ์ฌ ํผ๋์ ์ค ์ ์์ต๋๋ค.
โป๏ธ ๋จ์ผ ํธ๋ค๋ฌ๋ก ํต์ผํ๋ ๊ฒฝ์ฐ์ ์์
- const handleBackClick = () => {
- navigate(-1);
- };
-
...
const handleBackButton = () => {
navigate('/group');
};๊ทธ๋ฆฌ๊ณ ์๋ฌ ์ํ์ onLeftClick์ handleBackButton์ผ๋ก ๊ต์ฒด:
- onLeftClick={handleBackClick}
+ onLeftClick={handleBackButton}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, The
back-navigation behavior is inconsistent: handleBackClick uses navigate(-1)
while handleBackButton uses navigate('/group') and their names are confusing;
pick the intended behavior (either always go to '/group' or always go back one
step), replace the other handler so both error and normal/loading states call
the same function (e.g., unify into a single handler name like handleBack or
handleNavigateBack), update all onLeftClick usages (including the error-state
onLeftClick) to use that unified handler, and remove the redundant handler to
avoid duplicate implementations.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and canโt be posted inline due to platform limitations.
โ ๏ธ Outside diff range comments (2)
src/pages/memory/Memory.tsx (1)
127-161:โ ๏ธ Potential issue | ๐ก Minor์๋ฌ ๋ฐ์ ์ 500ms ์ต์ ๋ก๋ฉ ์๊ฐ์ด ๋ณด์ฅ๋์ง ์์ต๋๋ค.
Promise.all์ ํ๋์ Promise๊ฐ reject๋๋ฉด ์ฆ์ reject๋ฉ๋๋ค.getMemoryPosts๊ฐ 100ms ๋ง์ ์คํจํ๋ฉดminLoadingTime(500ms)์ ๊ธฐ๋ค๋ฆฌ์ง ์๊ณ ๋ฐ๋ก catch๋ก ์ง์ ํ์ฌ ์ค์ผ๋ ํค์ด ์งง๊ฒ ๊น๋นก์ด๊ฒ ๋ฉ๋๋ค.์ต์ ๋ก๋ฉ ์๊ฐ์ ์๋ฌ ๊ฒฝ๋ก์์๋ ๋ณด์ฅํ๋ ค๋ฉด fetch์ delay๋ฅผ ๋ถ๋ฆฌํด์ ์ฒ๋ฆฌํ์ธ์.
๐ ๏ธ ์์ ์ ์
- const minLoadingTime = new Promise(resolve => setTimeout(resolve, 500)); - const [response] = await Promise.all([getMemoryPosts(params), minLoadingTime]); - + const minLoadingTime = new Promise(resolve => setTimeout(resolve, 500)); + const [result] = await Promise.allSettled([getMemoryPosts(params), minLoadingTime]); + if (result.status === 'rejected') throw result.reason; + const response = result.value;๋๋ ๋ ๊ฐ๋จํ๊ฒ:
- const minLoadingTime = new Promise(resolve => setTimeout(resolve, 500)); - const [response] = await Promise.all([getMemoryPosts(params), minLoadingTime]); + const startTime = Date.now(); + const response = await getMemoryPosts(params); + const elapsed = Date.now() - startTime; + if (elapsed < 500) await new Promise(resolve => setTimeout(resolve, 500 - elapsed));๋ ๋ฒ์งธ ๋ฐฉ์์ ์๋ฌ ์์๋ catch ์ ์ ์ต์ ์๊ฐ์ ๋ณด์ฅํ๋ ค๋ฉด
finally๋ธ๋ก์์ delay๋ฅผ ์ฒ๋ฆฌํด์ผ ํ๋ฏ๋ก,Promise.allSettled๋ฐฉ์์ด ๋ ๊น๋ํฉ๋๋ค.๐ค 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 - 161, The current fetch uses Promise.all([getMemoryPosts(params), minLoadingTime]) so a fast rejection from getMemoryPosts skips the 500ms delay; change the logic to await the fetch and the delay separately (e.g., use Promise.allSettled([getMemoryPosts(params), minLoadingTime]) or await getMemoryPosts(params) and then await minLoadingTime in a finally) so that minLoadingTime is always honored even on error; adjust handling around getMemoryPosts, minLoadingTime, and the existing try/catch/finally blocks so setLoading(false) and UI updates occur after the ensured delay.src/pages/groupSearch/GroupSearch.tsx (1)
50-59:โ ๏ธ Potential issue | ๐ก Minor์ด๊ธฐ ๋ง์ดํธ ์ ์ต๊ทผ ๊ฒ์ ์์ฒญ์ด ์ค๋ณต ํธ์ถ๋ฉ๋๋ค.
Line 51์ ๋ง์ดํธ ์ ์ฉ effect์ Line 57์
searchStatus === 'idle'effect๊ฐ ์ฒซ ๋ ๋์์ ๋ชจ๋ ์คํ๋์ด ๋์ผ API๋ฅผ 2ํ ํธ์ถํฉ๋๋ค.๐ง ์ ์ ์์ ์
- useEffect(() => { - fetchRecentSearches(); - // eslint-disable-next-line react-hooks/exhaustive-deps - }, []);๐ค 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, The component calls fetchRecentSearches twice on first render because there's a mount-only useEffect and another useEffect that calls fetchRecentSearches when searchStatus === 'idle'; remove the duplicate by either deleting the mount-only effect or preventing the second effect from running on initial mount (e.g., add a hasFetchedRef or guard so the useEffect watching searchStatus only calls fetchRecentSearches when searchStatus becomes 'idle' after mount). Update code references: fetchRecentSearches, searchStatus, and the two useEffect blocks so only one path triggers the API call on initial render.
โป๏ธ Duplicate comments (2)
src/pages/mypage/Mypage.tsx (1)
108-119:โ ๏ธ Potential issue | ๐ก Minor์ค์ผ๋ ํค ๋ฌด๊ธฐํ ๋ ธ์ถ ๋ฒ๊ทธ ์์ ํ์ธ.
loading๊ณผ!profile์ ๋ถ๋ฆฌํ์ฌ ํ๋กํ ๋ก๋ ์คํจ ์์๋ ์ค์ผ๋ ํค์ด ์ฌ๋ผ์ง๊ฒ ๋ ์ ์ ์ฌ๋ฐ๋ฅธ ์์ ์ ๋๋ค.๋ค๋ง,
!profile ? null๋ถ๊ธฐ์์ ์ฌ์ฉ์์๊ฒ ์๋ฌด๋ฐ ํผ๋๋ฐฑ์ด ์ ๊ณต๋์ง ์์ต๋๋ค. ํ๋กํ ๋ก๋ ์คํจ ์ ๋น ํ๋ฉด ๋์ ๊ฐ๋จํ ์๋ฌ ๋ฉ์์ง๋ฅผ ํ์ํ๋ ๊ฒ์ ๊ถ์ฅํฉ๋๋ค.๐ก fallback UI ์ ์
- ) : !profile ? null : ( + ) : !profile ? ( + <UserProfile> + <div className="userInfo"> + <div className="user"> + <div className="username">ํ๋กํ์ ๋ถ๋ฌ์ค์ง ๋ชปํ์ด์</div> + </div> + </div> + </UserProfile> + ) : (๐ค 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 108 - 119, The current conditional rendering removes the skeleton when loading is false but returns null on the !profile branch, leaving users with no feedback on load failure; update the conditional in Mypage.tsx so that when loading is false and profile is falsy you render a simple fallback/error UI (e.g., a ProfileErrorContainer or plain message like "Failed to load profile. Retry.") instead of null; keep the loading branch using ProfileSkeletonContainer/ProfileSkeletonLeft/ProfileSkeletonText and ensure the new fallback replaces the !profile ? null branch so users see an error state and can act.src/pages/memory/Memory.tsx (1)
318-342:inert์์ฑ ์ ์ฉ โ ์ด์ ๋ฆฌ๋ทฐ ํผ๋๋ฐฑ์ด ์ฌ๋ฐ๋ฅด๊ฒ ๋ฐ์๋์์ต๋๋ค.
FixedSection์inert์์ฑ์ ์ถ๊ฐํ์ฌ ๋ก๋ฉ ์ค ํค๋ณด๋/ํฌ์ปค์ค/ํด๋ฆญ ์ํธ์์ฉ์ ์์ ํ ์ฐจ๋จํ ๊ฒ์ ์ด์ ๋ฆฌ๋ทฐ์์ ์ ์๋ ๋ด์ฉ๊ณผ ์ผ์นํฉ๋๋ค.๐ค 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 318 - 342, The "inert" application on FixedSection during loading is correct, but the diffs include an extraneous review marker [duplicate_comment]; remove that stray marker and ensure the JSX uses the inert prop on FixedSection only while loading (keep FixedSection inert as shown โ inert or inert={true} is acceptable) so that keyboard/focus/click interactions remain blocked; verify no other instances of the duplicate marker remain.
๐งน Nitpick comments (1)
src/pages/memory/Memory.tsx (1)
91-91: ํญ/ํํฐ ๋ณ๊ฒฝ ์์๋ ์ ์ฒด ์ค์ผ๋ ํค์ด ํ์๋ฉ๋๋ค โ ์๋๋ ๋์์ธ์ง ํ์ธํ์ธ์.
loading์ดtrue๋ก ์ด๊ธฐํ๋๊ณ ,loadMemoryPostsํธ์ถ ์๋ง๋คsetLoading(true)๋ก ์ค์ ๋๋ฏ๋ก ํญ ์ ํ์ด๋ ํํฐ ๋ณ๊ฒฝ ์์๋ ์ ์ฒด ์ฝํ ์ธ ๊ฐ ์ค์ผ๋ ํค์ผ๋ก ๋์ฒด๋ฉ๋๋ค. ์ด๋ฏธ ๋ฐ์ดํฐ๊ฐ ํ์๋ ์ํ์์์ ์ ํ์ด๋ผ๋ฉด ์ฝํ ์ธ ๊น๋นก์์ด ๋ฐ์ํ ์ ์์ต๋๋ค.์๋๋ ๋์์ธ์ด๋ผ๋ฉด ๋ฌด์ํ์ ๋ ๋ฉ๋๋ค. ๋ง์ฝ ์๋๋ผ๋ฉด, ์ด๊ธฐ ๋ก๋ฉ๊ณผ ์ดํ ์ฌ๋ก๋ฉ์ ๊ตฌ๋ถํ์ฌ(
isInitialLoad๋ฑ) ์ฌ๋ก๋ฉ ์์๋ ๊ธฐ์กด ์ฝํ ์ธ ์์ ๊ฐ๋ฒผ์ด ๋ก๋ฉ ์ธ๋์ผ์ดํฐ๋ฅผ ๋ณด์ฌ์ฃผ๋ ๋ฐฉ์๋ ๊ณ ๋ คํด ๋ณผ ์ ์์ต๋๋ค.Also applies to: 107-107
๐ค Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/memory/Memory.tsx` at line 91, The current behavior sets loading = true on every loadMemoryPosts call (useState( true ) + setLoading(true)), causing full skeleton replacement on tab/filter changes; update the logic to distinguish initial load vs subsequent reloads (e.g., add an isInitialLoad state or useRef) and only set loading=true for the initial fetch in the Memory component, while for subsequent calls show a lighter inline spinner or set a separate isRefetching flag so existing content remains visible during tab/filter changes; adjust references to loading, setLoading and loadMemoryPosts accordingly to use the new initial/refetch flags.
๐ค 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/groupSearch/GroupSearch.tsx`:
- Around line 64-66: The current Promise.all usage with minLoadingTime allows
early rejection and skips the 500ms delay on failures; change both places to use
Promise.allSettled (or otherwise ensure minLoadingTime is always awaited). For
the getRecentSearch flow, replace Promise.all([getRecentSearch('ROOM'),
minLoadingTime]) with Promise.allSettled([getRecentSearch('ROOM'),
minLoadingTime]) and then check the settled result for the getRecentSearch entry
(status === 'fulfilled' and value.isSuccess) before calling setRecentSearches,
defaulting to [] on failure; remove the later separate await minLoadingTime if
present. Do the same for the getSearchRooms call: include minLoadingTime in
Promise.allSettled([getSearchRooms(...), minLoadingTime]) and extract the rooms
only if the getSearchRooms settled entry succeeded, otherwise handle the error
path after the delay so success and failure both respect the 500ms minimum.
---
Outside diff comments:
In `@src/pages/groupSearch/GroupSearch.tsx`:
- Around line 50-59: The component calls fetchRecentSearches twice on first
render because there's a mount-only useEffect and another useEffect that calls
fetchRecentSearches when searchStatus === 'idle'; remove the duplicate by either
deleting the mount-only effect or preventing the second effect from running on
initial mount (e.g., add a hasFetchedRef or guard so the useEffect watching
searchStatus only calls fetchRecentSearches when searchStatus becomes 'idle'
after mount). Update code references: fetchRecentSearches, searchStatus, and the
two useEffect blocks so only one path triggers the API call on initial render.
In `@src/pages/memory/Memory.tsx`:
- Around line 127-161: The current fetch uses
Promise.all([getMemoryPosts(params), minLoadingTime]) so a fast rejection from
getMemoryPosts skips the 500ms delay; change the logic to await the fetch and
the delay separately (e.g., use Promise.allSettled([getMemoryPosts(params),
minLoadingTime]) or await getMemoryPosts(params) and then await minLoadingTime
in a finally) so that minLoadingTime is always honored even on error; adjust
handling around getMemoryPosts, minLoadingTime, and the existing
try/catch/finally blocks so setLoading(false) and UI updates occur after the
ensured delay.
---
Duplicate comments:
In `@src/pages/memory/Memory.tsx`:
- Around line 318-342: The "inert" application on FixedSection during loading is
correct, but the diffs include an extraneous review marker [duplicate_comment];
remove that stray marker and ensure the JSX uses the inert prop on FixedSection
only while loading (keep FixedSection inert as shown โ inert or inert={true} is
acceptable) so that keyboard/focus/click interactions remain blocked; verify no
other instances of the duplicate marker remain.
In `@src/pages/mypage/Mypage.tsx`:
- Around line 108-119: The current conditional rendering removes the skeleton
when loading is false but returns null on the !profile branch, leaving users
with no feedback on load failure; update the conditional in Mypage.tsx so that
when loading is false and profile is falsy you render a simple fallback/error UI
(e.g., a ProfileErrorContainer or plain message like "Failed to load profile.
Retry.") instead of null; keep the loading branch using
ProfileSkeletonContainer/ProfileSkeletonLeft/ProfileSkeletonText and ensure the
new fallback replaces the !profile ? null branch so users see an error state and
can act.
---
Nitpick comments:
In `@src/pages/memory/Memory.tsx`:
- Line 91: The current behavior sets loading = true on every loadMemoryPosts
call (useState( true ) + setLoading(true)), causing full skeleton replacement on
tab/filter changes; update the logic to distinguish initial load vs subsequent
reloads (e.g., add an isInitialLoad state or useRef) and only set loading=true
for the initial fetch in the Memory component, while for subsequent calls show a
lighter inline spinner or set a separate isRefetching flag so existing content
remains visible during tab/filter changes; adjust references to loading,
setLoading and loadMemoryPosts accordingly to use the new initial/refetch flags.
โน๏ธ 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 (3)
src/pages/groupSearch/GroupSearch.tsxsrc/pages/memory/Memory.tsxsrc/pages/mypage/Mypage.tsx
| const minLoadingTime = new Promise(resolve => setTimeout(resolve, 500)); | ||
| const [response] = await Promise.all([getRecentSearch('ROOM'), minLoadingTime]); | ||
| setRecentSearches(response.isSuccess ? response.data.recentSearchList : []); |
There was a problem hiding this comment.
๐งฉ Analysis chain
๐ Script executed:
fd 'GroupSearch.tsx' --type fRepository: THIP-TextHip/THIP-Web
Length of output: 102
๐ Script executed:
cat -n src/pages/groupSearch/GroupSearch.tsx | sed -n '50,110p'Repository: THIP-TextHip/THIP-Web
Length of output: 2189
๐ Script executed:
cat -n src/pages/groupSearch/GroupSearch.tsx | sed -n '105,125p'Repository: THIP-TextHip/THIP-Web
Length of output: 709
์คํจ ๊ฒฝ๋ก์์ 500ms ์ต์ ๋ก๋ฉ ์๊ฐ์ด ๋ณด์ฅ๋์ง ์์ต๋๋ค.
Line 65์์ Promise.all([getRecentSearch('ROOM'), minLoadingTime])์ ์ด๋ ํ Promise๊ฐ reject๋๋ฉด ์ฆ์ ์คํจํฉ๋๋ค. ์์ฒญ์ด ์คํจํ๋ฉด minLoadingTime์ด ์๋ฃ๋์ง ์์ ์ํ์์ catch ๋ธ๋ก(67-68)์ผ๋ก ๋น ์ ธ ์ต์ ๋ก๋ฉ ์๊ฐ์ ๋ณด์ฅํ์ง ๋ชปํฉ๋๋ค.
Line 94-104๋ ๋์ผํ ๋ฌธ์ ๊ฐ ์์ต๋๋ค. Promise.all([getSearchRooms(...)])์๋ minLoadingTime์ด ํฌํจ๋์ง ์์, ์์ฒญ์ด ์คํจํ๋ฉด catch ๋ธ๋ก(114-115)์ผ๋ก ์ฆ์ ์ด๋ํ์ฌ 104๋ฒ ์ค์ await minLoadingTime์ ๋๋ฌํ์ง ์์ต๋๋ค.
๊ฒฐ๊ณผ์ ์ผ๋ก ์ฑ๊ณต ์ผ์ด์ค์ ์คํจ ์ผ์ด์ค์ ๋ก๋ฉ UX๊ฐ ๋ถ์ผ์นํฉ๋๋ค.
๐ง ์ ์ ์์ ์
const fetchRecentSearches = async () => {
try {
setIsLoadingRecentSearches(true);
const minLoadingTime = new Promise(resolve => setTimeout(resolve, 500));
- const [response] = await Promise.all([getRecentSearch('ROOM'), minLoadingTime]);
- setRecentSearches(response.isSuccess ? response.data.recentSearchList : []);
+ const request = getRecentSearch('ROOM')
+ .then(response => ({ ok: true as const, response }))
+ .catch(() => ({ ok: false as const }));
+ const [result] = await Promise.all([request, minLoadingTime]);
+ setRecentSearches(
+ result.ok && result.response.isSuccess ? result.response.data.recentSearchList : [],
+ );
} catch {
setRecentSearches([]);
} finally {
setIsLoadingRecentSearches(false);
}
}; try {
const isFinalized = status === 'searched';
const minLoadingTime = new Promise(resolve => setTimeout(resolve, 500));
- const [res] = await Promise.all([
- getSearchRooms(
- term.trim(),
- sortKey,
- undefined,
- isFinalized,
- categoryParam,
- isAllCategory,
- ),
- ]);
- await minLoadingTime;
+ const request = getSearchRooms(
+ term.trim(),
+ sortKey,
+ undefined,
+ isFinalized,
+ categoryParam,
+ isAllCategory,
+ )
+ .then(res => ({ ok: true as const, res }))
+ .catch(error => ({ ok: false as const, error }));
+ const [result] = await Promise.all([request, minLoadingTime]);
+ if (!result.ok) throw result.error;
+ const res = result.res;
if (res.isSuccess) {
const { roomList, nextCursor: nc, isLast: last } = res.data;๐ 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 minLoadingTime = new Promise(resolve => setTimeout(resolve, 500)); | |
| const [response] = await Promise.all([getRecentSearch('ROOM'), minLoadingTime]); | |
| setRecentSearches(response.isSuccess ? response.data.recentSearchList : []); | |
| const minLoadingTime = new Promise(resolve => setTimeout(resolve, 500)); | |
| const request = getRecentSearch('ROOM') | |
| .then(response => ({ ok: true as const, response })) | |
| .catch(() => ({ ok: false as const })); | |
| const [result] = await Promise.all([request, minLoadingTime]); | |
| setRecentSearches( | |
| result.ok && result.response.isSuccess ? result.response.data.recentSearchList : [], | |
| ); |
๐ค 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 64 - 66, The current
Promise.all usage with minLoadingTime allows early rejection and skips the 500ms
delay on failures; change both places to use Promise.allSettled (or otherwise
ensure minLoadingTime is always awaited). For the getRecentSearch flow, replace
Promise.all([getRecentSearch('ROOM'), minLoadingTime]) with
Promise.allSettled([getRecentSearch('ROOM'), minLoadingTime]) and then check the
settled result for the getRecentSearch entry (status === 'fulfilled' and
value.isSuccess) before calling setRecentSearches, defaulting to [] on failure;
remove the later separate await minLoadingTime if present. Do the same for the
getSearchRooms call: include minLoadingTime in
Promise.allSettled([getSearchRooms(...), minLoadingTime]) and extract the rooms
only if the getSearchRooms settled entry succeeded, otherwise handle the error
path after the delay so success and failure both respect the 500ms minimum.
์ดํ๋์ผ
Summary by CodeRabbit
์๋ก์ด ๊ธฐ๋ฅ
์ฌ์ฉ์ ๊ฒฝํ ๊ฐ์
๋ฌธ์ํ