Conversation
|
Warning Rate limit exceeded@1jiwoo27 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 6 minutes and 43 seconds before requesting another review. ⌛ 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 (1)
Note
|
| Cohort / File(s) | 요약 |
|---|---|
예금 검색 페이지 src/pages/deposit-search/deposit-search-page.tsx, src/pages/deposit-search/deposit-search-page.css.ts |
신규 DepositSearchPage 및 스타일 추가: 은행 그리드(개별/전체선택), 기간 제어형 DropDown 연동, 검색 버튼, 초기 전체선택(saveTerm=36) 후 조회, 로딩·오류 처리, 결과 정렬 및 DepositBasic 렌더링. CSS에 bankLogo svg 글로벌 규칙 포함. |
적금 검색 페이지 src/pages/savings-search/savings-search-page.tsx, src/pages/savings-search/savings-search-page.css.ts |
신규 SavingsSearchPage 및 스타일 추가: 예금 페이지와 유사한 은행 선택/전체선택 UI, 기간 선택, 검색 흐름, 초기 로드 및 결과 정렬, SavingBasic 렌더링. CSS에 bankLogo svg 글로벌 규칙 포함. |
API 업데이트 src/shared/api/products.ts |
getSavingsList 및 getDepositsList 시그니처 확장: 선택적 bankNames?: string[], term?: number 파라미터 수용. 제공 시 bankNames는 콤마 직렬화해 쿼리로 전송, term은 존재하면 쿼리로 포함. |
상태 저장소 (Zustand) src/shared/stores/useSavingsSearchStore.ts, src/shared/stores/useDepositSearchStore.ts |
신규 Zustand 훅 추가: 검색 UI 상태 관리(selectedBanks, saveTerm, list, initialized) 및 뮤테이터(setSelectedBanks, setSaveTerm, setList, reset 등). |
드롭다운 컴포넌트 변경 src/shared/components/dropdown/dropdown.tsx, src/shared/components/dropdown/dropdown.css.ts |
DropDown을 제어형으로 전환: DropDownProps 추가, `value?: number |
라우팅 및 경로 추가 src/shared/router/index.tsx, src/shared/router/path.ts |
PATH에 SAVINGS_SEARCH(/savings/search) 및 DEPOSIT_SEARCH(/deposit/search) 추가하고 해당 페이지를 보호된 라우트로 등록. |
홈 페이지 네비게이션 src/pages/home/home-page.tsx |
홈 액션 버튼에 FontAwesome 아이콘 추가 및 클릭 시 적금/예금 검색 페이지로 네비게이션 후 부드러운 스크롤 수행(로딩/비로빙 경로 모두). |
은행 리스트 상수 및 에셋 매핑 src/shared/constants/bank-list.ts |
BankItem 타입 및 BANK_LIST 상수 추가(은행 id, name, logo 컴포넌트 매핑). |
UI 소소 변경 src/shared/components/deposit-basic/deposit-basic.css.ts, src/shared/components/savings-basic/savings-basic.css.ts |
depositContainer 및 savingsContainer에 cursor: 'pointer' 추가. |
비교 컴포넌트 스타일 조정 src/shared/components/deposit-compare/deposit-compare.css.ts, src/shared/components/savings-compare/savings-compare.css.ts |
detailContainer 너비를 10rem → 13rem으로 확장. |
텍스트/포맷팅 미세조정 src/shared/components/deposit-popular/deposit-popular.tsx, src/shared/components/savings-popular/savings-popular.tsx |
이모지와 텍스트 사이 공백 추가(표현상 변경). |
테마 토큰 src/styles/theme.css.ts |
사이즈 토큰 xxs: '1.4rem' 추가. |
패키지 package.json |
zustand 의존성 추가(^5.0.9). |
Sequence Diagram(s)
sequenceDiagram
participant User as 사용자
participant UI as SearchPage (React)
participant Store as Zustand Store
participant ClientAPI as 클라이언트(get*List)
participant Backend as 서버 API
User->>UI: 페이지 진입
UI->>Store: setSelectedBanks(all), setSaveTerm(36), setInitialized(true)
UI->>UI: loading=true
UI->>ClientAPI: get*List(bankNames=all, term=36)
ClientAPI->>Backend: GET /{deposits|savings}?bankNames=...&term=36
Backend-->>ClientAPI: ProductList[] 반환
ClientAPI-->>UI: ProductList[] 전달
UI->>UI: 정렬(maxRate ↓, baseRate ↓), setList, loading=false
User->>UI: 은행 토글 / DropDown 변경
UI->>Store: setSelectedBanks / setSaveTerm
User->>UI: 검색 버튼 클릭
UI->>ClientAPI: get*List(selectedBanksOrUndefined, term)
ClientAPI->>Backend: GET /{deposits|savings}?bankNames=...&term=...
Backend-->>ClientAPI: 필터링된 결과 반환
ClientAPI-->>UI: 결과 전달
UI->>UI: 정렬 및 렌더링
Estimated code review effort
🎯 3 (Moderate) | ⏱️ ~30 minutes
- 추가 검토 권장 항목:
src/shared/components/dropdown/dropdown.tsx: 제어형 전환으로 기존 사용처 호환성(널/언디파인드 처리).src/shared/api/products.ts:bankNames직렬화(인코딩),term이 없는 경우 쿼리 미포함 동작 확인.src/pages/deposit-search/*,src/pages/savings-search/*: 초기 전체선택/초기화(initialized) 로직과 선택이 없을 때 전체 조회 의도 검증, 정렬 로직 정확성.src/shared/constants/bank-list.ts: 로고 컴포넌트 임포트 경로 및 SVG 렌더링 호환성.- 글로벌 스타일 변경(
bankLogo svg) 및 레이아웃 변경(dropdownWrapper너비, compare detail width)이 UI에 미치는 영향 확인.
Pre-merge checks and finishing touches
❌ Failed checks (1 warning)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Out of Scope Changes check | 일부 변경사항이 주요 요구사항 범위를 벗어나 있습니다: 홈페이지 버튼 아이콘 추가, DropDown 너비 조정, 상세비교 페이지 너비 조정 등. | 홈페이지 아이콘 추가 및 UI 스타일 조정 변경사항들을 별도 이슈로 분리하거나 현재 PR 범위에서 제거하기를 권장합니다. |
✅ Passed checks (4 passed)
| Check name | Status | Explanation |
|---|---|---|
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | ✅ Passed | PR 제목은 예적금 검색페이지 구현이라는 주요 변경사항을 명확하고 간결하게 요약하고 있습니다. |
| Linked Issues check | ✅ Passed | PR은 예금/적금 검색 페이지 구현 요구사항을 충족하며, 필터링 조건(은행선택, 기간선택)을 포함한 완전한 구현을 제공합니다. |
| Docstring Coverage | ✅ Passed | No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. |
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 help to get the list of available commands and usage tips.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (14)
src/shared/api/products.ts (1)
4-10: 선택 파라미터 처리 로직은 적절하며,term처리만 약간 더 명시적으로 다듬을 수 있습니다
bankNames를 빈 배열일 때 제외하고 있을 때만join(',')으로 직렬화하는 방식은 백엔드 필터링 요구사항에 잘 맞는 구현 같습니다.
term: term || undefined는 현재 옵션 값이 1/3/6/12/24/36인 상황에서는 문제 없지만, 나중에 0 같은 값이 들어올 가능성을 고려하면 다음처럼 null 병합 연산자를 쓰면 의도가 더 분명합니다.- term: term || undefined + term: term ?? undefined필수는 아니고, 추후 확장성을 위한 선택적 정리로 보입니다.
Also applies to: 19-25
src/shared/components/dropdown/dropdown.tsx (1)
11-15: 제어형 전환은 잘 되어 있고,value표시 조건만 조금 정리하면 좋겠습니다
value를 부모에서 제어하도록 바꾼 구조는 DropDown 재사용성과 예적금 검색 페이지 요구사항에 잘 맞습니다.- 다만 placeholder 부분은 약간 정리할 여지가 있습니다.
<div className={styles.dropdownPlaceholder}> - {value ? `${value}개월` : DEFAULT_PLACEHOLDER || DEFAULT_PLACEHOLDER} + {value != null ? `${value}개월` : DEFAULT_PLACEHOLDER} </div>이렇게 하면:
- 0 같은 값이 들어와도 정상적으로 표시되고(미래 확장 대비),
- 불필요한
|| DEFAULT_PLACEHOLDER중복도 제거됩니다.기능 변경은 없고 가독성/명시성 개선 수준입니다.
Also applies to: 17-25, 35-35, 46-52
src/shared/components/deposit-basic/deposit-basic.css.ts (1)
4-13:cursor: 'pointer'추가는 상호작용 카드라는 의도를 잘 드러냅니다예금 카드(depositContainer)를 클릭해서 상세 페이지로 이동하는 패턴이라면 pointer 커서 추가가 자연스럽고 UX에도 도움이 됩니다.
다만 실제로 카드 전체에onClick이나 링크가 연결되어 있는지 한 번만 확인해 주시면 좋겠습니다. (시각적으로 클릭 가능해 보이는데 아무 동작이 없으면 혼란을 줄 수 있습니다.)src/shared/router/path.ts (1)
1-10: PATH 상수 추가는 적절하지만, 사용처에서 하드코딩된 문자열도 함께 치환하면 좋겠습니다
SAVINGS_SEARCH: '/savings/search',DEPOSIT_SEARCH: '/deposit/search'상수 추가는 라우터 설정과 잘 맞습니다.
다만HomePage에서navigate('/deposit/search'),navigate('/savings/search')처럼 문자열을 직접 쓰고 있으니, 거기도PATH.DEPOSIT_SEARCH,PATH.SAVINGS_SEARCH를 사용하도록 통일해 두면 변경에 더 강해질 것 같습니다.src/pages/home/home-page.tsx (1)
52-77: 검색 페이지 이동 버튼은 좋지만, 경로 상수 사용과 중복 제거를 고려해 보세요
- 홈에서 바로 예금/적금 검색 페이지로 이동할 수 있게 만든 UX 자체는 매우 좋습니다.
- 다만:
navigate('/deposit/search'),navigate('/savings/search')는PATH.DEPOSIT_SEARCH,PATH.SAVINGS_SEARCH를 사용하면 라우터와 한 곳에서 관리할 수 있습니다.- 로딩 분기(
if (Loading))와 일반 렌더 둘 다에서 거의 동일한 버튼 JSX가 반복되고 있어, 작은 컴포넌트로 추출하거나 공통 렌더 함수로 빼면 유지보수가 더 수월할 것 같습니다.예시 방향(개념):
// 공통 버튼 렌더 함수 const renderMainButtons = () => ( <div className={styles.mainButtonWrapper}> {/* 예금 버튼 */} {/* 적금 버튼 */} </div> ); // 로딩/비로딩 모두에서 renderMainButtons() 호출필수는 아니지만, 향후 스타일이나 텍스트를 수정할 때 실수 가능성을 줄여 줄 것 같습니다.
Also applies to: 88-108
src/pages/savings-search/savings-search-page.tsx (3)
48-59: 데이터 정렬 로직은 괜찮지만, 에러 처리와 타입을 조금 더 명시적으로 하면 좋겠습니다const fetchSavingList = async (banks: string[] | null, term: number) => { ... const res = await getSavingsList(banks || undefined, term); let data = res?.result || []; data = data.sort((a:ProductList, b:ProductList) => b.maxRate-a.maxRate); setSavingList(data); }
maxRate기준 내림차순 정렬은 의도에 잘 맞습니다.- 다만:
let data = res?.result || [];는any로 흘러갈 수 있어ProductList[]로 타입을 고정해 두면 좋습니다.catch (error) {}에서 에러를 완전히 무시하고 있어, 최소한console.error로그를 남기거나, 에러 상태를 별도 state로 관리해 사용자에게 "조회 실패" 메시지를 보여주는 게 좋겠습니다.예시:
let data: ProductList[] = res?.result ?? []; data = [...data].sort((a, b) => b.maxRate - a.maxRate); // 혹은 res 오류 시 savingList를 빈 배열로 초기화검색 페이지 특성상 네트워크 오류와 "검색 결과 없음"을 구분해 주면 UX가 더 좋아질 것 같습니다.
79-84: 은행이 하나도 선택되지 않았을 때의 처리 의도는 맞지만, 코드 의도를 조금 더 드러내면 좋겠습니다현재 로직:
const handleSearch = () => { if (selectedBanks.length === 0) { setSelectedBanks(BANK_LIST.map(b=>b.id)); } fetchSavingList(selectedBanks.length > 0 ? selectedBanks : null, saveTerm); };
- 선택된 은행이 없으면 UI 상으로는 전체 은행을 다시 선택하고, API 호출은
null을 넘겨 "전체 조회"로 처리하는 구조입니다.- 동작 자체는 문제 없지만,
setSelectedBanks는 비동기라서 처음 읽을 때는 "state가 반영되지 않은 값으로 호출되는 것 아닌가?" 하는 오해를 부를 수 있습니다.의도를 더 분명히 하는 방향 예시:
const handleSearch = () => { const banks = selectedBanks.length > 0 ? selectedBanks : null; if (!banks) { setSelectedBanks(BANK_LIST.map(b => b.id)); } fetchSavingList(banks, saveTerm); };기능 변화 없이 가독성과 의도 전달을 개선하는 수준이라 여유 있을 때 고려해 보시면 좋겠습니다.
92-139: 로딩/일반 렌더링에서 은행/기간/검색 버튼 UI가 중복되어 있습니다
if (loading)분기와 그 아래 기본return블록을 보면, Header + 은행 선택 섹션 + 기간 선택 섹션 + 검색 버튼까지 대부분이 동일하고, 차이는 결과 리스트 유무 정도입니다.이렇게 중복이 많으면:
- 스타일이나 텍스트를 수정할 때 한쪽만 바뀌는 버그가 생기기 쉽고,
- 유지보수 난이도가 올라갑니다.
구조를 다음과 같이 정리해 볼 수 있습니다.
- 공통 부분(헤더, 은행/기간 선택, 검색 버튼)을 하나의 JSX 블록/컴포넌트로 추출
savingList아래 영역에서만loading여부에 따라 스피너/스켈레톤 vs 결과 리스트를 분기개략 예:
const renderForm = () => ( <> {/* 은행 선택 섹션 */} {/* 기간 선택 섹션 */} {/* 검색 버튼 */} </> ); return ( <> <Header /> <main className={styles.MainContainer}> {renderForm()} <div className={styles.savingListContainer}> {loading ? <LoadingSkeleton /> : <ResultList />} </div> </main> </> );지금 구조에서도 동작은 문제 없지만, PR 단위에서 함께 정리해 두면 이후 검색 조건이 더 늘어날 때도 훨씬 편할 것 같습니다.
Also applies to: 142-205
src/pages/deposit-search/deposit-search-page.tsx (4)
47-58: 데이터 타입 명시와 에러 처리 보완을 고려해 주세요.현재
res?.result가 사실상any로 취급되고,catch블록이 비어 있어 API 오류 시 조용히 실패하고 단순히 빈 결과처럼 보일 수 있습니다. 타입 안정성과 디버깅 편의를 위해 결과를ProductList[]로 단언/가드하고, 최소한 콘솔 로그나 에러 상태 관리를 추가하는 편이 좋겠습니다.- const fetchDepositList = async (banks: string[] | null, term: number) => { - setIsLoading(true); - try { - const res = await getDepositsList(banks || undefined, term); - let data = res?.result || []; - data = data.sort((a:ProductList, b:ProductList) => b.maxRate-a.maxRate); - setDepositList(data); - } catch (error) { - } finally { - setIsLoading(false); - } - }; + const fetchDepositList = async (banks: string[] | null, term: number) => { + setIsLoading(true); + try { + const res = await getDepositsList(banks || undefined, term); + const data: ProductList[] = (res?.result ?? []).sort( + (a, b) => b.maxRate - a.maxRate, + ); + setDepositList(data); + } catch (error) { + // TODO: 사용자에게 에러를 보여줄 계획이 없다면 최소한 로그는 남겨 두는 편이 좋습니다. + console.error('예금 상품 조회 실패:', error); + } finally { + setIsLoading(false); + } + };
78-83: 검색 로직을 한 번에 읽기 쉽게 정리하면 좋겠습니다.현재는
selectedBanks.length === 0일 때setSelectedBanks와fetchDepositList(null, …)를 함께 호출하는 형태라 의도가 바로 눈에 들어오지 않습니다. “선택이 없으면 전체 선택 후 그 목록으로 검색한다”는 의도가 드러나도록, 유효한 은행 목록을 먼저 계산해서 상태와 API 호출에 함께 사용하는 식으로 단순화하면 가독성이 좋아집니다.- const handleSearch = () => { - if (selectedBanks.length === 0) { - setSelectedBanks(BANK_LIST.map(b=>b.id)); - } - fetchDepositList(selectedBanks.length > 0 ? selectedBanks : null, saveTerm); - }; + const handleSearch = () => { + const effectiveBanks = + selectedBanks.length > 0 ? selectedBanks : BANK_LIST.map((b) => b.id); + + if (selectedBanks.length === 0) { + setSelectedBanks(effectiveBanks); + } + + fetchDepositList(effectiveBanks, saveTerm); + };
91-139: 로딩 상태 분기에서 JSX가 거의 중복되고 있어 유지보수 비용이 높습니다.
if (loading)분기와 최종return쪽이 리스트 영역만 제외하고 거의 동일해서, 추후 UI 변경 시 두 군데를 동시에 수정해야 합니다. 공통 레이아웃은 한 번만 렌더링하고, 하단 리스트 영역만loading/데이터 없음/데이터 있음에 따라 분기하는 구조로 정리하면 코드가 더 단순해지고 실수 가능성도 줄어듭니다.Also applies to: 141-205
186-201: 리스트 key를 더 유니크하게 설정하는 것이 안전해 보입니다.현재 key가
productId만 사용되고 있는데, 같은 상품에 여러optionId가 존재할 수 있다면 key 충돌 가능성이 있습니다. 이런 경우optionId를 사용하거나\${productId}-${optionId}``처럼 조합해서 key를 주는 편이 안전합니다.src/pages/savings-search/savings-search-page.css.ts (1)
4-121: 전반적인 스타일 구조는 문제 없어 보이고, 예금 검색 페이지와의 일관성이 잘 맞습니다.레이아웃/그리드/버튼 구조가 예금 검색 CSS와 거의 동일해 두 페이지 간 UX 일관성이 좋습니다. 추후 유지보수를 생각하면, 공통되는 스타일(컨테이너, 은행 그리드 등)을 별도 공통 모듈로 분리해 두면 색상만 바꿔 재사용하기 더 편할 것 같습니다.
src/pages/deposit-search/deposit-search-page.css.ts (1)
4-121: 예금 검색 페이지 스타일도 구조적으로 잘 잡혀 있고, 적금 검색 페이지와 색상만 다른 형태로 일관성이 있습니다.공통 레이아웃과 컴포넌트(은행 버튼, 기간 섹션, 검색 버튼 등)가 이미 거의 동일하므로, 나중에 두 페이지가 더 늘어나면 이 파일과 적금 CSS에서 중복되는 부분을 공통 스타일로 추출해 두는 것을 고려해 볼 만합니다. 현재 상태로는 기능상 문제는 없어 보입니다.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
src/pages/deposit-search/deposit-search-page.css.ts(1 hunks)src/pages/deposit-search/deposit-search-page.tsx(1 hunks)src/pages/home/home-page.tsx(3 hunks)src/pages/savings-search/savings-search-page.css.ts(1 hunks)src/pages/savings-search/savings-search-page.tsx(1 hunks)src/shared/api/products.ts(2 hunks)src/shared/components/deposit-basic/deposit-basic.css.ts(1 hunks)src/shared/components/dropdown/dropdown.css.ts(1 hunks)src/shared/components/dropdown/dropdown.tsx(3 hunks)src/shared/router/index.tsx(1 hunks)src/shared/router/path.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (7)
src/shared/api/products.ts (2)
src/shared/api/axios.ts (1)
api(3-9)src/shared/api/endpoints.ts (1)
PRODUCTS(8-15)
src/pages/deposit-search/deposit-search-page.tsx (5)
src/shared/api/products.ts (1)
getDepositsList(19-27)src/pages/home/home-page.css.ts (1)
main(4-10)src/pages/deposit-search/deposit-search-page.css.ts (1)
section(15-20)src/shared/components/deposit-basic/deposit-basic.css.ts (1)
bank(15-19)src/shared/components/button/button.css.ts (1)
button(4-50)
src/shared/router/index.tsx (1)
src/shared/router/path.ts (1)
PATH(1-10)
src/pages/home/home-page.tsx (1)
src/shared/components/button/button.css.ts (1)
button(4-50)
src/pages/deposit-search/deposit-search-page.css.ts (1)
src/styles/theme.css.ts (1)
vars(3-47)
src/pages/savings-search/savings-search-page.tsx (3)
src/shared/api/products.ts (1)
getSavingsList(4-12)src/pages/savings-search/savings-search-page.css.ts (1)
section(15-20)src/shared/components/button/button.css.ts (1)
button(4-50)
src/pages/savings-search/savings-search-page.css.ts (1)
src/styles/theme.css.ts (1)
vars(3-47)
🪛 GitHub Actions: Build Test
src/pages/deposit-search/deposit-search-page.tsx
[error] 95-95: Property 'mainContainer' does not exist on type 'typeof import("/home/runner/work/fini-client/fini-client/src/pages/deposit-search/deposit-search-page.css")'. Did you mean 'MainContainer'?
[error] 95-95: TypeScript error: Incorrect CSS module class reference. Ensure the CSS module exports the class 'mainContainer' or use the correct exported name (e.g., 'MainContainer').
🪛 GitHub Check: build
src/pages/deposit-search/deposit-search-page.tsx
[failure] 144-144:
Property 'mainContainer' does not exist on type 'typeof import("/home/runner/work/fini-client/fini-client/src/pages/deposit-search/deposit-search-page.css")'. Did you mean 'MainContainer'?
[failure] 95-95:
Property 'mainContainer' does not exist on type 'typeof import("/home/runner/work/fini-client/fini-client/src/pages/deposit-search/deposit-search-page.css")'. Did you mean 'MainContainer'?
src/pages/savings-search/savings-search-page.tsx
[failure] 145-145:
Property 'mainContainer' does not exist on type 'typeof import("/home/runner/work/fini-client/fini-client/src/pages/savings-search/savings-search-page.css")'. Did you mean 'MainContainer'?
[failure] 96-96:
Property 'mainContainer' does not exist on type 'typeof import("/home/runner/work/fini-client/fini-client/src/pages/savings-search/savings-search-page.css")'. Did you mean 'MainContainer'?
🔇 Additional comments (2)
src/shared/router/index.tsx (1)
10-11: 검색 페이지 라우팅 추가는 구조적으로 문제 없어 보입니다
PATH.SAVINGS_SEARCH,PATH.DEPOSIT_SEARCH를 ProtectedRoute로 감싸 기존 상세 페이지와 동일한 인증 정책을 적용한 점이 일관적입니다.- 페이지 컴포넌트 import 및 PATH 정의와도 잘 맞습니다.
별다른 이슈 없이 바로 사용할 수 있는 구조로 보입니다.
Also applies to: 20-21
src/shared/components/dropdown/dropdown.css.ts (1)
5-8: width를 100%로 풀어준 변경은 검색 폼 레이아웃에 잘 맞습니다
dropdownWrapper를width: '100%'로 바꿔 부모 컨테이너 폭에 맞추도록 한 점은, 예적금 검색 페이지처럼 폼 요소를 한 줄에 정렬하는 레이아웃에 잘 어울립니다.
다른 페이지들에서 이 컴포넌트를 사용할 때도 보통 부모가 적절한 폭을 갖고 있을 것이므로, 부작용 가능성도 낮아 보입니다.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
src/pages/deposit-search/deposit-search-page.css.ts (2)
4-13: 고정 width(70rem) 대신 반응형 폭을 고려해보면 좋겠습니다.
mainContainer,bankContainer,termContainer모두width: '70rem'으로 고정되어 있어서 작은 화면에서는 가로 스크롤이 생길 수 있습니다.maxWidth+width: '100%'패턴으로 바꾸면 데스크톱 레이아웃은 유지하면서도 더 유연해집니다.예시 diff:
export const mainContainer = style({ - display: 'block', - width: '70rem', + display: 'block', + width: '100%', + maxWidth: '70rem', @@ export const bankContainer = style({ - display: 'flex', - flexDirection: 'column', - width: '70rem', + display: 'flex', + flexDirection: 'column', + width: '100%', @@ export const termContainer = style({ - display: 'flex', - gap: '1rem', - width: '70rem', + display: 'flex', + gap: '1rem', + width: '100%',Also applies to: 28-34, 89-95
44-55: 버튼 계열 스타일의 hover/focus 상태 및 border 일관성 제안
bankButton,selectAllButton,searchButton모두 클릭 가능한 요소지만, 커서 외에 시각적 피드백(hover/focus)이 없어 조금 밋밋할 수 있고,selectAllButton은 border 설정이 없는 반면searchButton은border: 'none'으로 명시돼 있어 약간 불일치가 있습니다. 접근성과 사용감 향상을 위해 공통 버튼 스타일에 hover/focus/disabled 등을 추가하는 것을 고려해보면 좋겠습니다.예시(참고용):
export const bankButton = style({ @@ - borderRadius: '5px', - cursor: 'pointer', + borderRadius: '5px', + cursor: 'pointer', + selectors: { + '&:hover': { + borderColor: vars.color.pink300, + }, + '&:focus-visible': { + outline: `2px solid ${vars.color.pink300}`, + outlineOffset: '2px', + }, + }, }); export const selectAllButton = style({ @@ - padding: '0.5rem 1rem', + padding: '0.5rem 1rem', + border: 'none', }); export const searchButton = style({ @@ - border: 'none', + border: 'none', @@ - width: '11rem', + width: '11rem', + selectors: { + '&:hover': { + backgroundColor: vars.color.pink400, + }, + '&:focus-visible': { + outline: `2px solid ${vars.color.pink400}`, + outlineOffset: '2px', + }, + }, });Also applies to: 66-69, 79-87, 97-114
src/pages/savings-search/savings-search-page.tsx (1)
92-140: 로딩/비로딩 분기에서 거의 동일한 JSX가 복제되어 유지보수 비용이 높습니다
if (loading) { return ( ... ) }분기와 최종return의 JSX가Header,<main>, 은행/기간/검색 섹션까지 거의 동일하게 두 번 작성되어 있습니다.
이 상태에서는 레이아웃이나 스타일을 수정할 때 두 곳을 항상 같이 수정해야 해서, 빠뜨리기 쉽고 버그로 이어질 가능성이 큽니다.한 개의 JSX 트리를 공통으로 반환하고,
- 상단 필터 UI는 항상 동일하게 렌더링하고,
- 상품 리스트 영역에서만
loading ? 로딩 UI : 결과/빈 상태 UI로 분기하는 형태로 리팩터링해 보시는 것을 권장드립니다.예:
savingListContainer안에서loading이면 “로딩 중” 플레이스홀더나 스켈레톤을, 아니면 현재 리스트/빈 결과 문구를 보여 주는 방식으로 정리하면 중복이 크게 줄어듭니다.Also applies to: 142-207
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/pages/deposit-search/deposit-search-page.css.ts(1 hunks)src/pages/savings-search/savings-search-page.css.ts(1 hunks)src/pages/savings-search/savings-search-page.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/pages/savings-search/savings-search-page.css.ts
🧰 Additional context used
🧬 Code graph analysis (1)
src/pages/savings-search/savings-search-page.tsx (5)
src/shared/api/products.ts (1)
getSavingsList(4-12)src/pages/home/home-page.css.ts (1)
main(4-10)src/pages/deposit-search/deposit-search-page.css.ts (1)
section(15-20)src/pages/savings-search/savings-search-page.css.ts (1)
section(15-20)src/shared/components/button/button.css.ts (1)
button(4-50)
🔇 Additional comments (4)
src/pages/deposit-search/deposit-search-page.css.ts (2)
36-43: 섹션/그리드 및 리스트 컨테이너 레이아웃 구성은 명확하고 일관적입니다.
bankGrid의 6열 그리드와depositListContainer의 컬럼 배치는 구조가 단순하면서도 페이지 역할에 잘 맞고, gap/margin 사용도 깔끔합니다. 현재 요구사항 기준으로는 그대로 유지해도 충분해 보입니다.Also applies to: 116-121
57-64: SVG 로고 컨테이너 + globalStyle 패턴이 적절합니다.
bankLogo에 고정 크기를 주고,globalStyle(\${bankLogo} svg`, …)`로 내부 SVG를 100% 채우도록 한 패턴이 일관된 로고 사이즈를 보장해 줘서 재사용성과 유지보수 측면에서 좋아 보입니다.Also applies to: 123-126
src/pages/savings-search/savings-search-page.tsx (2)
10-40: 상품 타입 및 은행 메타데이터 정의가 명확합니다
ProductList타입이 API 응답 구조를 잘 캡처하고 있고,BANK_LIST에 id/name/logo를 분리해 둔 구조도 이후 필터링/표시에 재사용하기 좋습니다. 현재 형태 그대로 유지해도 무방해 보입니다.
187-205: 결과 리스트 및 빈 상태 처리 로직이 직관적입니다
savingList.length > 0일 때는SavingBasic으로 매핑하고, 그렇지 않으면 “검색된 적금 상품이 없습니다.”라는 문구를 보여주는 흐름이 명확하고 사용자 입장에서도 이해하기 좋습니다. 빈 결과 문구에서 “적금”으로 용어를 맞춘 부분도 페이지 맥락과 잘 어울립니다.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/pages/deposit-search/deposit-search-page.tsx (1)
97-97: CSS 모듈 클래스 이름 불일치로 인한 빌드 실패 가능성이 여전히 있습니다.이전 리뷰/빌드 로그에 따르면 CSS 파일에서는
MainContainer를 export하는데, 여기서는styles.mainContainer를 사용하고 있어 타입/빌드 에러가 났었습니다. 현재 코드도 여전히styles.mainContainer를 사용하고 있어서, CSS 쪽에서 이름을 변경하지 않았다면 동일 에러가 재발할 가능성이 큽니다.- <main className={styles.mainContainer}> + <main className={styles.MainContainer}> ... - <main className={styles.mainContainer}> + <main className={styles.MainContainer}>CSS 모듈에서 실제로 export하는 이름(
MainContainervsmainContainer)을 다시 한 번 확인해서 둘 중 하나로 정리해 주세요.Also applies to: 146-146
🧹 Nitpick comments (5)
src/pages/deposit-search/deposit-search-page.tsx (5)
47-59: 정렬 시 원본 배열 변이 및 에러 처리 개선 여지가 있습니다.
res?.result가 재사용되는 참조일 수 있어서, 바로sort를 호출하면 원본을 변이할 수 있습니다. 그리고 에러 시에는 단순히 빈 배열만 세팅되어 원인 파악이 어렵습니다.- try { - const res = await getDepositsList(banks || undefined, term); - let data = res?.result || []; - data = data.sort((a:ProductList, b:ProductList) => b.maxRate-a.maxRate); - setDepositList(data); + try { + const res = await getDepositsList(banks || undefined, term); + const data: ProductList[] = [...(res?.result ?? [])].sort( + (a, b) => b.maxRate - a.maxRate, + ); + setDepositList(data); } catch (error) { - setDepositList([]); + // TODO: 에러 로깅/알림 필요 시 여기에서 처리 + setDepositList([]); } finally {이렇게 하면 원본 변이를 피하면서 정렬하고, 이후 에러 로깅도 쉽게 확장할 수 있습니다.
61-65: 기본 기간(36개월) 매직 넘버를 상수/초기값으로 한 번만 관리하는 것이 좋습니다.현재
useState<number>(36)과fetchDepositList(null, 36)에 동일한 숫자가 중복되어 있어서 나중에 기본 기간을 바꾸면 한 군데를 놓치기 쉽습니다.-const DepositSearchPage = () => { - const [depositList, setDepositList] = useState<ProductList[]>([]); - const [loading, setIsLoading] = useState(true); - const [selectedBanks, setSelectedBanks] = useState<string[]>([]); - const [saveTerm, setSaveTerm] = useState<number>(36); +const DEFAULT_TERM = 36; + +const DepositSearchPage = () => { + const [depositList, setDepositList] = useState<ProductList[]>([]); + const [loading, setIsLoading] = useState(true); + const [selectedBanks, setSelectedBanks] = useState<string[]>([]); + const [saveTerm, setSaveTerm] = useState<number>(DEFAULT_TERM); ... - setSelectedBanks(BANK_LIST.map(b=>b.id)); - fetchDepositList(null, 36); + setSelectedBanks(BANK_LIST.map(b => b.id)); + fetchDepositList(null, DEFAULT_TERM);이렇게 하면 기본값 변경 시 한 곳만 수정하면 되어 유지보수가 쉬워집니다.
79-92: 검색 시 선택 은행 상태와 실제 조회 대상 은행을 한 번에 계산하는 편이 더 명확합니다.현재
selectedBanks.length === 0일 때setSelectedBanks를 호출한 뒤, 조건식에서 다시BANK_LIST.map을 호출하고 있어 약간 중복/혼동 여지가 있습니다. 조회 대상 은행 배열을 한 번 계산해서 상태/조회에 같이 쓰면 의도가 더 분명합니다.- const handleSearch = () => { - if (loading) return; - if (selectedBanks.length === 0) { - setSelectedBanks(BANK_LIST.map(b=>b.id)); - } - fetchDepositList(selectedBanks.length > 0 ? selectedBanks : BANK_LIST.map(b=>b.id), saveTerm); - }; + const handleSearch = () => { + if (loading) return; + + const targetBanks = + selectedBanks.length > 0 ? selectedBanks : BANK_LIST.map((b) => b.id); + + if (selectedBanks.length === 0) { + setSelectedBanks(targetBanks); + } + + fetchDepositList(targetBanks, saveTerm); + };동일한 동작을 유지하면서도 “실제 어떤 은행들로 조회하는지”를 한눈에 볼 수 있습니다.
93-141: 로딩 상태와 일반 상태에서 동일 JSX가 중복되어 있어 유지보수 비용이 커질 수 있습니다.
if (loading)분기와 일반return에 필터/기간/검색 버튼 UI가 거의 동일하게 두 번 등장합니다. 이후 UI 변경 시 한쪽만 수정해서 동기화가 깨질 위험이 있습니다.공통 레이아웃을 별도 컴포넌트나 렌더 함수로 분리하고, 로딩 여부에 따라 “목록 영역만” 달리 렌더하는 형태를 고려해 볼 만합니다.
예시 스케치:
const FilterSection = () => ( <main className={styles.MainContainer /* 또는 mainContainer */}> {/* 은행/기간/검색 버튼 섹션 공통 */} </main> ); if (loading) { return ( <> <Header /> <FilterSection /> {/* 여기에는 스피너 / 스켈레톤 등만 추가 */} </> ); } return ( <> <Header /> <FilterSection /> {/* 여기에서만 depositList 렌더링 */} </> );이렇게 하면 UI 수정 시 한 곳만 손보면 되고, 테스트 관점에서도 중복이 줄어듭니다.
Also applies to: 143-206
189-201: 리스트 key에productId만 사용하는 경우 중복 가능성을 한 번 점검해보는 것이 좋습니다.
DepositBasic의 key를productId하나만 쓰고 있는데, 동일 상품(productId) 내에 여러 옵션(optionId)이 있을 수 있다면 key 충돌이 날 수 있습니다. 그런 구조라면 두 값을 조합한 key가 더 안전합니다.- depositList.map(item => ( + depositList.map((item) => ( <DepositBasic - key={item.productId} + key={`${item.productId}-${item.optionId}`}백엔드 도메인 상
productId가 항상 유니크한지 확실치 않다면, 위와 같이 변경하는 쪽이 리스크가 적습니다.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/pages/deposit-search/deposit-search-page.tsx(1 hunks)src/pages/savings-search/savings-search-page.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/pages/savings-search/savings-search-page.tsx
🔇 Additional comments (1)
src/pages/deposit-search/deposit-search-page.tsx (1)
10-39: 은행 메타데이터/타입 구조 전반적으로 명확합니다.
ProductList타입과BANK_LIST구성(식별용 id, 사용자 노출 name, 로고 컴포넌트 분리)이 명확해서 이후 필터링·렌더링 로직 이해하기 좋습니다. 현 시점에서 별도 수정 필요 없이 그대로 사용해도 좋아 보입니다.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
src/pages/savings-search/savings-search-page.tsx (4)
10-19:ProductList타입을 공용 타입으로 분리하는 걸 고려해 주세요현재 이 파일 안에서만
ProductList를 정의하고 있는데, API 응답이나SavingBasic컴포넌트 props와 구조가 사실상 같을 가능성이 큽니다. 이미 비슷한 타입이 다른 곳에 있다면 여기서 새로 정의하기보다는 공용 타입으로 추출해서 import하는 편이 유지보수에 더 유리합니다 (필드 변경 시 한 군데만 수정하면 됨).
21-40: BANK_LIST의id값과 로고 매핑이 실제 스펙과 일치하는지 한 번 더 점검이 필요해 보입니다
id값이 그대로getSavingsList의bankNames파라미터로 사용되기 때문에, 백엔드에서 기대하는 은행 식별자와 정확히 일치하지 않으면 필터링이 제대로 되지 않을 수 있습니다.BusanGyeongnamBankLogo,JeonbukGwangjuBankLogo등 일부 로고 컴포넌트를 여러 은행에서 재사용하고 있는데, 디자인/브랜딩 측면에서 의도된 것인지 확인이 필요해 보입니다.- 동일한 BANK_LIST가 예금/적금 검색 페이지 모두에서 사용된다면, 추후에는 공용 상수(예:
shared/constants/banks.ts)로 분리해 두면 중복을 줄일 수 있습니다.
65-69:useEffect와fetchSavingList의존성 관계를 명확히 하면 훅 린트 경고를 줄일 수 있습니다현재
useEffect의 의존성 배열이[]인데, 내부에서 컴포넌트 스코프의fetchSavingList를 호출하고 있어react-hooks/exhaustive-deps규칙이 켜져 있다면 경고가 날 수 있습니다.
fetchSavingList를useCallback으로 감싼 뒤:const fetchSavingList = useCallback(async (banks: string[] | null, term: number) => { // ... }, []);
useEffect에서는[fetchSavingList]를 의존성으로 두는 방식으로 정리하면 의도가 더 명확해지고 린트 경고도 피할 수 있습니다.
100-148: 로딩 분기에서 JSX가 크게 중복되고, 로딩 상태 표시가 모호합니다
- Line 100-148과 150-216의 필터 UI JSX가 거의 동일하게 두 번 선언되어 있어, 추후 UI 변경 시 두 군데를 모두 수정해야 하는 중복이 생깁니다.
- 로딩 중에는 동일한 필터 화면만 보이고 상품 리스트 영역이 사라지기 때문에, 사용자가 “요청이 진행 중인지 / 단순히 결과가 없는 것인지”를 직관적으로 구분하기 어렵습니다.
아래처럼 구조를 바꾸면 중복을 줄이면서 로딩 상태도 더 명확히 표현할 수 있습니다.
- if (loading) { - return( - <> - <Header/> - <main className={styles.mainContainer}> - {/* 은행/기간/검색 버튼 영역 ... (상품 리스트 없음) */} - </main> - </> - ); - }; - - return ( - <> - <Header/> - <main className={styles.mainContainer}> - {/* 은행/기간/검색 버튼 영역 ... */} - {/*상품 리스트*/} - <div className={styles.savingListContainer}> - {savingList.length > 0 ? ( - ... - ) : ( - <p>검색된 적금 상품이 없습니다.</p> - )} - </div> - </main> - </> - )}; + return ( + <> + <Header/> + <main className={styles.mainContainer}> + {/* 은행/기간/검색 버튼 영역 ... (한 번만 정의) */} + {/*상품 리스트*/} + <div className={styles.savingListContainer}> + {loading ? ( + <p>적금 상품을 불러오는 중입니다...</p> + ) : savingList.length > 0 ? ( + ... + ) : ( + <p>검색된 적금 상품이 없습니다.</p> + )} + </div> + </main> + </> + );이렇게 하면
- 필터 UI JSX 중복이 사라지고,
- 로딩/결과 없음/정상 결과 상태를 한 곳에서 명확하게 분기할 수 있어 유지보수와 UX 모두 좋아질 것 같습니다.
Also applies to: 150-216
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/pages/deposit-search/deposit-search-page.tsx(1 hunks)src/pages/savings-search/savings-search-page.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/pages/deposit-search/deposit-search-page.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
src/pages/savings-search/savings-search-page.tsx (5)
src/shared/api/products.ts (1)
getSavingsList(4-12)src/pages/home/home-page.css.ts (1)
main(4-10)src/pages/savings-search/savings-search-page.css.ts (1)
section(15-20)src/pages/deposit-search/deposit-search-page.css.ts (1)
section(15-20)src/shared/components/button/button.css.ts (1)
button(4-50)
🔇 Additional comments (2)
src/pages/savings-search/savings-search-page.tsx (2)
71-79: 은행 선택/전체 선택 토글 로직이 깔끔합니다
handleBankToggle과handleSelectAll둘 다 불변 배열 패턴으로 잘 구현되어 있어, 상태 변경 흐름이 직관적이고 버그 여지가 적어 보입니다. 별다른 수정 없이 이대로 유지하셔도 좋겠습니다.Also applies to: 93-99
83-91: 검색 처리에서 로딩 가드와 은행 목록 계산이 잘 정리되어 있습니다
if (loading) return;으로 로딩 중 중복 검색 호출을 막아, 이전에 지적되었던 응답 경쟁(race) 이슈를 잘 해소하신 것 같습니다.selectedBanks가 비어 있을 때BANK_LIST전체를 한 번targetBanks로 계산해 조회에 사용하고, 동시에setSelectedBanks(targetBanks)로 UI 상태도 맞춰 주는 구조라 이후 유지보수 시 의도가 분명해 보입니다.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
src/pages/savings-search/savings-search-page.tsx (3)
63-67: 초기 useEffect의setIsLoading(true)와null파라미터는 조금 정리해 볼 수 있습니다
fetchSavingList내부에서 이미setIsLoading(true)를 호출하고 있어서,useEffect안의setIsLoading(true)는 기능적으로 중복입니다. 제거해도 동작에는 영향이 없습니다.- 초기 조회에서도 UI 상으로는
BANK_LIST전체를 선택하고 있으니,fetchSavingList(null, DEFAULT_TERM)대신 아래처럼 실제 API에도 동일한 리스트를 넘겨 주면 “선택된 은행”과 “요청 파라미터”가 항상 일치해 유지보수 관점에서 더 직관적입니다.useEffect(() => { const allBankIds = BANK_LIST.map((b) => b.id); setSelectedBanks(allBankIds); fetchSavingList(allBankIds, DEFAULT_TERM); }, []);
81-90:loading가드와 함께 검색 버튼을 비활성화하면 중복 호출을 더 명확히 막을 수 있습니다
handleSearch에서if (loading) return;으로 중복 검색 호출을 막은 점은 좋습니다.
UX 관점에서는 로딩 중 검색 버튼을disabled처리해 시각적으로도 “지금은 검색 중”이라는 상태를 보여 주면 더 명확합니다. (이 부분은 이전 리뷰에서 한번 언급되었던 내용이기도 합니다.)예시:
const handleSearch = () => { if (loading) return; @@ - <button - className={`${styles.searchButton} ${button({variant:'blue'})}`} - onClick={handleSearch} - >검색</button> + <button + className={`${styles.searchButton} ${button({ variant: 'blue' })}`} + onClick={handleSearch} + disabled={loading} + > + 검색 + </button> @@ - <button - className={`${styles.searchButton} ${button({variant:'blue'})}`} - onClick={handleSearch} - >검색</button> + <button + className={`${styles.searchButton} ${button({ variant: 'blue' })}`} + onClick={handleSearch} + disabled={loading} + > + 검색 + </button>Also applies to: 137-141, 187-190
45-61: API 실패 시 초기 로딩/재검색 상황에서 모두 오해의 소지가 있는 UI 상태가 될 수 있습니다현재
fetchSavingList에서try/finally만 사용하고 있어:
- 초기 진입 시 호출이 실패하면
savingList는 빈 배열 그대로인데, 로딩이 끝난 뒤 화면에는 단순히 “검색된 적금 상품이 없습니다.”가 표시됩니다.- 이후 검색에서 실패할 경우에는 이전 검색의
savingList가 그대로 남아서, 사용자는 “새 조건으로 검색했는데 결과가 안 바뀌는” 상황을 겪을 수 있습니다.- 에러 로그도 남지 않아 디버깅이 어렵습니다.
간단한 에러 상태를 추가해서 “정상적으로 결과가 0건인 경우”와 “네트워크/서버 에러인 경우”를 구분해 주는 쪽을 추천드립니다. 예시:
@@ - const [selectedBanks, setSelectedBanks] = useState<string[]>([]); - const [saveTerm, setSaveTerm] = useState<number>(DEFAULT_TERM); - - const fetchSavingList = async (banks: string[] | null, term: number) => { - setIsLoading(true); - try { - const res = await getSavingsList(banks || undefined, term); - const data: ProductList[] = [...(res?.result ?? [])].sort( - (a, b) => b.maxRate - a.maxRate, - ); - setSavingList(data); - } finally { - setIsLoading(false); - } - }; + const [selectedBanks, setSelectedBanks] = useState<string[]>([]); + const [saveTerm, setSaveTerm] = useState<number>(DEFAULT_TERM); + const [fetchError, setFetchError] = useState<string | null>(null); + + const fetchSavingList = async (banks: string[] | null, term: number) => { + setIsLoading(true); + setFetchError(null); + try { + const res = await getSavingsList(banks || undefined, term); + const data: ProductList[] = [...(res?.result ?? [])].sort( + (a, b) => b.maxRate - a.maxRate, + ); + setSavingList(data); + } catch (err) { + console.error('적금 상품 조회 실패:', err); + // 에러 시 이전 결과가 남지 않도록 초기화 + setSavingList([]); + setFetchError('적금 상품 조회에 실패했습니다. 잠시 후 다시 시도해 주세요.'); + } finally { + setIsLoading(false); + } + }; @@ - <div className={styles.savingListContainer}> - {savingList.length > 0 ? ( + <div className={styles.savingListContainer}> + {fetchError ? ( + <p>{fetchError}</p> + ) : savingList.length > 0 ? ( savingList.map(item => ( @@ - ) : ( - <p>검색된 적금 상품이 없습니다.</p> - )} + ) : ( + <p>검색된 적금 상품이 없습니다.</p> + )} </div>이렇게 하면:
- 에러 시에는 명시적으로 “조회 실패” 메시지를 보여 줄 수 있고
- 정상 응답 0건인 경우에만 “검색된 적금 상품이 없습니다.”가 노출되어 UX가 명확해집니다.
Also applies to: 193-210
🧹 Nitpick comments (3)
src/pages/savings-search/savings-search-page.tsx (3)
10-19: ProductList 타입을 공용 도메인 타입으로 분리하면 유지보수성이 더 좋아집니다
ProductList가 실제 API 응답과SavingBasic컴포넌트 props 구조와 동일하다면, 이 파일 내에서 새로 정의하기보다는shared쪽(예: API 타입 혹은 공용 도메인 타입 파일)에서 한 번 정의한 타입을 import 해서 재사용하는 편이 안전합니다.
향후 필드가 추가·변경될 때 타입 정의를 여러 곳에서 맞춰줘야 하는 중복을 줄일 수 있습니다.
21-40: BANK_LIST를 공통 상수로 분리하면 예·적금 페이지 간 중복과 불일치를 줄일 수 있습니다여기 정의된
BANK_LIST는 순수한 메타데이터라서, 다른 페이지(예: 예금 검색, 인기 적금 카드 등)에서도 동일/유사하게 쓰이고 있을 가능성이 높습니다.
만약 실제로 여러 곳에서 같은 은행 리스트를 사용한다면,shared/constants/banks.ts같은 위치로 분리해서 재사용하면:
- 은행명/아이디/로고 변경 시 한 곳만 수정하면 되고
- 일부 페이지에서 은행 누락/오타가 나는 리스크도 줄일 수 있습니다.
현재 구현 자체는 문제 없고, 구조 정리 측면에서의 제안입니다.
98-146:if (loading)분기에서 전체 레이아웃을 복제하지 않고 공통 JSX를 공유하는 쪽이 깔끔합니다현재 로딩 분기와 일반 렌더 분기에서
<Header />,<main>, 은행/기간/검색 버튼 섹션이 거의 동일하게 두 번 작성되어 있습니다. 이 구조는:
- 스타일 수정이나 섹션 구조 변경 시 두 군데를 모두 수정해야 하고
- 일부 수정이 한 군데에만 반영되면 미묘한 UI 차이가 날 수 있습니다.
if (loading)분기를 제거하고, 항상 하나의 레이아웃만 렌더링한 뒤:
savingListContainer안에서loading,fetchError, 결과 유무에 따라
- 스켈레톤/스피너
- 에러 메시지
- “검색된 적금 상품이 없습니다.”
- 상품 리스트
를 조건부로 나누는 패턴을 쓰면 중복이 없어지고 가독성도 좋아집니다.
Also applies to: 148-214
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/pages/deposit-search/deposit-search-page.tsx(1 hunks)src/pages/savings-search/savings-search-page.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/pages/deposit-search/deposit-search-page.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
src/pages/savings-search/savings-search-page.tsx (5)
src/shared/api/products.ts (1)
getSavingsList(4-12)src/pages/savings-search/savings-search-page.css.ts (1)
section(15-20)src/pages/deposit-search/deposit-search-page.css.ts (1)
section(15-20)src/shared/components/savings-basic/savings-basic.tsx (2)
SavingsBasicProps(15-57)SavingsBasicProps(4-13)src/shared/components/savings-popular/savings-popular.tsx (2)
SavingsPopularProps(12-40)SavingsPopularProps(4-10)
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/pages/savings-search/savings-search-page.tsx (1)
50-61: API 오류가 발생하면 예외가 전파되어 컴포넌트가 중단될 수 있습니다.현재
try-finally구조만 있고catch블록이 완전히 없어서, API 호출 실패 시:
- 오류가 처리되지 않고 상위로 전파됩니다
- 사용자에게 오류 피드백이 제공되지 않습니다
- 이전 검색 결과가 그대로 남아 혼란을 줄 수 있습니다
최소한 오류 로깅과 상태 초기화를 추가해야 합니다.
다음과 같이 수정하세요:
const fetchSavingList = async (banks: string[] | null, term: number) => { setIsLoading(true); try { const res = await getSavingsList(banks || undefined, term); const data: ProductList[] = [...(res?.result ?? [])].sort( (a, b) => b.maxRate - a.maxRate, ); setSavingList(data); + } catch (error) { + console.error('적금 상품 조회 실패:', error); + setSavingList([]); } finally { setIsLoading(false); } };더 나은 사용자 경험을 위해 오류 상태를 추가하여 "조회에 실패했습니다" 메시지를 표시하는 것도 고려해 보세요.
🧹 Nitpick comments (2)
src/pages/savings-search/savings-search-page.tsx (2)
10-40: 공유 타입과 상수를 별도 파일로 분리하는 것을 고려해 보세요.
ProductList타입과BANK_LIST상수가 다른 검색 페이지(예: deposit-search-page)에서도 사용될 가능성이 있습니다. 공통 타입과 상수를src/shared/types또는src/shared/constants디렉토리로 추출하면 코드 재사용성과 일관성이 향상됩니다.또한 일부 은행이 동일한 로고를 공유하고 있습니다(예: 경남은행과 부산은행이
BusanGyeongnamBankLogo를 사용). 이것이 의도된 것인지 확인해 주세요.
81-90: 검색 로직을 단순화할 수 있습니다.Lines 84-88의 로직이 약간 중복됩니다.
targetBanks를 계산한 후selectedBanks가 비어있는지 다시 확인하고 있습니다. 다음과 같이 단순화하는 것이 더 명확합니다:const handleSearch = () => { if (loading) return; const targetBanks = selectedBanks.length > 0 ? selectedBanks : BANK_LIST.map((b) => b.id); if (selectedBanks.length === 0) { setSelectedBanks(targetBanks); } fetchSavingList(targetBanks, saveTerm); };또는 더 간결하게:
const handleSearch = () => { if (loading) return; - const targetBanks = selectedBanks.length > 0 ? selectedBanks : BANK_LIST.map((b) => b.id); - - if (selectedBanks.length === 0) { - setSelectedBanks(targetBanks); - } - fetchSavingList(targetBanks, saveTerm); + if (selectedBanks.length === 0) { + const allBanks = BANK_LIST.map((b) => b.id); + setSelectedBanks(allBanks); + fetchSavingList(allBanks, saveTerm); + } else { + fetchSavingList(selectedBanks, saveTerm); + } };
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/pages/deposit-search/deposit-search-page.tsx(1 hunks)src/pages/savings-search/savings-search-page.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/pages/deposit-search/deposit-search-page.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
src/pages/savings-search/savings-search-page.tsx (3)
src/shared/api/products.ts (1)
getSavingsList(4-12)src/pages/savings-search/savings-search-page.css.ts (1)
section(15-20)src/shared/components/button/button.css.ts (1)
button(4-50)
🔇 Additional comments (1)
src/pages/savings-search/savings-search-page.tsx (1)
63-67: 초기화 로직이 올바릅니다.컴포넌트 마운트 시 모든 은행을 선택하고 데이터를 가져오는 로직이 적절합니다. 빈 의존성 배열도 정확합니다.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (7)
src/pages/deposit-search/deposit-search-page.tsx (7)
1-18: 타입 정의와 기본 구조는 무난합니다
ProductList타입이 API 응답과DepositBasic에 넘기는 props 구조를 잘 반영하고 있어서 전체 흐름이 이해하기 쉽습니다.
이미 공용 예·적금 상품 타입이 다른 모듈(예:products.ts나 공통 타입 모듈) 어딘가에 정의돼 있다면, 그쪽을 import해서 재사용하면 타입 중복을 줄일 수 있을 것 같습니다. 없다면 이대로 두셔도 괜찮습니다.
43-60: fetch 로직은 잘 구성돼 있지만, 에러 처리와 banks 파라미터 타입을 조금 더 정리할 여지가 있습니다
- 정렬(
maxRate내림차순)과 로딩 플래그 on/off 흐름은 깔끔합니다.- 다만
getDepositsList가 실패하면finally덕분에loading은 false로 내려가지만, 에러가 상위로 전파되면서 unhandled rejection 로그가 남을 수 있고, 사용자 입장에서는 단순히 “상품 없음” 상태로 오해할 수 있습니다.fetchDepositList의banks인자를string[] | null로 받지만 실제 호출은 모두string[]이어서, 타입을 단순화하거나null을 명시적으로 처리해도 됩니다.예를 들면 다음 정도의 보강을 고려할 수 있습니다:
- const fetchDepositList = async (banks: string[] | null, term: number) => { + const fetchDepositList = async (banks: string[] | null, term: number) => { setIsLoading(true); try { - const res = await getDepositsList(banks || undefined, term); + const res = await getDepositsList(banks ?? undefined, term); const data: ProductList[] = [...(res?.result ?? [])].sort( (a, b) => b.maxRate - a.maxRate, ); setDepositList(data); - } finally { + } catch (error) { + // TODO: 공통 에러 처리 (토스트/에러 상태 등) 적용 시 이 부분에서 연결 + console.error(error); + } finally { setIsLoading(false); } };필수는 아니고, 추후 에러 UX를 설계할 때 같이 정리하면 좋을 것 같습니다.
62-66: 마운트 시 전체 은행 선택 + 기본 36개월 조회 흐름은 타당합니다초기 마운트 시:
BANK_LIST에서 모든id를 뽑아selectedBanks를 채우고- 동일한 리스트/기간으로 바로 조회하는 흐름은 자연스럽고 UX 상 문제 없어 보입니다.
react-hooks/exhaustive-deps규칙을 사용 중이라면, 이useEffect에서fetchDepositList미포함으로 경고가 나올 수 있으니, 필요하면void fetchDepositList(...)형태로만 호출하고 lint 예외를 두는 등의 방식을 고려해 보셔도 좋겠습니다.
68-96: 은행 선택/전체선택/검색 로직 전반은 일관적이지만, UX 관점에서 몇 가지 선택지를 고민해볼 수 있습니다좋은 점:
handleBankToggle이 함수형 업데이트를 사용해서 동시 클릭에도 안전하게 토글됩니다.handleSearch에서selectedBanks.length === 0이면 내부적으로 전체 은행을 대상으로 검색하고, 동시에setSelectedBanks로 UI 상태도 전체 선택으로 맞춰주는 점은 일관성이 있습니다.handleSelectAll이 “전부 선택되어 있으면 해제, 아니면 전부 선택” 토글을 제공하는 것도 자연스럽습니다.고려해볼 점(선택 사항):
- 현재 “전체선택” 버튼은 토글(전체 선택/전체 해제) 역할을 하지만, 라벨은 항상 “전체선택”으로 유지됩니다. 기획 의도에 따라 “전체선택/전체해제”를 상태에 따라 바꾸거나, 안내 문구로 “전체 선택 / 전체 해제 토글”임을 명확히 할지 한 번 더 논의해 보셔도 좋겠습니다.
- 사용자가 일부러 은행을 모두 해제한 뒤 검색을 눌렀을 때, 자동으로 다시 전체 은행이 선택되는 동작(
selectedBanks.length === 0분기)이 직관적인지(=“은행을 하나도 선택 안 하면 전체 검색으로 간주한다”는 규칙) 기획과 UX 측면에서 한 번만 더 확인해 보시면 좋겠습니다.로직 자체에는 치명적인 버그는 없어 보이고, 주로 UX 정책에 대한 선택의 문제라서, 현재 방식이
savings-search페이지와 동일하다면 일단 그대로 가도 괜찮아 보입니다.
98-127: 은행 선택 버튼 JSX 구조는 괜찮지만, 접근성 측면에서 약간의 보강을 추천드립니다현재 구조:
- 버튼 + 아이콘 + 텍스트로 시각적으로는 충분히 상태가 구분됩니다.
- 하지만 스크린 리더 기준으로는 “선택됨/선택 안 됨” 상태를 알기 어렵습니다.
작은 수정으로 접근성을 개선할 수 있습니다:
- {BANK_LIST.map((bank) => { - const isSelected = selectedBanks.includes(bank.id); - return ( - <button key={bank.id} className={`${styles.bankButton} ${isSelected ? styles.bankButtonSelected : ''}`} - onClick={() => handleBankToggle(bank.id)}> + {BANK_LIST.map((bank) => { + const isSelected = selectedBanks.includes(bank.id); + return ( + <button + key={bank.id} + type="button" + aria-pressed={isSelected} + className={`${styles.bankButton} ${isSelected ? styles.bankButtonSelected : ''}`} + onClick={() => handleBankToggle(bank.id)} + >
type="button"은 혹시 나중에 이 섹션이<form>안으로 들어가더라도 예기치 않은 submit을 막아 줍니다.aria-pressed는 토글 버튼 패턴에 맞는 속성으로, 현재 선택 상태를 보조 기술에 전달해 줍니다.
3-3:buttonCSS recipe import 이름을 조금 더 명확히 하면 가독성이 좋아질 수 있습니다지금은 CSS recipe를
button이라는 이름으로 import해서, JSX의<button>태그와 이름이 겹쳐 약간 혼동될 수 있습니다.-import { button } from '../../shared/components/button/button.css'; +import { button as buttonStyle } from '../../shared/components/button/button.css'; ... - className={`${styles.selectAllButton} ${button({variant:'pink'})}`} + className={`${styles.selectAllButton} ${buttonStyle({ variant: 'pink' })}`} ... - className={`${styles.searchButton} ${button({variant:'pink'})}`} + className={`${styles.searchButton} ${buttonStyle({ variant: 'pink' })}`}필수는 아니지만, 추후 유지보수 시 “이게 HTML 버튼인지 스타일 함수인지” 순간적으로 헷갈릴 여지를 줄여 줍니다.
Also applies to: 120-120, 138-138
144-163: 결과 리스트 렌더링 로직은 명확합니다
!loading일 때만 리스트/메시지를 보여주고,depositList.length에 따라 “상품 카드 목록” vs “검색된 예금 상품이 없습니다.” 분기하는 구조가 직관적입니다.- 위에서 정렬까지 해 주고 내려오기 때문에, 여기서는 렌더링에만 집중하고 있는 것도 좋습니다.
앞서 언급한 fetch 에러 처리만 보완된다면, “실패로 인한 빈 리스트”와 “정상 조회 결과 0건”을 구분해서 보여주는 것도 추후 UX 개선 포인트로 고려할 수 있겠습니다.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/pages/deposit-search/deposit-search-page.tsx(1 hunks)src/pages/savings-search/savings-search-page.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/pages/savings-search/savings-search-page.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
src/pages/deposit-search/deposit-search-page.tsx (4)
src/shared/api/products.ts (1)
getDepositsList(19-27)src/pages/deposit-search/deposit-search-page.css.ts (1)
section(15-20)src/shared/components/deposit-basic/deposit-basic.css.ts (1)
bank(15-19)src/shared/components/button/button.css.ts (1)
button(4-50)
🔇 Additional comments (2)
src/pages/deposit-search/deposit-search-page.tsx (2)
20-39:BANK_LIST의 id 값이 백엔드bankNames스펙과 정확히 맞는지 한 번 더 확인하는 게 좋겠습니다이 리스트의
id값이 그대로getDepositsList의bankNames쿼리 파라미터로 전달되기 때문에, 문자열이 백엔드에서 기대하는 값과 조금만 달라도 해당 은행 상품이 전혀 조회되지 않을 수 있습니다.
- 예:
'국민은행'vs'KB국민은행','아이엠뱅크'등 실제 API 스펙/샘플 응답과 비교 필요- 동일 패턴을 쓰는
savings-search페이지의 BANK_LIST와도 값이 완전히 일치하는지 비교하면 안전합니다.현재 코드 자체에는 즉시 보이는 버그는 없지만, 문자열 불일치가 있으면 디버깅이 까다로우니 스펙 기준으로 한 번 더 점검해 주세요.
129-142: 기간 DropDown과 검색 버튼 플로우는 자연스럽습니다 (타입 일치만 한 번 확인)
saveTerm을number상태로 두고,<DropDown color='pink' value={saveTerm} onChange={handleTermChange} />로 제어형 컴포넌트로 사용한 부분은 의도에 잘 맞습니다.handleSearch에서disabled={loading}을 함께 사용한 것도 중복 검색 방지에 도움이 됩니다(완전히 막지는 못해도 UX 상 충분히 괜찮은 수준입니다).한 가지 확인할 점:
DropDown컴포넌트의onChange가 실제로number를 인자로 넘기도록 구현되어 있는지(내부에서event.target.value를Number(...)로 변환하는지) 타입/구현을 한 번만 더 체크해 주세요. 만약 string이라면handleTermChange시점에서 변환해 주는 편이 안전합니다.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
src/pages/savings-search/savings-search-page.tsx (2)
140-146: 검색 버튼에 로딩 중 비활성화 처리가 누락되었습니다.현재 검색 버튼에
disabled속성이 없어서, 로딩 중에도 버튼이 시각적으로 활성화되어 있습니다. 사용자가 여러 번 클릭을 시도할 수 있으며, 로딩 상태가 명확하게 전달되지 않습니다.다음과 같이 수정해주세요:
<div className={styles.searchContainer}> <button className={`${styles.searchButton} ${button({variant:'blue'})}`} onClick={handleSearch} - + disabled={loading} >검색</button> </div>
51-64: finally 블록의 로직 오류로 검색 결과가 절대 표시되지 않습니다.Lines 60-63의 finally 블록에서
setIsError(true)를 무조건 실행하고 있습니다. finally 블록은 성공/실패 여부와 관계없이 항상 실행되므로, API 호출이 성공해도 에러 상태가 true로 설정됩니다.이로 인해 Line 148의 조건
!loading && !IsError가 항상 false가 되어 상품 리스트가 절대 표시되지 않습니다.다음과 같이 catch 블록을 추가하고 finally 블록을 수정해주세요:
const fetchSavingList = async (banks: string[] | null, term: number) => { setIsLoading(true); try { const res = await getSavingsList(banks || undefined, term); const data: ProductList[] = [...(res?.result ?? [])].sort( (a, b) => b.maxRate - a.maxRate, ); setSavingList(data); setIsError(false); + } catch (error) { + console.error('적금 상품 조회 실패:', error); + setSavingList([]); + setIsError(true); } finally { setIsLoading(false); - setIsError(true); } };
🧹 Nitpick comments (3)
src/pages/deposit-search/deposit-search-page.tsx (3)
10-39: 은행 상수 및 타입 정의 전반적으로 괜찮습니다만, 백엔드 키 값 재확인 부탁드립니다.
ProductList타입 정의와BANK_LIST구성 자체는 명확합니다. 다만 Line 20-39의id값이 그대로getDepositsList의bankNames쿼리 파라미터로 전달되므로, 백엔드에서 사용하는 은행명 키와 1:1로 일치하는지(특히'아이엠뱅크','한국스탠다드차타드은행'처럼 긴 문자열) 한 번만 더 확인해 주세요. 오타나 공백 차이만 있어도 특정 은행이 조회되지 않을 수 있습니다. 추가로,ProductList는 리스트가 아니라 개별 상품을 의미하므로DepositProduct같은 이름이 약간 더 직관적일 수 있습니다.
41-48: 기본 기간 상수와 DropDown 옵션/백엔드 지원 여부 확인 및 네이밍 정리 제안Line 41-48에서
DEFAULT_TERM = 36으로 초기 조회 기간을 설정하고, Line 82-84에서DropDown의value={saveTerm}로 제어하는 흐름은 일관적입니다. 다만 DropDown 컴포넌트의 옵션에 실제로 36개월 값이 존재하는지, 그리고 백엔드가term=36을 지원하는지 한 번만 확인해 두면 안전합니다. 또 변수명이saveTerm이라 의미가 약간 모호해 보여서,term또는selectedTerm정도로 바꾸면 이후 유지보수 시 읽기 더 편할 것 같습니다.Also applies to: 82-84
50-71: fetchDepositList/검색 핸들러 로직은 안정적이지만, 타입/의존성·UX를 약간 다듬을 수 있습니다.Line 50의
banks: string[] | null은 실제 호출부(Line 70, 93)를 보면 항상string[]만 넘기고 있어null케이스가 불필요해 보입니다.banks: string[]로 단순화하고 그대로getDepositsList(banks, term)에 전달하면 타입과 구현이 같이 깔끔해집니다. 또한useEffect(Line 67-71)에서fetchDepositList를 의존성 없이 사용하는 구조라 eslintexhaustive-deps를 쓰고 있다면 경고가 날 수 있으니,fetchDepositList를useCallback으로 감싸거나 effect 안에서 직접 API를 호출하는 형태로 바꾸는 것도 고려해 볼 만합니다. 마지막으로, 현재는 필터를 변경해도 검색 버튼을 누르기 전까지는 조회되지 않는 구조인데, 기획 의도가 “수동 검색(버튼 클릭 필수)”이 맞는지 한 번 확인해 보시면 좋겠습니다.Also applies to: 85-101
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/pages/deposit-search/deposit-search-page.tsx(1 hunks)src/pages/savings-search/savings-search-page.tsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
src/pages/deposit-search/deposit-search-page.tsx (5)
src/shared/api/products.ts (1)
getDepositsList(19-27)src/pages/home/home-page.css.ts (1)
main(4-10)src/pages/deposit-search/deposit-search-page.css.ts (1)
section(15-20)src/shared/components/deposit-basic/deposit-basic.css.ts (1)
bank(15-19)src/shared/components/button/button.css.ts (1)
button(4-50)
src/pages/savings-search/savings-search-page.tsx (3)
src/shared/api/products.ts (1)
getSavingsList(4-12)src/pages/savings-search/savings-search-page.css.ts (1)
section(15-20)src/shared/components/button/button.css.ts (1)
button(4-50)
🔇 Additional comments (5)
src/pages/savings-search/savings-search-page.tsx (4)
1-19: LGTM!임포트 구문과 타입 정의가 적절합니다.
21-42: LGTM!상수 정의가 명확하고 적절합니다.
72-100: LGTM!이벤트 핸들러 로직이 올바르게 구현되어 있습니다. 로딩 중 중복 검색 방지 및 빈 은행 선택 처리가 적절합니다.
105-105: No changes required —styles.mainContaineris correct.The CSS file exports the class as
mainContainer(lowercase camelCase), which matches the current usage in line 105. The previous review's suggestion to useMainContainerwas incorrect.src/pages/deposit-search/deposit-search-page.tsx (1)
103-147: 은행 선택/기간/검색 UI 구조와 상태 제어는 직관적이고 잘 구성되어 있습니다.Line 103-147의 Header + main 레이아웃, 은행 그리드 토글, 기간 DropDown, 검색 버튼까지 한 화면에 자연스럽게 연결되어 있어 사용 플로우가 명확합니다.
buttonrecipe 클래스와styles.*를 조합해 버튼 스타일을 재사용한 점, 검색 버튼에disabled={loading}을 걸어 중복 요청을 막은 부분도 구현이 깔끔합니다.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/pages/savings-search/savings-search-page.tsx (1)
104-104:styles.mainContainer가 CSS 모듈 export 이름과 불일치할 가능성이 있습니다이전에 정적 분석에서
mainContainer대신MainContainer를 제안한 코멘트가 있었는데, 현재도 Line 104에서styles.mainContainer를 사용 중입니다. CSS 파일에서 아직MainContainer로 export 하고 있다면 빌드/타입 에러가 계속 날 수 있습니다.CSS 모듈의 실제 export 이름과 맞춰 주세요. 만약
MainContainer가 맞다면 이 파일은 다음처럼 수정되어야 합니다.- <main className={styles.mainContainer}> + <main className={styles.MainContainer}>(반대로 CSS에서
mainContainer로 이미 변경했다면 이 코멘트는 무시하셔도 됩니다만, 한 번 더 확인 부탁드립니다.)
🧹 Nitpick comments (1)
src/pages/savings-search/savings-search-page.tsx (1)
147-172: 로딩 중 상품 영역이 완전히 비어 있어 사용자가 동작 여부를 파악하기 어렵습니다현재는
loading === true일 때 상품 리스트 영역이 렌더링되지 않아, 검색 버튼을 눌러도 “아무 일도 안 일어나는 것처럼” 느껴질 수 있습니다.필수는 아니지만, 아래처럼 리스트 영역 안에서 로딩/에러/결과를 한 번에 처리하면 UX가 더 자연스러울 것 같습니다.
- {/*상품 리스트*/} - {!loading && !isError && ( - <div className={styles.savingListContainer}> - {savingList.length > 0 ? ( - ... - ) : ( - <p>검색된 적금 상품이 없습니다.</p> - )} - </div> - )} - {!loading && isError && ( - <p> - 적금 상품을 불러오는 중 오류가 발생했습니다. 다시 시도해주세요. - </p> - )} + {/*상품 리스트*/} + <div className={styles.savingListContainer}> + {loading ? ( + <p>로딩 중입니다...</p> + ) : isError ? ( + <p>적금 상품을 불러오는 중 오류가 발생했습니다. 다시 시도해주세요.</p> + ) : savingList.length > 0 ? ( + savingList.map((item) => ( + <SavingBasic + key={`${item.productId}-${item.optionId}`} + ... + /> + )) + ) : ( + <p>검색된 적금 상품이 없습니다.</p> + )} + </div>이건 순수 UI 개선 포인트라, 우선순위에 따라 나중에 반영하셔도 괜찮습니다.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/pages/savings-search/savings-search-page.tsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/pages/savings-search/savings-search-page.tsx (3)
src/shared/api/products.ts (1)
getSavingsList(4-12)src/pages/savings-search/savings-search-page.css.ts (1)
section(15-20)src/shared/components/button/button.css.ts (1)
button(4-50)
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/pages/savings-search/savings-search-page.tsx (1)
51-65: 성공 시 에러 상태가 초기화되지 않아, 이전 에러 후 정상 조회해도 에러 메시지가 계속 표시됩니다현재
catch블록에서만setIsError(true)를 호출하고 있는데, 성공 시setIsError(false)를 호출하지 않아서 한 번 에러가 발생하면 이후 정상 조회 시에도isError상태가 그대로 남아있게 됩니다.또한 이전 리뷰에서 제안된 에러 로깅(
console.error)도 여전히 누락되어 있어 디버깅이 어렵습니다.다음과 같이 수정하세요:
const fetchSavingList = async (banks: string[] | null, term: number) => { setIsLoading(true); + setIsError(false); try { const res = await getSavingsList(banks || undefined, term); const data: ProductList[] = [...(res?.result ?? [])].sort( (a, b) => b.maxRate - a.maxRate, ); setSavingList(data); } catch (error) { + console.error('적금 상품 조회 실패:', error); setSavingList([]); setIsError(true); } finally { setIsLoading(false); } };
🧹 Nitpick comments (1)
src/pages/savings-search/savings-search-page.tsx (1)
149-174: 로딩 중 사용자 피드백이 부족합니다현재
loading상태일 때는 Line 149와 Line 170의 조건문 모두 해당되지 않아, 상품 리스트 영역에 아무것도 표시되지 않습니다. 사용자는 검색 버튼이 비활성화된 것만 보고 로딩 중인지 알기 어렵습니다.로딩 상태를 명시적으로 표시하도록 수정하는 것을 권장합니다:
{/*상품 리스트*/} - {!loading && !isError && ( + {loading ? ( + <div className={styles.savingListContainer}> + <p>상품을 불러오는 중입니다...</p> + </div> + ) : !isError ? ( <div className={styles.savingListContainer}> {savingList.length > 0 ? ( savingList.map(item => ( <SavingBasic key={`${item.productId}-${item.optionId}`} productId={item.productId} optionId={item.optionId} bankName={item.bankName} productName={item.productName} rsrvTypeNm={item.rsrvTypeNm} saveTerm={item.saveTerm} baseRate={item.baseRate} maxRate={item.maxRate} /> )) ) : ( <p>검색된 적금 상품이 없습니다.</p> )} </div> - )} - {!loading && isError && ( + ) : ( <p> 적금 상품을 불러오는 중 오류가 발생했습니다. 다시 시도해주세요. </p> )}필요하다면 로딩 스피너 컴포넌트를 사용해서 더 나은 UX를 제공할 수도 있습니다.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/pages/savings-search/savings-search-page.tsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/pages/savings-search/savings-search-page.tsx (4)
src/shared/api/products.ts (1)
getSavingsList(4-12)src/pages/home/home-page.css.ts (1)
main(4-10)src/pages/savings-search/savings-search-page.css.ts (1)
section(15-20)src/shared/components/button/button.css.ts (1)
button(4-50)
🔇 Additional comments (1)
src/pages/savings-search/savings-search-page.tsx (1)
103-180: 전반적인 구조와 과거 이슈 수정사항을 확인했습니다과거 리뷰에서 지적된 주요 이슈들(UI 중복, 로딩 경합 조건, 에러 상태 관리, 명명 규칙 등)이 대부분 잘 수정되었습니다. 컴포넌트 구조가 명확하고, 상태 관리와 핸들러 분리가 적절하며, 타입 안정성도 유지되고 있습니다.
위에서 언급한 에러 상태 초기화와 로딩 피드백 개선만 보완하시면 좋을 것 같습니다.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/pages/deposit-search/deposit-search-page.tsx (2)
50-65: 정렬·에러 처리 로직은 좋은데, 에러 UI 레이아웃만 리스트와 맞춰두면 더 자연스러울 것 같습니다.
fetchDepositList에서maxRate기준 내림차순 정렬과 에러 상태 초기화까지 포함해서 데이터 흐름이 잘 정리돼 있습니다.- 다만 에러일 때(
isError === true)는 Line 173-175에서 단순<p>만 렌더링해서, 정상/빈 데이터일 때 사용하는styles.depositListContainer와 레이아웃·여백이 달라질 수 있습니다.- 에러 메시지도 동일한 컨테이너 안에 넣으면 화면이 덜 튀고, 스크롤/패딩 일관성이 유지될 것 같습니다.
예시:
) : isError ? ( <div className={styles.depositListContainer}> <p>예금 상품을 불러오는 중 오류가 발생했습니다. 다시 시도해주세요.</p> </div> ) : ( ... )Also applies to: 149-176
67-71: '전체선택' / 무선택 시 동작을 조금 더 명확하게 해두면 UX가 좋아질 것 같습니다.
- 마운트 시에는 모든 은행을 선택한 뒤 기본 조건으로 자동 조회하는 플로우는 괜찮습니다.
- 이후 사용자가 모든 은행을 해제한 상태에서 검색을 누르면
selectedBanks.length === 0이어도 내부적으로 다시 전체 은행을 선택해 검색하는데, 사용자가 “정말 아무 은행도 선택하지 않겠다”는 의도였을 수도 있어 약간 헷갈릴 수 있습니다.handleSelectAll은 전체 선택/해제 토글 역할을 하지만 버튼 라벨은 항상 “전체선택”으로만 보이는 점도 같이 헷갈릴 수 있는 부분입니다.- 기획이 “무선택 = 전체 검색”이라면, 라벨을 “전체 선택/해제”로 바꾸거나, 선택 개수를 함께 보여주는 등 의도를 더 드러내면 좋고,
반대로 최소 1개를 반드시 선택하게 하려면selectedBanks.length === 0에서 검색 시 경고 문구를 띄우고 요청을 막는 방향도 고려해 볼 만합니다.Also applies to: 85-94, 95-101, 123-130
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/pages/deposit-search/deposit-search-page.tsx(1 hunks)src/pages/savings-search/savings-search-page.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/pages/savings-search/savings-search-page.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
src/pages/deposit-search/deposit-search-page.tsx (4)
src/shared/api/products.ts (1)
getDepositsList(19-27)src/pages/deposit-search/deposit-search-page.css.ts (1)
section(15-20)src/shared/components/deposit-basic/deposit-basic.css.ts (1)
bank(15-19)src/shared/components/button/button.css.ts (1)
button(4-50)
🔇 Additional comments (1)
src/pages/deposit-search/deposit-search-page.tsx (1)
1-182: 전반적인 구조와 상태 관리가 명확해서 바로 사용 가능한 수준으로 보입니다.
- 초기 진입 시 기본 조건(전체 은행 +
DEFAULT_TERM)으로 자동 조회 후, 필터 변경 뒤 “검색” 버튼으로 재조회하는 흐름이 직관적입니다.- 로딩/에러/빈 결과 상태를 분리해 처리한 점도 좋고, 정렬 기준(
maxRate내림차순) 역시 요구사항에 부합해 보입니다.- 위에 남긴 소소한 UX 제안 정도만 고민해 보면 바로 머지해도 될 것 같습니다.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
src/pages/deposit-search/deposit-search-page.css.ts (1)
1-132: 전체적으로 잘 구성된 CSS 모듈입니다.Vanilla Extract 스타일 정의가 적절하게 구성되어 있고, 테마 변수를 일관되게 사용하고 있습니다.
globalStyle을 통한 SVG 크기 조정도 적절합니다.다만
70rem값이mainContainer,bankContainer,termContainer에서 반복되고 있어, 추후 유지보수를 위해 공통 상수로 추출하는 것도 고려해 볼 수 있습니다.src/pages/savings-search/savings-search-page.tsx (3)
74-76:catch블록에 에러 로깅을 추가하면 디버깅에 도움이 됩니다.현재 에러 발생 시 상태만 업데이트하고 있어, 실제 에러 원인을 파악하기 어려울 수 있습니다.
} catch (error) { + console.error('적금 상품 조회 실패:', error); setSavingList([]); setIsError(true); } finally {
144-144: 불필요한 화살표 함수 래퍼를 제거할 수 있습니다.
handleSelectAll은 파라미터를 받지 않으므로 직접 참조가 가능합니다.- <button - className={`${styles.selectAllButton} ${button({ variant: 'blue' })}`} - onClick={() => handleSelectAll()} - > + <button + className={`${styles.selectAllButton} ${button({ variant: 'blue' })}`} + onClick={handleSelectAll} + >
26-56: ExtractBANK_LISTandProductListtype to a shared constants module.These definitions are duplicated in both
src/pages/deposit-search/deposit-search-page.tsxandsrc/pages/savings-search/savings-search-page.tsx. Consolidating them into a shared module (e.g.,src/constants/bank.tsorsrc/types/product.ts) will reduce maintenance burden and ensure consistency.src/pages/savings-search/savings-search-page.css.ts (1)
1-132: 예금 검색 페이지 CSS와 구조가 거의 동일합니다.
deposit-search-page.css.ts와 색상(blue vs pink)만 다르고 나머지 스타일 구조가 동일합니다. 현재 상태로도 동작에 문제는 없지만, 추후 공통 스타일을 추출하여 색상만 변수로 받는 방식으로 리팩토링하면 유지보수성을 높일 수 있습니다.예시로 공통 스타일 팩토리를 만들 수 있습니다:
// shared/styles/search-page.css.ts export const createSearchPageStyles = (colorScheme: 'blue' | 'pink') => { const primaryColor = colorScheme === 'blue' ? vars.color.blue300 : vars.color.pink300; const bgColor = colorScheme === 'blue' ? vars.color.blue100 : vars.color.pink100; // ... 공통 스타일 정의 };
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/pages/deposit-search/deposit-search-page.css.ts(1 hunks)src/pages/deposit-search/deposit-search-page.tsx(1 hunks)src/pages/savings-search/savings-search-page.css.ts(1 hunks)src/pages/savings-search/savings-search-page.tsx(1 hunks)src/styles/theme.css.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/pages/deposit-search/deposit-search-page.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
src/pages/savings-search/savings-search-page.css.ts (2)
src/pages/deposit-search/deposit-search-page.css.ts (4)
bankButton(44-56)bankButtonSelected(72-75)selectAllButton(85-93)searchButton(110-120)src/styles/theme.css.ts (1)
vars(3-48)
🔇 Additional comments (3)
src/styles/theme.css.ts (1)
37-37: LGTM!
xxs사이즈 토큰이 기존 사이즈 체계와 일관되게 추가되었습니다.xs(1.5rem)보다 작은 값(1.4rem)으로 적절하게 설정되어 있습니다.src/pages/savings-search/savings-search-page.tsx (2)
60-80: 상태 관리와 에러 처리 로직이 잘 구현되어 있습니다.
loading,isError상태를 적절히 활용하여 사용자 경험을 개선했고,try/catch/finally패턴으로 안정적인 에러 처리가 구현되어 있습니다.
169-195: 조건부 렌더링이 명확하게 구현되어 있습니다.로딩, 에러, 정상 상태를 삼항 연산자로 깔끔하게 분기 처리하여 사용자에게 적절한 피드백을 제공하고 있습니다.
1jiwoo27
left a comment
1jiwoo27
left a comment
There was a problem hiding this comment.
- 상품 선택했다가 뒤로 가기 하면 검색 조건 (은행 버튼, 기간)이 초기화되는데, 유지하도록 하면 어떨까요?!
- 그리고 BANK_LIST는 따로 파일 만들어서 저장해두면 예금과 적금에서 불러쓰기 좋을 것 같아요!
나머지 리뷰는 이따 추가적으로 남길게요 ~~ 수고하셨어요 검색 잘 되네요 👍🏻👍🏻🫡
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/shared/components/dropdown/dropdown.tsx (1)
33-35: placeholder 렌더링 표현 간소화 및 truthy 체크 개선 제안
{value ? \${value}개월` : DEFAULT_PLACEHOLDER || DEFAULT_PLACEHOLDER}는DEFAULT_PLACEHOLDER를 두 번 사용해 중복이고,value`를 truthy로만 체크해서 이론상 0일 때는 placeholder가 나옵니다.도메인상 0개월이 들어올 일은 거의 없겠지만, 조금 더 명확하게 적으면 좋겠습니다.
- {value ? `${value}개월` : DEFAULT_PLACEHOLDER || DEFAULT_PLACEHOLDER} + {value != null ? `${value}개월` : DEFAULT_PLACEHOLDER}이렇게 바꾸면 가독성이 좋아지고, 값이 0이어도 일관되게 레이블을 출력할 수 있습니다.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/shared/components/dropdown/dropdown.tsx(3 hunks)
🔇 Additional comments (2)
src/shared/components/dropdown/dropdown.tsx (2)
45-51: 외부value기반isSelected연동 👍
isSelected={option === value}로 선택 상태를 외부value에만 의존하게 바꾼 부분이 제어형 컴포넌트 의도와 잘 맞습니다.
옵션 목록과 상위 상태가 일관되게 동기화될 수 있는 구조라서 괜찮아 보입니다.
11-24: All DropDown usages already provide bothvalueandonChangeprops—no breaking changes detected.Search found only 2 usages of the component, both in
savings-search-page.tsx(line 155) anddeposit-search-page.tsx(line 154), and both explicitly passvalueandonChange. No uncontrolled usages exist that would silently break.However, line 34 has a minor code quality issue: the placeholder expression
DEFAULT_PLACEHOLDER || DEFAULT_PLACEHOLDERis redundant and should be simplified to justDEFAULT_PLACEHOLDER.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
src/pages/deposit-search/deposit-search-page.tsx (3)
29-42: 예금 조회 실패 시 최소한의 로깅을 추가하면 디버깅에 도움이 됩니다에러 시 리스트를 비우고
isError를 세팅해 UI에서 잘 분기하고 있어서 동작은 문제없습니다만, 운영 중 원인 분석을 위해catch에서 간단한 로그 정도는 남기는 편이 좋습니다.const fetchDepositList = async (banks: string[] | null, term: number) => { setIsLoading(true); setIsError(false); try { const res = await getDepositsList(banks || undefined, term); const data: ProductList[] = [...(res?.result ?? [])].sort((a, b) => b.maxRate - a.maxRate); setDepositList(data); } catch (error) { - setDepositList([]); - setIsError(true); + console.error('예금 상품 조회 실패:', error); + setDepositList([]); + setIsError(true); } finally { setIsLoading(false); } };
63-71: ‘은행 미선택 시 자동 전체 선택’ 동작이 기획 의도인지 한 번만 확인해 주세요
selectedBanks.length === 0이면 전체 은행을 선택해 주고 그 목록으로 조회하는 로직은 “아무 것도 선택 안 하면 곧바로 전체 검색”이라는 UX로 이해됩니다. 일부 사용자에게는 “은행을 선택해 달라”는 검증 메시지를 기대할 수도 있어서, 현재 동작이 기획/디자인과 합의된 방향인지 한 번만 점검해 보시면 좋겠습니다. 필요하다면 이 부분을 분기해 “경고 메시지 + 조회 막기” 패턴으로 바꾸는 것도 가능합니다.
133-158: 에러 상태도 리스트 컨테이너 안에서 렌더링하면 레이아웃 일관성이 좋아집니다지금은
loading/정상일 때는styles.depositListContainer안에서 메시지·리스트를 보여주고, 에러일 때만 맨 바깥에<p>만 렌더링되어 세로 여백이나 정렬이 달라질 수 있습니다. 적금 페이지와 마찬가지로 에러 메시지도 동일 컨테이너 안에서 렌더링하면 레이아웃이 더 균일해집니다.- ) : ( - <p>예금 상품을 불러오는 중 오류가 발생했습니다. 다시 시도해주세요.</p> - )} + ) : ( + <div className={styles.depositListContainer}> + <p>예금 상품을 불러오는 중 오류가 발생했습니다. 다시 시도해주세요.</p> + </div> + )}src/pages/savings-search/savings-search-page.tsx (2)
30-42: 예금/적금 fetch 로직이 완전히 동일해 공통 훅/유틸로 추출하기 좋습니다
fetchSavingList와 예금 쪽fetchDepositList가 파라미터 이름과 API 함수만 다르고 구조가 완전히 동일합니다. 나중에 에러 처리나 정렬 기준을 바꾸게 되면 두 군데를 모두 손봐야 하니,useProductSearch같은 공통 커스텀 훅이나 유틸 함수로 빼두면 유지보수 비용을 줄일 수 있을 것 같습니다. 이번 PR에서 꼭 하실 필요는 없지만, 이후 리팩터링 후보로 고려해 볼 만합니다.
81-132: Savings/Deposit 검색 페이지 레이아웃 공통 컴포넌트 도입 제안은행 그리드, 기간 선택, 검색 버튼까지의 JSX 구조와 상태 사용 패턴이 예금/적금 페이지에서 거의 동일합니다(색상과 Basic 컴포넌트만 다름). 향후 필터 항목이 늘어나거나 UI 스펙이 바뀔 때 두 페이지를 각각 수정해야 하므로, 공통
ProductSearchLayout컴포넌트(은행 선택 영역 + 기간 DropDown + 검색 버튼)를 만들어 색상/핸들러/Basic 컴포넌트만 props로 받는 구조로 정리해 두면 변경 비용과 UI 불일치 리스크를 줄일 수 있습니다.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/pages/deposit-search/deposit-search-page.tsx(1 hunks)src/pages/savings-search/savings-search-page.tsx(1 hunks)src/shared/constants/bank-list.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
src/pages/deposit-search/deposit-search-page.tsx (2)
src/shared/api/products.ts (1)
getDepositsList(19-27)src/shared/constants/bank-list.ts (1)
BANK_LIST(25-48)
src/pages/savings-search/savings-search-page.tsx (6)
src/shared/api/products.ts (1)
getSavingsList(4-12)src/shared/constants/bank-list.ts (1)
BANK_LIST(25-48)src/pages/home/home-page.css.ts (1)
main(4-10)src/pages/savings-search/savings-search-page.css.ts (1)
section(15-20)src/pages/deposit-search/deposit-search-page.css.ts (1)
section(15-20)src/shared/components/button/button.css.ts (1)
button(4-50)
🔇 Additional comments (1)
src/shared/constants/bank-list.ts (1)
19-48: BANK_LIST 중앙화 설계 좋습니다은행 메타데이터(id·표시명·로고)를 한 곳에서 관리하도록 잘 정리돼 있어서 예금/적금 검색 페이지에서 일관성 있게 재사용하기 좋아 보입니다. 추후 백엔드에서 사용하는
bankNames값이 변경될 수 있으니, 이id필드와 항상 동기화만 잘 챙겨주시면 될 것 같습니다.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/pages/deposit-search/deposit-search-page.css.ts (1)
1-135: 공통 스타일 추출을 고려해 보세요.이 파일은
savings-search-page.css.ts와 거의 동일하며, 색상 테마(핑크 vs 블루)만 다릅니다. 향후 유지보수를 위해 공통 스타일을 별도 모듈로 추출하고 색상만 파라미터화하는 것을 고려해 볼 수 있습니다.예시 구조:
// shared/styles/search-page-base.css.ts export const createSearchPageStyles = (themeColor: { primary: string; background: string }) => ({ // 공통 스타일 정의 });현재 구현도 정상 동작하므로 선택적 개선 사항입니다.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/pages/deposit-search/deposit-search-page.css.ts(1 hunks)src/pages/savings-search/savings-search-page.css.ts(1 hunks)src/shared/components/deposit-popular/deposit-popular.tsx(1 hunks)src/shared/components/savings-popular/savings-popular.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/pages/savings-search/savings-search-page.css.ts
🧰 Additional context used
🧬 Code graph analysis (1)
src/pages/deposit-search/deposit-search-page.css.ts (2)
src/pages/savings-search/savings-search-page.css.ts (14)
mainContainer(4-13)section(15-20)sectionTitle(22-27)bankContainer(29-35)bankGrid(37-43)bankButton(45-57)bankLogo(59-66)bankName(68-72)bankButtonSelected(74-77)selectAllContainer(79-85)selectAllButton(87-96)termContainer(98-104)searchContainer(106-111)searchButton(113-123)src/styles/theme.css.ts (1)
vars(3-48)
🔇 Additional comments (7)
src/shared/components/deposit-popular/deposit-popular.tsx (1)
32-32: AI 요약 앞 공백 추가로 가독성 개선전구 이모지 뒤에 공백을 넣어 텍스트 가독성이 좋아졌고, 동일 패턴 컴포넌트들과의 UI 일관성도 맞춰져서 그대로 가져가면 될 것 같습니다.
src/shared/components/savings-popular/savings-popular.tsx (1)
36-36: 동일 패턴 컴포넌트와 UI 일관성 잘 맞춤이모지 뒤 공백 추가로 deposit-popular와 시각적 스타일이 통일되었고, 기능 영향 없이 미세한 UI 품질만 개선되어 괜찮습니다.
src/pages/deposit-search/deposit-search-page.css.ts (5)
1-2: LGTM!Vanilla Extract의
style,globalStyle및 테마 변수를 올바르게 임포트하고 있습니다.
4-43: LGTM!컨테이너 및 그리드 스타일이 기존
savings-search-page.css.ts와 일관된 패턴을 따르고 있습니다.
74-96: 핑크 테마 적용 확인.예금 페이지에 핑크 색상 테마(
pink100,pink300)를 적용하여 적금 페이지(블루 테마)와 시각적으로 구분한 점이 좋습니다. 사용자가 두 페이지를 쉽게 구별할 수 있습니다.
125-130: LGTM!
depositListContainer스타일이 목록 아이템에 적절한 간격과 레이아웃을 제공합니다.
132-135: LGTM!
globalStyle을 사용하여 은행 로고 SVG가 컨테이너에 맞게 확장되도록 처리한 점이 좋습니다.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
src/shared/stores/useDepositSearchStore.ts (1)
1-48: 선택적으로 두 검색 스토어의 중복 코드를 제거할 수 있습니다.
useDepositSearchStore와useSavingsSearchStore가 거의 동일한 구조를 가지고 있습니다. 현재 코드는 명확하고 작동하지만, 향후 제네릭 팩토리 함수로 리팩토링하면 유지보수성을 높일 수 있습니다.현재로서는 두 스토어의 타입이 약간 다르고(예:
rsrvTypeNm필드) 코드가 충분히 작아서 중복을 허용해도 괜찮습니다.src/pages/savings-search/savings-search-page.tsx (2)
45-54: useEffect 의존성 배열이 불완전합니다.useEffect가
initialized,setSelectedBanks,fetchSavingsList,setInitialized,saveTerm을 사용하지만 의존성 배열이 비어있습니다. Zustand 셀렉터는 안정적이므로 의도적인 것으로 보이지만, ESLint 경고가 발생할 수 있습니다.다음 중 하나를 선택하세요:
방법 1: ESLint를 억제하고 의도를 명시
useEffect(() => { // Mount-only effect: initialize search with all banks if (!initialized) { const allBankIds = BANK_LIST.map((b) => b.id); setSelectedBanks(allBankIds); fetchSavingsList(allBankIds, saveTerm); setInitialized(true); } else { setIsLoading(false); } // eslint-disable-next-line react-hooks/exhaustive-deps }, []);방법 2: 필요한 의존성을 추가 (store 셀렉터는 안정적이므로 안전)
}, [initialized, setSelectedBanks, setInitialized, saveTerm]);
163-165: 에러 메시지를 컨테이너 안으로 이동하세요.에러 메시지가
savingListContainer밖에 렌더링되어 일관된 스타일링이 적용되지 않을 수 있습니다.다음과 같이 수정하면 더 일관된 레이아웃을 제공합니다:
) : ( + <div className={styles.savingListContainer}> <p>적금 상품을 불러오는 중 오류가 발생했습니다. 다시 시도해주세요.</p> + </div> )}src/pages/deposit-search/deposit-search-page.tsx (2)
46-55: useEffect 의존성 배열이 불완전합니다.useEffect가 여러 값을 사용하지만 의존성 배열이 비어있어 ESLint 경고가 발생할 수 있습니다. savings-search-page.tsx와 동일한 패턴입니다.
다음 중 하나를 선택하세요:
방법 1: ESLint를 억제하고 의도를 명시
useEffect(() => { // Mount-only effect: initialize search with all banks if (!initialized) { const allBankIds = BANK_LIST.map((b) => b.id); setSelectedBanks(allBankIds); fetchDepositList(allBankIds, saveTerm); setInitialized(true); } else { setIsLoading(false); } // eslint-disable-next-line react-hooks/exhaustive-deps }, []);방법 2: 필요한 의존성을 추가
}, [initialized, setSelectedBanks, setInitialized, saveTerm]);
165-165: 에러 메시지를 컨테이너 안으로 이동하세요.에러 메시지가
depositListContainer밖에 렌더링되어 일관된 스타일링이 적용되지 않을 수 있습니다.다음과 같이 수정하면 더 일관된 레이아웃을 제공합니다:
) : ( + <div className={styles.depositListContainer}> <p>예금 상품을 불러오는 중 오류가 발생했습니다. 다시 시도해주세요.</p> + </div> )}
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (5)
package.json(1 hunks)src/pages/deposit-search/deposit-search-page.tsx(1 hunks)src/pages/savings-search/savings-search-page.tsx(1 hunks)src/shared/stores/useDepositSearchStore.ts(1 hunks)src/shared/stores/useSavingsSearchStore.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
src/pages/savings-search/savings-search-page.tsx (4)
src/shared/stores/useSavingsSearchStore.ts (1)
useSavingsSearchStore(28-49)src/shared/api/products.ts (1)
getSavingsList(4-12)src/shared/constants/bank-list.ts (1)
BANK_LIST(25-48)src/shared/components/savings-basic/savings-basic.tsx (2)
SavingsBasicProps(15-57)SavingsBasicProps(4-13)
src/pages/deposit-search/deposit-search-page.tsx (8)
src/shared/stores/useDepositSearchStore.ts (1)
useDepositSearchStore(27-48)src/shared/api/products.ts (1)
getDepositsList(19-27)src/shared/constants/bank-list.ts (1)
BANK_LIST(25-48)src/pages/home/home-page.css.ts (1)
main(4-10)src/pages/deposit-search/deposit-search-page.css.ts (1)
section(15-20)src/pages/savings-search/savings-search-page.css.ts (1)
section(15-20)src/shared/components/deposit-basic/deposit-basic.css.ts (1)
bank(15-19)src/shared/components/button/button.css.ts (1)
button(4-50)
🔇 Additional comments (4)
package.json (1)
22-22: LGTM!Zustand는 가볍고 현대적인 상태 관리 라이브러리로, 검색 페이지의 상태 관리에 적합한 선택입니다.
src/shared/stores/useSavingsSearchStore.ts (1)
1-49: LGTM!Zustand 스토어 구현이 깔끔하고 모범 사례를 잘 따르고 있습니다. 함수형 업데이트 패턴과 reset 기능이 잘 구현되어 있습니다.
src/shared/stores/useDepositSearchStore.ts (1)
1-48: LGTM!구현이 적금 스토어와 일관된 패턴을 따르고 있어 유지보수에 좋습니다.
src/pages/deposit-search/deposit-search-page.tsx (1)
26-44: LGTM!Line 31의 옵셔널 체이닝과 nullish coalescing 패턴
[...(res?.result ?? [])]이 안전하게 구현되어 있습니다. API 응답이 예상과 다를 때도 런타임 에러 없이 안전하게 처리됩니다.
2 participants
📌 Related Issue
✅ Checklist
📚 Tasks
예적금 검색 페이지를 구현하고 홈페이지 버튼과 연결했습니다.
Summary by CodeRabbit
새로운 기능
개선사항
스타일
✏️ Tip: You can customize this high-level summary in your review settings.