Conversation
WalkthroughNew ItemList React component and styles added, fetching items from Changes
Sequence DiagramsequenceDiagram
participant User
participant ItemList as ItemList Component
participant API as API (v2/item/)
participant Mock as Mock Data
User->>ItemList: Mount
activate ItemList
ItemList->>API: GET /v2/item/
activate API
alt API returns 200 with data
API-->>ItemList: items[]
ItemList->>ItemList: map to Item, set items state
else API returns 204 / error / empty
API-->>ItemList: 204 / error / no data
ItemList->>Mock: load mockItems
Mock-->>ItemList: mockItems[]
ItemList->>ItemList: set items state from mock
end
deactivate API
User->>ItemList: select category / enter search
ItemList->>ItemList: filter items by category + searchTerm
ItemList-->>User: render filtered product grid
deactivate ItemList
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (7)
src/components/User/ItemList/index.tsx (3)
27-172: Consider extracting mock data to a separate file.The 146-line mock dataset adds significant size to the component file. Moving it to a separate constants file (e.g.,
mockItems.ts) would improve maintainability and make the component logic easier to read.Example structure:
// mockItems.ts export const mockItems: Item[] = [ // ... mock data ];Then import it:
+import { mockItems } from './mockItems'; + -// 목 데이터 -const mockItems: Item[] = [ - // ... (remove inline data) -];
207-239: Consider improvements to error handling and type safety.A few refinements would strengthen this function:
- Console statements (lines 214, 232): Consider using a proper logging service or removing these before production deployment.
- Type safety (line 218): The
anytype for the mapping function parameter bypasses TypeScript's type checking. Consider defining an API response interface.- User feedback: Errors silently fall back to mock data without informing users. Consider showing a toast notification or status message when the API call fails.
Example for type safety:
interface ApiItem { itemId: number; itemName: string; itemCode: string; itemPrice: number; category?: string; isNew?: boolean; isHot?: boolean; } // Then in the mapping: const remappedData: Item[] = response.data.itemList.map((item: ApiItem) => ({ // ... }));
249-312: LGTM! Clean render logic with minor accessibility consideration.The conditional rendering and grid layout are well-structured. Consider adding accessibility enhancements:
- Add
aria-labelor associate a<label>with the SearchInput (line 267-271)- Ensure CategoryTab buttons are keyboard navigable (already using
<button>, which is good)src/components/User/ItemList/style.ts (4)
3-64: Consider using a theme system for colors and spacing.The hardcoded color values (
#111111,#666666,#F49E15, etc.) and spacing values appear throughout the file. Using a centralized theme or design tokens would improve maintainability and consistency across the application.Example approach:
const theme = { colors: { primary: '#F49E15', textPrimary: '#111111', textSecondary: '#666666', // ... }, // ... }; // Then use: color: ${props => props.theme.colors.textPrimary};
179-188: Review the unusual positioning value.The
right: -0.5pxpositioning (line 181) is unusual and might indicate a workaround for border or gap rendering issues. Consider investigating whether the layout can be adjusted to avoid fractional pixel offsets, which can cause inconsistent rendering across browsers.
147-156: Consider overflow handling for long product names.The
max-width: 273pxon ProductInfo (line 155) might cause long product names to overflow. Consider addingoverflow: hiddenandtext-overflow: ellipsisto ProductTitle, or allowing the container to grow.
190-208: LGTM! Clear state messaging.The loading and empty state components provide appropriate user feedback. As an optional improvement, these could share a common base component since they have identical styling.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/components/User/ItemList/index.tsx(1 hunks)src/components/User/ItemList/style.ts(1 hunks)src/routers/UserRouter.tsx(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/components/User/ItemList/index.tsx (1)
src/utils/Axios.ts (1)
axiosInstance(18-23)
🔇 Additional comments (4)
src/routers/UserRouter.tsx (1)
12-12: LGTM! Clean route integration.The import and route registration follow the established patterns in this router.
Also applies to: 26-26
src/components/User/ItemList/index.tsx (3)
5-13: LGTM! Well-defined interface.The Item interface clearly defines the data structure with appropriate optional fields.
181-197: LGTM! Efficient filtering logic.The filtering implementation correctly handles both category and search term filters with case-insensitive matching. The useCallback dependencies are appropriate.
199-205: LGTM! Proper effect usage.The effects are correctly structured: initial data fetch on mount and filtering when dependencies change. The filterItems dependency in the second effect is safe since it's memoized via useCallback.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/components/User/ItemList/index.tsx (1)
141-144: Consider clarifying badge priority logic.When both
isNewandisHotare true, only the NEW badge displays. If this priority is intentional, consider adding a comment to make it explicit. Alternatively, you could display both badges if that's desired behavior.Current behavior (NEW takes priority):
{(item.isNew || item.isHot) && ( <S.Badge type={item.isNew ? 'new' : 'hot'}> {item.isNew ? 'NEW' : 'HOT'} </S.Badge> )}If you want to show both badges:
{item.isNew && ( <S.Badge type='new'>NEW</S.Badge> )} {item.isHot && !item.isNew && ( <S.Badge type='hot'>HOT</S.Badge> )}Note: Regarding the past review comment asking how NEW/HOT are managed - the code shows these are provided by the backend API with fallback defaults to
false(lines 74-75).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/components/User/ItemList/index.tsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/components/User/ItemList/index.tsx (3)
src/components/Admin/Transaction/ManyCharge/index.jsx (2)
searchTerm(12-12)handleSearchChange(39-41)src/contexts/loadingContext.jsx (1)
setLoading(25-32)src/utils/Axios.ts (1)
axiosInstance(18-23)
🔇 Additional comments (4)
src/components/User/ItemList/index.tsx (4)
1-13: LGTM!The imports and interface definition are clean and appropriate for the component's needs.
33-49: LGTM!The filtering logic is well-implemented with proper memoization and case-insensitive search. The sequential application of category and search filters works correctly.
51-57: LGTM!The useEffect hooks are correctly structured. The mount effect handles initial data fetch, and the filter effect properly responds to changes in dependencies through the memoized filterItems callback.
95-157: LGTM!The render logic is well-structured with proper conditional states (loading, empty, and populated). The price formatting with
toLocaleString()provides good localization, and the category tabs with active state create a clear user interface.
| if (response.status === 204 || !response.data.itemList || response.data.itemList.length === 0) { | ||
| // API 데이터가 없으면 목 데이터 사용 | ||
| alert("반환된 상품이 없습니다."); | ||
| } else { | ||
| const remappedData: Item[] = response.data.itemList.map((item: any) => ({ | ||
| itemId: item.itemId, | ||
| itemName: item.itemName, | ||
| itemCode: item.itemCode, | ||
| itemPrice: item.itemPrice, | ||
| category: item.category || '기타', | ||
| isNew: item.isNew || false, | ||
| isHot: item.isHot || false, | ||
| })); | ||
| setItems(remappedData); | ||
| setFilteredItems(remappedData); |
There was a problem hiding this comment.
Remove redundant alert in favor of UI empty state.
The alert() call is redundant since the component already displays "상품이 없습니다." in the UI (line 136) when there are no items. Additionally, alert() blocks user interaction and provides poor UX.
Apply this diff to remove the alert:
if (response.status === 204 || !response.data.itemList || response.data.itemList.length === 0) {
- // API 데이터가 없으면 목 데이터 사용
- alert("반환된 상품이 없습니다.");
+ // API returned no items - empty state will be shown in UI
} else {🤖 Prompt for AI Agents
In src/components/User/ItemList/index.tsx around lines 64 to 78, remove the
blocking alert call when the API returns no items and instead leave the items
state untouched (or explicitly set to an empty array) so the component's
existing UI empty state ("상품이 없습니다.") at line 136 can render; update the branch
that handles response.status === 204 || !response.data.itemList ||
response.data.itemList.length === 0 to omit alert(...) and ensure setItems([])
and setFilteredItems([]) are used (or no-op if already empty) so UX is
non-blocking and consistent with the component's empty-state rendering.
| } catch (error) { | ||
| console.error('Error fetching items:', error); | ||
| } finally { | ||
| setLoading(false); |
There was a problem hiding this comment.
Add user-facing error feedback for API failures.
The catch block silently handles errors with only a console log. Users cannot distinguish between "no items available" and "API error occurred". When the API fails, users see the misleading "상품이 없습니다." message instead of understanding that a system error prevented loading items.
Consider adding error state and user notification:
const [loading, setLoading] = useState<boolean>(true);
+ const [error, setError] = useState<string | null>(null);
const fetchItems = async () => {
try {
setLoading(true);
+ setError(null);
const response = await axiosInstance.get('v2/item/');
// ... existing code ...
} catch (error) {
console.error('Error fetching items:', error);
+ setError('상품 목록을 불러오는 중 오류가 발생했습니다. 다시 시도해 주세요.');
} finally {
setLoading(false);
}
};Then update the render to show error state:
{loading ? (
<S.LoadingMessage>로딩 중...</S.LoadingMessage>
+ ) : error ? (
+ <S.EmptyMessage>{error}</S.EmptyMessage>
) : filteredItems.length === 0 ? (
<S.EmptyMessage>상품이 없습니다.</S.EmptyMessage>
) : (Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/components/User/ItemList/index.tsx around lines 80-83, the catch block
only logs API errors so the UI shows the same "상품이 없습니다." message for both empty
results and failures; add a local error state (e.g., error string or boolean)
and set it in the catch block with a user-friendly message when the fetch fails,
ensure setLoading(false) remains in finally, and update the render logic to show
a distinct error UI (toast, alert banner, or inline message with retry) when
error is set instead of the "no items" empty state.
2 participants
개요
스크린샷
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.