fix: 발견 탭 필터 칩 전체 표시 및 모달 북마크 API 연동#99
Open
ballsona wants to merge 5 commits into
Open
Conversation
- Discovery DTO 타입 및 API 함수/쿼리 훅 추가 (피드, 검색) - 스크랩 PUT/DELETE API 및 optimistic update 뮤테이션 훅 추가 - 75% 스크롤 임계치 기반 무한 스크롤 훅 추가 - DiscoverFeed, DiscoverSearchResults 실API 연동 완료
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughDiscover 탭의 피드·검색 위젯을 MOCK 데이터에서 실제 API 기반으로 전환했다. Discovery DTO 타입, API 함수, TanStack 무한 쿼리 훅, 낙관적 업데이트를 포함한 스크랩 뮤테이션, 공용 ChangesDiscovery 탭 API 연동 및 무한 스크롤
Sequence Diagram(s)sequenceDiagram
actor User
participant Widget as DiscoverFeed / DiscoverSearchResults
participant InfiniteScroll as useInfiniteScroll
participant Query as useDiscoveryFeedQuery / useDiscoverySearchQuery
participant ScrapMutation as useScrapMutation
participant Server as Discovery API Server
User->>Widget: 탭 진입 / 검색어 입력
Widget->>Query: genre / query, sort 전달
Query->>Server: GET /discovery/quotes[/search]?cursor=&genre=
Server-->>Query: DiscoveryQuoteListResponse (items, nextCursor, hasNext)
Query-->>Widget: quotes 배열 렌더링
User->>InfiniteScroll: 스크롤 하단 도달 (threshold 초과)
InfiniteScroll->>Widget: onLoad() 콜백 호출
Widget->>Query: fetchNextPage()
Query->>Server: GET /discovery/quotes?cursor={nextCursor}
Server-->>Query: 다음 페이지 데이터
Query-->>Widget: 추가 quotes 평탄화 렌더링
User->>Widget: 북마크 버튼 클릭
Widget->>ScrapMutation: toggle(quoteId, currentIsScrapped)
ScrapMutation->>Query: cancelQueries + 낙관적 isScrapped 패치
ScrapMutation->>Server: PUT or DELETE /quotes/{quoteId}/scrap
alt 성공
Server-->>ScrapMutation: 200 OK
else 실패
ScrapMutation->>Query: 이전 캐시 롤백
end
ScrapMutation->>Query: invalidateQueries(discoveryKeys.all)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/features/discover-filter/ui/GenreFilterChips.tsx (1)
14-25:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win필터 칩 선택 상태를 접근성 속성으로 노출해주세요.
현재 선택 여부가 시각 스타일에만 반영됩니다. 토글 버튼 패턴이므로
aria-pressed를 함께 넣는 것이 좋습니다.수정 제안
<button key={genre} type="button" onClick={() => onChange(genre)} + aria-pressed={selected === genre} className={`caption1 shrink-0 whitespace-nowrap rounded-full px-3 py-1 ${ selected === genre ? "bg-gray-700 text-white" : "border border-gray-200 bg-background text-gray-500" }`} >🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/features/discover-filter/ui/GenreFilterChips.tsx` around lines 14 - 25, The button element in the GenreFilterChips component lacks accessibility attributes to communicate its selection state to assistive technologies. Add the aria-pressed attribute to the button element, setting its value to true when selected === genre and false otherwise, so that screen readers and other assistive tools can properly convey the toggle button's current state to users.
🧹 Nitpick comments (3)
src/features/post-bookmark/model/useScrapMutation.ts (1)
25-25: ⚡ Quick win약어 변수명
q는 가독성과 가이드라인 일관성을 깨뜨립니다.의미가 명확한 이름으로 바꿔 주세요.
✏️ 제안 수정
- quotes: page.quotes.map((q) => (q.quoteId === quoteId ? { ...q, isScrapped } : q)), + quotes: page.quotes.map((quote) => + quote.quoteId === quoteId ? { ...quote, isScrapped } : quote, + ),As per coding guidelines, "Avoid abbreviations in variable and function names (e.g., use
buttoninstead ofbtn)."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/features/post-bookmark/model/useScrapMutation.ts` at line 25, The variable name `q` used as the parameter in the map function is an abbreviation that violates coding guidelines and reduces readability. Replace the abbreviated parameter name `q` with a more descriptive name such as `quote` throughout the arrow function in the quotes map operation. This means updating the parameter itself and all references to `q` within the function body (including `q.quoteId` and the spread operator `...q`) to use the new descriptive variable name.Source: Coding guidelines
src/shared/hooks/useInfiniteScroll.ts (1)
11-11: ⚡ Quick win
el대신 의미 있는 변수명을 사용해 주세요.As per coding guidelines, "Avoid abbreviations in variable and function names (e.g., use
buttoninstead ofbtn)."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/shared/hooks/useInfiniteScroll.ts` at line 11, The variable name `el` in the useInfiniteScroll hook is an abbreviation that violates the coding guidelines requiring full, meaningful variable names. Rename the `el` variable to a more descriptive name such as `element` or `scrollElement` (choose based on the actual purpose of the variable in the context where it's used) to improve code readability and consistency with the guidelines that recommend avoiding abbreviations like `el` in favor of complete names.Source: Coding guidelines
src/widgets/discover-feed/ui/DiscoverFeed.tsx (1)
16-31: ⚡ Quick win시간/무드 포맷터가 검색 위젯과 중복되어 유지보수 포인트가 분산됩니다.
동일 로직(
getMood,formatRelativeTime)이DiscoverSearchResults에도 복제되어 있어 기준 변경 시 쉽게 드리프트가 납니다. 공용 유틸로 추출해 단일 소스로 관리하는 편이 안전합니다.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/widgets/discover-feed/ui/DiscoverFeed.tsx` around lines 16 - 31, The functions getMood and formatRelativeTime are duplicated in both DiscoverFeed.tsx and DiscoverSearchResults, creating a maintenance risk where logic changes must be applied in multiple locations. Extract both getMood and formatRelativeTime functions into a shared utility module in the appropriate location (such as src/widgets/common/utils or src/utils), then import and use these functions from the utility in both DiscoverFeed.tsx and DiscoverSearchResults instead of keeping local copies. This ensures a single source of truth for the formatting logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/entities/post/api/postQueries.ts`:
- Around line 47-65: The enabled condition in the useDiscoverySearchQuery
function currently checks only if query.length > 0, which allows whitespace-only
strings to trigger unnecessary API calls. Modify the enabled condition to use
query.trim().length > 0 instead to filter out whitespace-only queries.
Additionally, consider trimming the query parameter when passing it to
fetchDiscoveryQuotesSearch to ensure consistency and prevent sending empty or
whitespace-only search terms to the backend API.
In `@src/features/post-bookmark/model/useScrapMutation.ts`:
- Around line 62-70: The catch block in useScrapMutation rolls back the query
data by restoring previousFeed and previousSearch, but does not re-throw the
error after rollback. This causes the mutation to appear successful to the
caller even though it failed, preventing the caller from detecting the failure
or performing recovery actions like UI re-synchronization or displaying error
toasts. After the rollback loops that restore previousFeed and previousSearch,
add a throw statement to re-propagate the error to the caller so they can handle
the failure appropriately.
In `@src/shared/hooks/useInfiniteScroll.ts`:
- Around line 10-23: The useInfiniteScroll hook only calls stableOnLoad when a
scroll event occurs, which means if the initial content is too short to fill the
container, no scrolling is possible and the next page is never loaded. After the
addEventListener call in the useEffect hook, immediately invoke the handleScroll
function once to check the initial scroll position and trigger onLoad if needed,
ensuring that content loads even when the initial page doesn't fill the
viewport.
In `@src/widgets/discover-feed/ui/DiscoverFeed.tsx`:
- Around line 39-40: The useDiscoveryFeedQuery hook call is missing the isError
property in its destructuring, which prevents the component from distinguishing
between network/server errors and genuinely empty results. Add isError to the
destructured properties from useDiscoveryFeedQuery and then update the rendering
logic in the section around lines 63-103 to check for isError first and display
an appropriate error message to users, before falling through to the empty state
message. This ensures users see meaningful feedback about actual failures
instead of confusing "empty results" messages when the query encounters errors.
In `@src/widgets/discover-search-results/ui/DiscoverSearchResults.tsx`:
- Around line 50-52: The useDiscoverySearchQuery hook call in the
DiscoverSearchResults component is missing error state handling, causing search
failures to be displayed as empty results instead of showing a distinct error
UI. Add isError to the destructured values from useDiscoverySearchQuery, then
add a separate conditional branch to render error UI when isError is true, and
ensure the empty state display (referenced in the render logic around lines
108-148) only shows when the query is successful (not loading and not in error
state) but returns no data.
In `@src/widgets/post-share-modal/ui/PostShareModal.tsx`:
- Around line 11-12: The onToggleBookmark callback has a void return type which
prevents error handling and causes the local bookmark state to be toggled
immediately without waiting for the API operation to complete or fail. Change
the onToggleBookmark callback signature to return a Promise (async callback)
instead of void, then update all places where this callback is invoked
(including the calls in the locations around lines 30-33 and 67-71) to await the
promise before toggling the local bookmark state. This ensures the UI state only
updates after the API call succeeds, preventing the state from becoming out of
sync when the operation fails.
---
Outside diff comments:
In `@src/features/discover-filter/ui/GenreFilterChips.tsx`:
- Around line 14-25: The button element in the GenreFilterChips component lacks
accessibility attributes to communicate its selection state to assistive
technologies. Add the aria-pressed attribute to the button element, setting its
value to true when selected === genre and false otherwise, so that screen
readers and other assistive tools can properly convey the toggle button's
current state to users.
---
Nitpick comments:
In `@src/features/post-bookmark/model/useScrapMutation.ts`:
- Line 25: The variable name `q` used as the parameter in the map function is an
abbreviation that violates coding guidelines and reduces readability. Replace
the abbreviated parameter name `q` with a more descriptive name such as `quote`
throughout the arrow function in the quotes map operation. This means updating
the parameter itself and all references to `q` within the function body
(including `q.quoteId` and the spread operator `...q`) to use the new
descriptive variable name.
In `@src/shared/hooks/useInfiniteScroll.ts`:
- Line 11: The variable name `el` in the useInfiniteScroll hook is an
abbreviation that violates the coding guidelines requiring full, meaningful
variable names. Rename the `el` variable to a more descriptive name such as
`element` or `scrollElement` (choose based on the actual purpose of the variable
in the context where it's used) to improve code readability and consistency with
the guidelines that recommend avoiding abbreviations like `el` in favor of
complete names.
In `@src/widgets/discover-feed/ui/DiscoverFeed.tsx`:
- Around line 16-31: The functions getMood and formatRelativeTime are duplicated
in both DiscoverFeed.tsx and DiscoverSearchResults, creating a maintenance risk
where logic changes must be applied in multiple locations. Extract both getMood
and formatRelativeTime functions into a shared utility module in the appropriate
location (such as src/widgets/common/utils or src/utils), then import and use
these functions from the utility in both DiscoverFeed.tsx and
DiscoverSearchResults instead of keeping local copies. This ensures a single
source of truth for the formatting logic.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 5499661a-700d-4aaf-978a-6ee2a16e8ba1
⛔ Files ignored due to path filters (3)
public/images/favicon.svgis excluded by!**/*.svg,!**/*.svg,!**/public/**public/images/og_image.pngis excluded by!**/*.png,!**/*.png,!**/public/**src/app/favicon.icois excluded by!**/*.ico
📒 Files selected for processing (19)
docs/plans/discover-tab-api.mdsrc/app/layout.tsxsrc/entities/post/api/postApi.tssrc/entities/post/api/postQueries.tssrc/entities/post/index.tssrc/entities/post/model/post.types.tssrc/entities/post/ui/PostListItem.tsxsrc/features/discover-filter/model/useDiscoverFilter.tssrc/features/discover-filter/ui/GenreFilterChips.stories.tsxsrc/features/discover-filter/ui/GenreFilterChips.tsxsrc/features/discover-filter/ui/GenreFilterDropdown.stories.tsxsrc/features/post-bookmark/api/scrapApi.tssrc/features/post-bookmark/index.tssrc/features/post-bookmark/model/useScrapMutation.tssrc/shared/hooks/useInfiniteScroll.tssrc/widgets/discover-feed/ui/DiscoverFeed.tsxsrc/widgets/discover-search-results/ui/DiscoverSearchResults.tsxsrc/widgets/nav-bar/ui/NavBar.tsxsrc/widgets/post-share-modal/ui/PostShareModal.tsx
Comment on lines
+47
to
+65
| export const useDiscoverySearchQuery = ( | ||
| query: string, | ||
| sort: "latest" | "scrap" = "latest", | ||
| genre?: string, | ||
| ) => | ||
| useInfiniteQuery({ | ||
| queryKey: discoveryKeys.search(query, sort, genre), | ||
| queryFn: ({ pageParam }) => | ||
| fetchDiscoveryQuotesSearch({ | ||
| query, | ||
| sort, | ||
| cursor: pageParam as string | undefined, | ||
| genre, | ||
| }), | ||
| initialPageParam: undefined as string | undefined, | ||
| getNextPageParam: (lastPage) => | ||
| lastPage.hasNext ? (lastPage.nextCursor ?? undefined) : undefined, | ||
| enabled: query.length > 0, | ||
| staleTime: 30 * 1000, |
There was a problem hiding this comment.
공백-only 검색어가 불필요한 API 호출을 유발합니다.
현재 enabled: query.length > 0 조건이라 공백 문자열도 요청이 나갑니다. trim() 기준으로 활성화/전달을 맞추는 게 안전합니다.
수정 제안
export const useDiscoverySearchQuery = (
query: string,
sort: "latest" | "scrap" = "latest",
genre?: string,
) =>
+ {
+ const normalizedQuery = query.trim();
+ return useInfiniteQuery({
+ queryKey: discoveryKeys.search(normalizedQuery, sort, genre),
+ queryFn: ({ pageParam }) =>
+ fetchDiscoveryQuotesSearch({
+ query: normalizedQuery,
+ sort,
+ cursor: pageParam as string | undefined,
+ genre,
+ }),
+ initialPageParam: undefined as string | undefined,
+ getNextPageParam: (lastPage) =>
+ lastPage.hasNext ? (lastPage.nextCursor ?? undefined) : undefined,
+ enabled: normalizedQuery.length > 0,
+ staleTime: 30 * 1000,
+ });
+ };
- useInfiniteQuery({
- queryKey: discoveryKeys.search(query, sort, genre),
- queryFn: ({ pageParam }) =>
- fetchDiscoveryQuotesSearch({
- query,
- sort,
- cursor: pageParam as string | undefined,
- genre,
- }),
- initialPageParam: undefined as string | undefined,
- getNextPageParam: (lastPage) =>
- lastPage.hasNext ? (lastPage.nextCursor ?? undefined) : undefined,
- enabled: query.length > 0,
- staleTime: 30 * 1000,
- });📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| export const useDiscoverySearchQuery = ( | |
| query: string, | |
| sort: "latest" | "scrap" = "latest", | |
| genre?: string, | |
| ) => | |
| useInfiniteQuery({ | |
| queryKey: discoveryKeys.search(query, sort, genre), | |
| queryFn: ({ pageParam }) => | |
| fetchDiscoveryQuotesSearch({ | |
| query, | |
| sort, | |
| cursor: pageParam as string | undefined, | |
| genre, | |
| }), | |
| initialPageParam: undefined as string | undefined, | |
| getNextPageParam: (lastPage) => | |
| lastPage.hasNext ? (lastPage.nextCursor ?? undefined) : undefined, | |
| enabled: query.length > 0, | |
| staleTime: 30 * 1000, | |
| export const useDiscoverySearchQuery = ( | |
| query: string, | |
| sort: "latest" | "scrap" = "latest", | |
| genre?: string, | |
| ) => { | |
| const normalizedQuery = query.trim(); | |
| return useInfiniteQuery({ | |
| queryKey: discoveryKeys.search(normalizedQuery, sort, genre), | |
| queryFn: ({ pageParam }) => | |
| fetchDiscoveryQuotesSearch({ | |
| query: normalizedQuery, | |
| sort, | |
| cursor: pageParam as string | undefined, | |
| genre, | |
| }), | |
| initialPageParam: undefined as string | undefined, | |
| getNextPageParam: (lastPage) => | |
| lastPage.hasNext ? (lastPage.nextCursor ?? undefined) : undefined, | |
| enabled: normalizedQuery.length > 0, | |
| staleTime: 30 * 1000, | |
| }); | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/entities/post/api/postQueries.ts` around lines 47 - 65, The enabled
condition in the useDiscoverySearchQuery function currently checks only if
query.length > 0, which allows whitespace-only strings to trigger unnecessary
API calls. Modify the enabled condition to use query.trim().length > 0 instead
to filter out whitespace-only queries. Additionally, consider trimming the query
parameter when passing it to fetchDiscoveryQuotesSearch to ensure consistency
and prevent sending empty or whitespace-only search terms to the backend API.
Comment on lines
+62
to
+70
| } catch (error) { | ||
| for (const [key, value] of previousFeed) { | ||
| queryClient.setQueryData(key, value); | ||
| } | ||
| for (const [key, value] of previousSearch) { | ||
| queryClient.setQueryData(key, value); | ||
| } | ||
| console.error("스크랩 처리 실패, 롤백됨:", error); | ||
| } finally { |
There was a problem hiding this comment.
실패를 삼키면 호출자 상태가 성공으로 고정됩니다.
현재 롤백 후 에러를 다시 던지지 않아, 호출부가 실패를 감지하거나 후속 복구(UI 재동기화/토스트)를 할 수 없습니다. 롤백 뒤 throw로 전파해 주세요.
🔧 제안 수정
} catch (error) {
for (const [key, value] of previousFeed) {
queryClient.setQueryData(key, value);
}
for (const [key, value] of previousSearch) {
queryClient.setQueryData(key, value);
}
console.error("스크랩 처리 실패, 롤백됨:", error);
+ throw error;
} finally {
queryClient.invalidateQueries({ queryKey: discoveryKeys.all });
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| } catch (error) { | |
| for (const [key, value] of previousFeed) { | |
| queryClient.setQueryData(key, value); | |
| } | |
| for (const [key, value] of previousSearch) { | |
| queryClient.setQueryData(key, value); | |
| } | |
| console.error("스크랩 처리 실패, 롤백됨:", error); | |
| } finally { | |
| } catch (error) { | |
| for (const [key, value] of previousFeed) { | |
| queryClient.setQueryData(key, value); | |
| } | |
| for (const [key, value] of previousSearch) { | |
| queryClient.setQueryData(key, value); | |
| } | |
| console.error("스크랩 처리 실패, 롤백됨:", error); | |
| throw error; | |
| } finally { |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/features/post-bookmark/model/useScrapMutation.ts` around lines 62 - 70,
The catch block in useScrapMutation rolls back the query data by restoring
previousFeed and previousSearch, but does not re-throw the error after rollback.
This causes the mutation to appear successful to the caller even though it
failed, preventing the caller from detecting the failure or performing recovery
actions like UI re-synchronization or displaying error toasts. After the
rollback loops that restore previousFeed and previousSearch, add a throw
statement to re-propagate the error to the caller so they can handle the failure
appropriately.
Comment on lines
+10
to
+23
| useEffect(() => { | ||
| const el = ref.current; | ||
| if (!el) return; | ||
|
|
||
| const handleScroll = () => { | ||
| const { scrollTop, scrollHeight, clientHeight } = el; | ||
| if ((scrollTop + clientHeight) / scrollHeight >= threshold) { | ||
| stableOnLoad(); | ||
| } | ||
| }; | ||
|
|
||
| el.addEventListener("scroll", handleScroll, { passive: true }); | ||
| return () => el.removeEventListener("scroll", handleScroll); | ||
| }, [stableOnLoad, threshold]); |
There was a problem hiding this comment.
초기 콘텐츠가 짧은 경우 다음 페이지 로드가 영구 정지될 수 있습니다.
현재는 scroll 이벤트가 발생해야만 onLoad가 실행됩니다. 첫 페이지가 컨테이너를 채우지 못하면 스크롤 자체가 불가능해 다음 페이지를 못 가져옵니다. 리스너 등록 직후 한 번 체크를 실행해 주세요.
🔧 제안 수정
el.addEventListener("scroll", handleScroll, { passive: true });
+ handleScroll();
return () => el.removeEventListener("scroll", handleScroll);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| useEffect(() => { | |
| const el = ref.current; | |
| if (!el) return; | |
| const handleScroll = () => { | |
| const { scrollTop, scrollHeight, clientHeight } = el; | |
| if ((scrollTop + clientHeight) / scrollHeight >= threshold) { | |
| stableOnLoad(); | |
| } | |
| }; | |
| el.addEventListener("scroll", handleScroll, { passive: true }); | |
| return () => el.removeEventListener("scroll", handleScroll); | |
| }, [stableOnLoad, threshold]); | |
| useEffect(() => { | |
| const el = ref.current; | |
| if (!el) return; | |
| const handleScroll = () => { | |
| const { scrollTop, scrollHeight, clientHeight } = el; | |
| if ((scrollTop + clientHeight) / scrollHeight >= threshold) { | |
| stableOnLoad(); | |
| } | |
| }; | |
| el.addEventListener("scroll", handleScroll, { passive: true }); | |
| handleScroll(); | |
| return () => el.removeEventListener("scroll", handleScroll); | |
| }, [stableOnLoad, threshold]); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/shared/hooks/useInfiniteScroll.ts` around lines 10 - 23, The
useInfiniteScroll hook only calls stableOnLoad when a scroll event occurs, which
means if the initial content is too short to fill the container, no scrolling is
possible and the next page is never loaded. After the addEventListener call in
the useEffect hook, immediately invoke the handleScroll function once to check
the initial scroll position and trigger onLoad if needed, ensuring that content
loads even when the initial page doesn't fill the viewport.
Comment on lines
+39
to
+40
| const { data, fetchNextPage, hasNextPage, isFetchingNextPage, isLoading } = | ||
| useDiscoveryFeedQuery(genre); |
There was a problem hiding this comment.
쿼리 실패가 “빈 결과”로 오인 표시됩니다.
isError 분기가 없어 네트워크/서버 오류 시에도 빈 상태 메시지로 떨어집니다. 에러 상태를 별도로 렌더링해 사용자 혼란을 막아 주세요.
🔧 제안 수정
- const { data, fetchNextPage, hasNextPage, isFetchingNextPage, isLoading } =
+ const { data, fetchNextPage, hasNextPage, isFetchingNextPage, isLoading, isError } =
useDiscoveryFeedQuery(genre);
@@
- {!isLoading &&
+ {!isLoading && isError && (
+ <div className="flex items-center justify-center py-20">
+ <span className="caption1 text-gray-300">문장을 불러오지 못했어요. 잠시 후 다시 시도해 주세요.</span>
+ </div>
+ )}
+
+ {!isLoading && !isError &&
quotes.map((quote) => (
...
))}
@@
- {!isLoading && quotes.length === 0 && (
+ {!isLoading && !isError && quotes.length === 0 && (
<div className="flex items-center justify-center py-20">
<span className="caption1 text-gray-300">아직 추천된 문장이 없어요.</span>
</div>
)}Also applies to: 63-103
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/widgets/discover-feed/ui/DiscoverFeed.tsx` around lines 39 - 40, The
useDiscoveryFeedQuery hook call is missing the isError property in its
destructuring, which prevents the component from distinguishing between
network/server errors and genuinely empty results. Add isError to the
destructured properties from useDiscoveryFeedQuery and then update the rendering
logic in the section around lines 63-103 to check for isError first and display
an appropriate error message to users, before falling through to the empty state
message. This ensures users see meaningful feedback about actual failures
instead of confusing "empty results" messages when the query encounters errors.
Comment on lines
+50
to
+52
| const { data, fetchNextPage, hasNextPage, isFetchingNextPage, isLoading } = | ||
| useDiscoverySearchQuery(initialQuery, activeSort, genre); | ||
|
|
There was a problem hiding this comment.
검색 쿼리 실패가 빈 결과로 표시됩니다.
isError 분기가 없어 실패와 “결과 없음”이 구분되지 않습니다. 에러 UI를 분리하고 빈 상태는 성공 응답일 때만 보여 주세요.
Also applies to: 108-148
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/widgets/discover-search-results/ui/DiscoverSearchResults.tsx` around
lines 50 - 52, The useDiscoverySearchQuery hook call in the
DiscoverSearchResults component is missing error state handling, causing search
failures to be displayed as empty results instead of showing a distinct error
UI. Add isError to the destructured values from useDiscoverySearchQuery, then
add a separate conditional branch to render error UI when isError is true, and
ensure the empty state display (referenced in the render logic around lines
108-148) only shows when the query is successful (not loading and not in error
state) but returns no data.
Comment on lines
+11
to
12
| onToggleBookmark?: (currentIsBookmarked: boolean) => void; | ||
| } |
There was a problem hiding this comment.
북마크 로컬 상태가 API 실패와 분리되어 잘못 고정될 수 있습니다.
콜백 시그니처가 void라 실패 전파를 받을 수 없고, 현재는 호출 직후 로컬 상태를 무조건 토글합니다. 비동기 콜백으로 바꾸고 성공 시에만 토글(또는 실패 시 롤백)해 주세요.
🔧 제안 수정
interface PostShareModalProps {
post: Post;
isOpen: boolean;
onClose: () => void;
- onToggleBookmark?: (currentIsBookmarked: boolean) => void;
+ onToggleBookmark?: (currentIsBookmarked: boolean) => Promise<void>;
}
-const handleToggleBookmark = () => {
- onToggleBookmark?.(isBookmarked);
- setIsBookmarked((prev) => !prev);
+const handleToggleBookmark = async () => {
+ try {
+ await onToggleBookmark?.(isBookmarked);
+ setIsBookmarked((prev) => !prev);
+ } catch {
+ // 실패 알림/복구 처리 지점
+ }
};Also applies to: 30-33, 67-71
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/widgets/post-share-modal/ui/PostShareModal.tsx` around lines 11 - 12, The
onToggleBookmark callback has a void return type which prevents error handling
and causes the local bookmark state to be toggled immediately without waiting
for the API operation to complete or fail. Change the onToggleBookmark callback
signature to return a Promise (async callback) instead of void, then update all
places where this callback is invoked (including the calls in the locations
around lines 30-33 and 67-71) to await the promise before toggling the local
bookmark state. This ensures the UI state only updates after the API call
succeeds, preventing the state from becoming out of sync when the operation
fails.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
📝 작업 내용
📸 스크린샷
📌 PR 중요도
💬 기타 사항