Merged
Conversation
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Owner
Author
|
/describe |
Owner
Author
|
/review |
Owner
Author
|
/improve |
Review Summary by QodoImprove search history UI with fixed height and loading states
WalkthroughsDescription• Add fixed height (h-30) to prevent layout shift in search history components • Display loading spinner during Zustand hydration in SearchHistory • Display loading spinner during API requests in PopularSearchHistory • Fix slice bug: change slice(10) to slice(0, 10) in SearchHistory Diagramflowchart LR
A["SearchHistory Component"] -->|Add hydration state| B["Loading Spinner"]
A -->|Fix slice bug| C["Display first 10 items"]
A -->|Add fixed height| D["Prevent layout shift"]
E["PopularSearchHistory Component"] -->|Add isPending state| F["Loading Spinner"]
E -->|Add fixed height| G["Prevent layout shift"]
File Changes1. apps/web/src/app/search/SearchHistory.tsx
|
Code Review by Qodo
1. isHydrated tied to history length
|
Code Review by QodoNew Review StartedThis review has been superseded by a new analysisⓘ The new review experience is currently in Beta. Learn more |
Code Review by Qodo
1. isHydrated tied to history
|
|
PR Description updated to latest commit (2580aea)
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Comment on lines
+14
to
+18
| useEffect(() => { | ||
| if (searchHistory.length > 0) { | ||
| setIsHydrated(true); | ||
| } | ||
| }, [searchHistory]); |
There was a problem hiding this comment.
1. ishydrated tied to history 📎 Requirement gap ☼ Reliability
SearchHistory sets isHydrated to true only when searchHistory.length > 0, so users with no stored history will see the loading spinner forever even after Zustand persist hydration completes. This makes the hydration transition incorrect/unclear and violates the requirement to show the spinner only until hydration finishes.
Agent Prompt
## Issue description
`SearchHistory` shows a loading spinner until `isHydrated` becomes true, but `isHydrated` is currently set based on `searchHistory.length > 0` instead of Zustand persist hydration completion. If the persisted history is empty, hydration can finish while the spinner never disappears.
## Issue Context
`useSearchHistoryStore` uses `zustand/middleware` `persist`, so hydration completion is independent of whether the hydrated `searchHistory` array has items.
## Fix Focus Areas
- apps/web/src/app/search/SearchHistory.tsx[14-33]
- apps/web/src/stores/useSearchHistoryStore.ts[18-44]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+12
to
+30
| const [isHydrated, setIsHydrated] = useState(false); | ||
|
|
||
| if (searchHistory.length === 0) return null; | ||
| useEffect(() => { | ||
| if (searchHistory.length > 0) { | ||
| setIsHydrated(true); | ||
| } | ||
| }, [searchHistory]); | ||
|
|
||
| return ( | ||
| <div> | ||
| <div className="h-30 overflow-hidden"> | ||
| <div className="flex items-center gap-2"> | ||
| <Clock className="h-4 w-4" /> | ||
| <p className="m-2">최근 검색어</p> | ||
| </div> | ||
| <div className="flex flex-wrap gap-2 pb-4"> | ||
| {searchHistory.slice(10).map((term, index) => ( | ||
| <div | ||
| key={`${term}-${index}`} | ||
| className="bg-background flex items-center gap-2 rounded-full border px-3 py-1.5 text-sm" | ||
| > | ||
| <span | ||
| className="hover:text-primary max-w-30 cursor-pointer truncate text-left" | ||
| onClick={() => onHistoryClick(term)} | ||
| {!isHydrated ? ( | ||
| <div className="flex items-center justify-center py-4"> | ||
| <Loader2 className="h-5 w-5 animate-spin" /> | ||
| </div> | ||
| ) : ( |
There was a problem hiding this comment.
1. ishydrated tied to history length 📎 Requirement gap ≡ Correctness
SearchHistory sets isHydrated to true only when searchHistory.length > 0, so if the persisted store hydrates to an empty array the component can remain stuck showing the spinner forever. This fails the requirement to transition from the pre-hydration spinner to the hydrated UI once Zustand persist hydration completes.
Agent Prompt
## Issue description
`SearchHistory` determines hydration via `searchHistory.length > 0`, which can leave the UI stuck on the loading spinner when hydration completes with an empty list.
## Issue Context
Zustand `persist` provides hydration lifecycle helpers (e.g., `useStore.persist.hasHydrated()` / `useStore.persist.onFinishHydration(...)`) that should be used to track hydration completion independently of the stored data length.
## Fix Focus Areas
- apps/web/src/app/search/SearchHistory.tsx[12-30]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
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.
User description
📌 PR 제목
[Feat] : 검색어 히스토리 컴포넌트 UI 개선
📌 변경 사항
SearchHistory,PopularSearchHistory컴포넌트에 고정 높이(h-30) 적용하여 레이아웃 시프트 방지SearchHistory: Zustand persist 하이드레이션 전 로딩 스피너 표시PopularSearchHistory: API 요청 중(isPending) 로딩 스피너 표시SearchHistory에서slice(10)→slice(0, 10)버그 수정💬 추가 참고 사항
PR Type
Enhancement
Description
Add fixed height (
h-30) to prevent layout shift in search history componentsDisplay loading spinner during Zustand hydration in
SearchHistoryDisplay loading spinner during API requests in
PopularSearchHistoryFix slice bug: change
slice(10)toslice(0, 10)inSearchHistoryDiagram Walkthrough
File Walkthrough
SearchHistory.tsx
Add hydration loading state and fix slice bugapps/web/src/app/search/SearchHistory.tsx
Loader2icon and React hooks for hydration trackingisHydratedstate to detect Zustand store hydration completionhydration
h-30to root containerslice(10)toslice(0, 10)to show first 10 itemsPopularSearchHistory.tsx
Add loading spinner and fixed height containerapps/web/src/app/search/PopularSearchHistory.tsx
Loader2icon for loading indicatorisPendingstate fromuseSearchLogQueryhookh-30to root containerisPendingistrue