Skip to content

[Feat] 북마크 페이지 구현#163

Open
qowjdals23 wants to merge 10 commits intodevfrom
feat/#154/bookmark-page
Open

[Feat] 북마크 페이지 구현#163
qowjdals23 wants to merge 10 commits intodevfrom
feat/#154/bookmark-page

Conversation

@qowjdals23
Copy link
Copy Markdown
Collaborator

@qowjdals23 qowjdals23 commented Mar 24, 2026

✏️ Summary

📑 Tasks

체크박스 컴포넌트 구현

bookmark-checkbox.tsx
bookmark-checkbox.css.ts

북마크 페이지에서 사용하는 체크박스 UI 컴포넌트를 분리했습니다.
전체 선택/개별 선택 모두 같은 컴포넌트로 재사용되도록 구현했습니다.

bookmark-table.tsx
bookmark-table.css.ts

테이블 마크업과 스타일은 페이지 파일에서 분리해 BookmarkTable로 컴포넌트화했습니다 !

북마크 페이지 UI 구현

북마크 페이지의 전체 레이아웃과 테이블 UI를 구현습니다.

북마크 목록 화면에서 빈 상태를 2가지로 분기해서 처리했습니다.

const isBookmarkEmpty = rows.length === 0;
const isSearchResultEmpty = rows.length > 0 && filteredRows.length === 0;
<section className={styles.tableSection}>
  {isBookmarkEmpty ? (
    <BookmarkEmptyState type="bookmark" />
  ) : isSearchResultEmpty ? (
    <BookmarkEmptyState type="search" />
  ) : (
    <BookmarkTable
      rows={currentPageRows}
      pageSize={BOOKMARK_PAGE_SIZE}
      selectedIds={selectedIds}
      isAllSelected={isAllSelected}
      onToggleAll={toggleAll}
      onToggleRow={toggleRow}
      onClickCompany={handleClickCompany}
    />
  )}
</section>

👀 To Reviewer

  • path.tspublic-routes.tsx에서의 북마크 라우팅은 임시 라우팅입니다. 머지 전에 최종 위치로 수정하겠습니다 !!
  • header.tsx 에서도 에러 이슈 때문에 프로필 api를 호출하지 않도록 임시로 처리해뒀는데, 머지 전에 수정하겠습니다 !!!!!

📸 Screenshot

image
  • 검색 결과가 없는 경우
image
  • 북마크한 기업이 없는 경우
image
  • 모달
image

🔔 ETC

Summary by CodeRabbit

  • New Features
    • 북마크 페이지 추가: 저장된 회사 목록 조회·검색, 다중 선택 및 삭제, 페이지네이션으로 목록 탐색 가능. 빈 상태와 검색 결과 없음에 대한 사용자 친화적 안내 제공.
  • Chores
    • 헤더 동작 조정: 북마크 페이지에서는 프로필 이름이 표시되지 않도록 변경되어 일관된 UI 동작 제공.

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Mar 24, 2026

📝 Walkthrough

Walkthrough

북마크 기능이 새로 추가되었습니다. 라우트 상수 및 공개 라우트 등록, 북마크 페이지(검색·필터·페이지네이션·삭제 포함), 관련 UI 컴포넌트·스타일·상수·이미지가 추가되었고, 헤더는 북마크 경로에서 프로필 조회를 건너뜁니다.

Changes

Cohort / File(s) Summary
라우트
src/app/routes/paths.ts, src/app/routes/public-routes.tsx
ROUTES.BOOKMARK 추가 및 BookmarkPage를 지연 로드하여 publicRoutes에 등록.
북마크 페이지 (페이지 + 스타일)
src/pages/bookmark/bookmark-page.tsx, src/pages/bookmark/bookmark-page.css.ts
검색 입력, 필터링, 페이지네이션, 다중선택, 삭제 모달 등 상태 관리와 조건부 렌더링을 구현한 메인 페이지 컴포넌트 및 스타일 추가.
페이지 설정 / 모의 데이터
src/pages/bookmark/config/bookmark-page.constant.ts
BookmarkRow 타입, BOOKMARK_PAGE_SIZE=4, 15개 모의 북마크 데이터 추가.
테이블 UI (컴포넌트 + 스타일)
src/pages/bookmark/ui/bookmark-table.tsx, src/pages/bookmark/ui/bookmark-table.css.ts
체크박스 열, 회사명 버튼, 스크랩 날짜, 연결 상태 표시, 플레이스홀더 행 유지 로직을 갖춘 테이블 컴포넌트 및 스타일 추가.
체크박스 UI (컴포넌트 + 스타일)
src/pages/bookmark/ui/bookmark-checkbox.tsx, src/pages/bookmark/ui/bookmark-checkbox.css.ts
커스텀 체크박스 컴포넌트(aria-label 포함) 및 SVG 기반 스타일 추가.
빈 상태 UI (컴포넌트 + 스타일)
src/pages/bookmark/ui/bookmark-empty-state.tsx, src/pages/bookmark/ui/bookmark-empty-state.css.ts
북마크 없음 / 검색 결과 없음 상태를 렌더링하는 접근성 고려 컴포넌트 및 스타일 추가.
공유 이미지 자산
src/shared/assets/images/index.ts
SEARCH_IMG 이미지 export 추가.
헤더 변경
src/widgets/header/header.tsx
현재 경로가 북마크인지 확인하여 북마크 페이지에서는 useGetProfile의 데이터 조회를 비활성화하도록 조건 추가.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Suggested labels

채영🥦

Suggested reviewers

  • u-zzn
  • odukong
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed PR 제목은 북마크 페이지 구현이라는 핵심 변경사항을 명확하게 요약하고 있습니다.
Description check ✅ Passed PR 설명이 템플릿 구조를 따르며 Summary, Tasks, Screenshots를 포함하고 있고, 변경사항이 상세히 문서화되어 있습니다.
Linked Issues check ✅ Passed PR의 모든 변경사항(체크박스 컴포넌트, 북마크 테이블, 페이지 UI 구현)이 #154 '북마크 페이지 구현' 요구사항을 충족합니다.
Out of Scope Changes check ✅ Passed header.tsx의 프로필 API 호출 임시 비활성화는 에러 해결을 위한 것으로, 저자가 병합 전 수정 예정을 명시했으므로 의도된 임시 변경입니다.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions github-actions bot added 🌟FEAT 새 기능 추가 정민🍐 labels Mar 24, 2026
@github-actions
Copy link
Copy Markdown

github-actions bot commented Mar 24, 2026

🚀 빌드 결과

린트 검사 완료
빌드 성공

로그 확인하기
Actions 탭에서 자세히 보기

@qowjdals23 qowjdals23 marked this pull request as ready for review March 24, 2026 12:57
Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 9

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/pages/bookmark/bookmark-page.css.ts`:
- Around line 69-78: The CSS rule applied via globalStyle for
`${deleteButtonWrap} > button` uses a rem value for borderRadius; update it to a
px value (e.g., change "1.2rem" to "12px") and ensure any related border
properties on the same selector (if added later) also use px units instead of
rem; locate the globalStyle call for `${deleteButtonWrap} > button` and replace
the borderRadius string value accordingly.
- Around line 80-94: The three globalStyle rules for `${deleteButtonWrap} >
button` currently reuse the same colors so hover/active/disabled give no visual
feedback; update the selectors (`${deleteButtonWrap} >
button:hover:not(:disabled)`, `${deleteButtonWrap} >
button:active:not(:disabled)`, `${deleteButtonWrap} > button:disabled`) to use
distinct themeVars values (e.g., a darker blue like themeVars.color.blue700 for
:hover, a slightly different blue like themeVars.color.blue500 or a pressed
shade for :active, and a muted/gray tone like themeVars.color.gray300 for
:disabled), set appropriate text color for contrast (e.g., themeVars.color.white
on active/hover, muted text color for disabled), and add a disabled cursor
(cursor: not-allowed) for the :disabled rule to provide clear visual and
affordance differences while keeping the existing selectors (`deleteButtonWrap`
and the specific pseudo-classes) intact.

In `@src/pages/bookmark/bookmark-page.tsx`:
- Around line 73-107: Rename the event-handler functions to follow the team's
"handle" prefix convention: rename toggleAll -> handleToggleAll, toggleRow ->
handleToggleRow, openDeleteModal -> handleOpenDeleteModal, and closeDeleteModal
-> handleCloseDeleteModal; update all uses (e.g., in JSX props or callbacks that
currently reference toggleAll, toggleRow, openDeleteModal, closeDeleteModal) and
keep existing signatures for setSelectedIds, setRows, setIsDeleteModalOpen,
handleDeleteConfirm, and handleClickCompany unchanged so behavior remains
identical.
- Around line 180-188: The delete modal's button styling is reversed: the
destructive action (onClick handler handleDeleteConfirm labeled "삭제하기") is using
variant="secondary" while the safe action (closeDeleteModal "취소하기") uses
variant="primary"; update the Button props so that the "삭제하기" Button uses the
destructive/primary visual (e.g., variant="primary" or the app's dangerous
variant) and the "취소하기" Button uses the secondary/neutral variant to reflect
proper visual priority.

In `@src/pages/bookmark/config/bookmark-page.constant.ts`:
- Around line 1-6: BookmarkRow 인터페이스가 중복 정의되어 타입 드리프트 위험이 있으니 현재 정의된 BookmarkRow
인터페이스에 export 키워드를 추가하여 공용으로 노출하고, 중복 정의된 곳(예: ui/bookmark-table.tsx)에 있는 로컬
BookmarkRow 타입을 제거한 뒤 해당 파일에서 export된 BookmarkRow를 import하여 재사용하도록 변경하세요; 대상 심볼:
BookmarkRow (export), 참조 파일 내 기존 로컬 타입 선언 제거 및 import 구문 추가를 잊지 마세요.

In `@src/pages/bookmark/ui/bookmark-checkbox.css.ts`:
- Around line 6-26: The checkbox style currently removes the browser focus
indicator via outline: "none" on the exported checkbox style; add explicit focus
styles for keyboard users by updating the selectors in the checkbox style to
include &:focus and &:focus-visible (or both) and provide a visible,
high-contrast indicator (for example a visible outline, ring or box-shadow with
sufficient contrast and some offset) while keeping the rest of the visual design
intact; ensure the focus style is applied only on keyboard focus (use
:focus-visible if supported) and matches the checkboxPressed/checkboxDisabled
sizes so the focus ring is clearly visible around the control.

In `@src/pages/bookmark/ui/bookmark-empty-state.tsx`:
- Around line 29-36: Replace the outer wrapper div with a semantic <section> to
improve page structure: change the element that currently uses
className={styles.emptySection} to a <section> (keeping the same className),
keep the inner container (className={styles.emptyContent}) as-is, and ensure the
image and text nodes remain unchanged; optionally add a concise aria-label or
aria-labelledby to the <section> for accessibility if this empty state needs a
descriptive label.

In `@src/pages/bookmark/ui/bookmark-table.tsx`:
- Around line 84-90: The span always renders the fixed text "연결" causing false
positives; update the JSX in bookmark-table.tsx so the span that uses
styles.connectionStatus({ connected: row.isConnected }) also conditionally
renders its inner text based on row.isConnected (e.g., show "연결" when
row.isConnected is true and an appropriate "미연결"/"연결 끊김" when false) so the
displayed label matches the boolean state.

In `@src/widgets/header/header.tsx`:
- Around line 20-24: The profile query is disabled on the bookmark route which,
combined with useGetProfile's staleTime: Infinity, causes stale or undefined
names to render; either remove the conditional disabling (remove the enabled:
isLoggedIn && !isBookmarkRoute) so useGetProfile always runs and fixes the stale
cache, or keep it disabled but guard rendering of {data?.name} by checking data
and query states from useGetProfile (isLoading/isError) and show a
skeleton/default label (e.g., "회원님") or a loader when data is missing; reference
useGetProfile, isBookmarkRoute, ROUTES.BOOKMARK, isLoggedIn and data?.name to
locate and implement the change.
🪄 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: ASSERTIVE

Plan: Pro

Run ID: 5957cbe7-2e63-4a49-83fa-5713c4167500

📥 Commits

Reviewing files that changed from the base of the PR and between 7d08abb and 2aef8eb.

⛔ Files ignored due to path filters (8)
  • src/shared/assets/icons/checkbox_small_default.svg is excluded by !**/*.svg and included by src/**
  • src/shared/assets/icons/checkbox_small_disabled.svg is excluded by !**/*.svg and included by src/**
  • src/shared/assets/icons/checkbox_small_pressed.svg is excluded by !**/*.svg and included by src/**
  • src/shared/assets/icons/icon_bookmark.svg is excluded by !**/*.svg and included by src/**
  • src/shared/assets/icons/icon_bookmark_before.svg is excluded by !**/*.svg and included by src/**
  • src/shared/assets/icons/icon_trash.svg is excluded by !**/*.svg and included by src/**
  • src/shared/assets/icons/icon_trash_off.svg is excluded by !**/*.svg and included by src/**
  • src/shared/assets/images/search_img.png is excluded by !**/*.png and included by src/**
📒 Files selected for processing (13)
  • src/app/routes/paths.ts
  • src/app/routes/public-routes.tsx
  • src/pages/bookmark/bookmark-page.css.ts
  • src/pages/bookmark/bookmark-page.tsx
  • src/pages/bookmark/config/bookmark-page.constant.ts
  • src/pages/bookmark/ui/bookmark-checkbox.css.ts
  • src/pages/bookmark/ui/bookmark-checkbox.tsx
  • src/pages/bookmark/ui/bookmark-empty-state.css.ts
  • src/pages/bookmark/ui/bookmark-empty-state.tsx
  • src/pages/bookmark/ui/bookmark-table.css.ts
  • src/pages/bookmark/ui/bookmark-table.tsx
  • src/shared/assets/images/index.ts
  • src/widgets/header/header.tsx

Comment on lines +80 to +94
globalStyle(`${deleteButtonWrap} > button:hover:not(:disabled)`, {
backgroundColor: themeVars.color.blue600,
borderColor: themeVars.color.blue600,
});

globalStyle(`${deleteButtonWrap} > button:active:not(:disabled)`, {
backgroundColor: themeVars.color.blue600,
borderColor: themeVars.color.blue600,
});

globalStyle(`${deleteButtonWrap} > button:disabled`, {
backgroundColor: themeVars.color.blue600,
borderColor: themeVars.color.blue600,
color: themeVars.color.white,
});
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

버튼 상태(hover, active, disabled)에 시각적 피드백이 없어요.

현재 hover, active, disabled 상태 모두 동일한 색상을 사용하고 있어서 사용자에게 시각적 피드백이 제공되지 않아요. UX 개선을 위해 각 상태별로 다른 스타일을 적용하는 것을 권장해요.

♻️ 상태별 스타일 예시
 globalStyle(`${deleteButtonWrap} > button:hover:not(:disabled)`, {
-  backgroundColor: themeVars.color.blue600,
-  borderColor: themeVars.color.blue600,
+  backgroundColor: themeVars.color.blue700, // 또는 더 어두운 톤
+  borderColor: themeVars.color.blue700,
 });

 globalStyle(`${deleteButtonWrap} > button:active:not(:disabled)`, {
-  backgroundColor: themeVars.color.blue600,
-  borderColor: themeVars.color.blue600,
+  backgroundColor: themeVars.color.blue800, // 또는 더 어두운 톤
+  borderColor: themeVars.color.blue800,
 });

 globalStyle(`${deleteButtonWrap} > button:disabled`, {
-  backgroundColor: themeVars.color.blue600,
-  borderColor: themeVars.color.blue600,
+  backgroundColor: themeVars.color.gray300, // 비활성화 상태 표시
+  borderColor: themeVars.color.gray300,
   color: themeVars.color.white,
+  cursor: "not-allowed",
 });
📝 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
globalStyle(`${deleteButtonWrap} > button:hover:not(:disabled)`, {
backgroundColor: themeVars.color.blue600,
borderColor: themeVars.color.blue600,
});
globalStyle(`${deleteButtonWrap} > button:active:not(:disabled)`, {
backgroundColor: themeVars.color.blue600,
borderColor: themeVars.color.blue600,
});
globalStyle(`${deleteButtonWrap} > button:disabled`, {
backgroundColor: themeVars.color.blue600,
borderColor: themeVars.color.blue600,
color: themeVars.color.white,
});
globalStyle(`${deleteButtonWrap} > button:hover:not(:disabled)`, {
backgroundColor: themeVars.color.blue700,
borderColor: themeVars.color.blue700,
});
globalStyle(`${deleteButtonWrap} > button:active:not(:disabled)`, {
backgroundColor: themeVars.color.blue800,
borderColor: themeVars.color.blue800,
});
globalStyle(`${deleteButtonWrap} > button:disabled`, {
backgroundColor: themeVars.color.gray300,
borderColor: themeVars.color.gray300,
color: themeVars.color.white,
cursor: "not-allowed",
});
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/pages/bookmark/bookmark-page.css.ts` around lines 80 - 94, The three
globalStyle rules for `${deleteButtonWrap} > button` currently reuse the same
colors so hover/active/disabled give no visual feedback; update the selectors
(`${deleteButtonWrap} > button:hover:not(:disabled)`, `${deleteButtonWrap} >
button:active:not(:disabled)`, `${deleteButtonWrap} > button:disabled`) to use
distinct themeVars values (e.g., a darker blue like themeVars.color.blue700 for
:hover, a slightly different blue like themeVars.color.blue500 or a pressed
shade for :active, and a muted/gray tone like themeVars.color.gray300 for
:disabled), set appropriate text color for contrast (e.g., themeVars.color.white
on active/hover, muted text color for disabled), and add a disabled cursor
(cursor: not-allowed) for the :disabled rule to provide clear visual and
affordance differences while keeping the existing selectors (`deleteButtonWrap`
and the specific pseudo-classes) intact.

Comment on lines +180 to +188
<Button
variant="secondary"
size="large"
onClick={handleDeleteConfirm}
>
삭제하기
</Button>
<Button variant="primary" size="large" onClick={closeDeleteModal}>
취소하기
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

삭제 모달의 버튼 의미(강조/위험도)가 반대로 배치되어 있습니다.

현재는 "삭제하기"secondary, "취소하기"primary라 파괴적 액션의 시각적 우선순위가 뒤집혀 있습니다. 사용자 오동작 리스크가 있어 수정이 필요합니다.

🛠️ 제안 diff
-          <Button
-            variant="secondary"
-            size="large"
-            onClick={handleDeleteConfirm}
-          >
-            삭제하기
-          </Button>
-          <Button variant="primary" size="large" onClick={closeDeleteModal}>
-            취소하기
-          </Button>
+          <Button variant="secondary" size="large" onClick={closeDeleteModal}>
+            취소하기
+          </Button>
+          <Button
+            variant="primary"
+            size="large"
+            onClick={handleDeleteConfirm}
+          >
+            삭제하기
+          </Button>
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/pages/bookmark/bookmark-page.tsx` around lines 180 - 188, The delete
modal's button styling is reversed: the destructive action (onClick handler
handleDeleteConfirm labeled "삭제하기") is using variant="secondary" while the safe
action (closeDeleteModal "취소하기") uses variant="primary"; update the Button props
so that the "삭제하기" Button uses the destructive/primary visual (e.g.,
variant="primary" or the app's dangerous variant) and the "취소하기" Button uses the
secondary/neutral variant to reflect proper visual priority.

Comment on lines +6 to +26
export const checkbox = style({
appearance: "none",
boxSizing: "border-box",
width: "2.4rem",
height: "2.4rem",

borderRadius: 0,
backgroundColor: "transparent",
backgroundPosition: "center",
backgroundSize: "2.4rem 2.4rem",
backgroundImage: `url("${checkboxDisabled}")`,
cursor: "pointer",
display: "inline-block",
verticalAlign: "middle",
outline: "none",
selectors: {
"&:checked": {
backgroundImage: `url("${checkboxPressed}")`,
},
},
});
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

접근성: 포커스 상태 스타일이 필요해요.

outline: "none"으로 기본 포커스 인디케이터를 제거했는데, 키보드 사용자를 위한 대체 포커스 스타일이 필요해요. 접근성(a11y) 가이드라인을 준수하려면 :focus 또는 :focus-visible 상태에서 시각적 피드백을 제공해야 해요.

🛠️ 포커스 스타일 추가 제안
 export const checkbox = style({
   appearance: "none",
   boxSizing: "border-box",
   width: "2.4rem",
   height: "2.4rem",

   borderRadius: 0,
   backgroundColor: "transparent",
   backgroundPosition: "center",
   backgroundSize: "2.4rem 2.4rem",
   backgroundImage: `url("${checkboxDisabled}")`,
   cursor: "pointer",
   display: "inline-block",
   verticalAlign: "middle",
   outline: "none",
   selectors: {
     "&:checked": {
       backgroundImage: `url("${checkboxPressed}")`,
     },
+    "&:focus-visible": {
+      boxShadow: "0 0 0 2px `#3182F6`", // 또는 themeVars.color.blue600
+    },
   },
 });
📝 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 checkbox = style({
appearance: "none",
boxSizing: "border-box",
width: "2.4rem",
height: "2.4rem",
borderRadius: 0,
backgroundColor: "transparent",
backgroundPosition: "center",
backgroundSize: "2.4rem 2.4rem",
backgroundImage: `url("${checkboxDisabled}")`,
cursor: "pointer",
display: "inline-block",
verticalAlign: "middle",
outline: "none",
selectors: {
"&:checked": {
backgroundImage: `url("${checkboxPressed}")`,
},
},
});
export const checkbox = style({
appearance: "none",
boxSizing: "border-box",
width: "2.4rem",
height: "2.4rem",
borderRadius: 0,
backgroundColor: "transparent",
backgroundPosition: "center",
backgroundSize: "2.4rem 2.4rem",
backgroundImage: `url("${checkboxDisabled}")`,
cursor: "pointer",
display: "inline-block",
verticalAlign: "middle",
outline: "none",
selectors: {
"&:checked": {
backgroundImage: `url("${checkboxPressed}")`,
},
"&:focus-visible": {
boxShadow: `0 0 0 2px ${themeVars.color.blue600}`,
},
},
});
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/pages/bookmark/ui/bookmark-checkbox.css.ts` around lines 6 - 26, The
checkbox style currently removes the browser focus indicator via outline: "none"
on the exported checkbox style; add explicit focus styles for keyboard users by
updating the selectors in the checkbox style to include &:focus and
&:focus-visible (or both) and provide a visible, high-contrast indicator (for
example a visible outline, ring or box-shadow with sufficient contrast and some
offset) while keeping the rest of the visual design intact; ensure the focus
style is applied only on keyboard focus (use :focus-visible if supported) and
matches the checkboxPressed/checkboxDisabled sizes so the focus ring is clearly
visible around the control.

Comment on lines +84 to +90
<span
className={styles.connectionStatus({
connected: row.isConnected,
})}
>
연결
</span>
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

연결 상태 텍스트가 고정되어 있어 오표시됩니다.

Line 89가 항상 "연결"이라 row.isConnected === false인 행도 연결로 보입니다. boolean 값에 따라 텍스트도 분기해야 합니다.

🛠️ 제안 diff
               <span
                 className={styles.connectionStatus({
                   connected: row.isConnected,
                 })}
               >
-                연결
+                {row.isConnected ? "연결" : "미연결"}
               </span>
📝 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
<span
className={styles.connectionStatus({
connected: row.isConnected,
})}
>
연결
</span>
<span
className={styles.connectionStatus({
connected: row.isConnected,
})}
>
{row.isConnected ? "연결" : "미연결"}
</span>
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/pages/bookmark/ui/bookmark-table.tsx` around lines 84 - 90, The span
always renders the fixed text "연결" causing false positives; update the JSX in
bookmark-table.tsx so the span that uses styles.connectionStatus({ connected:
row.isConnected }) also conditionally renders its inner text based on
row.isConnected (e.g., show "연결" when row.isConnected is true and an appropriate
"미연결"/"연결 끊김" when false) so the displayed label matches the boolean state.

Comment on lines +20 to +24
const location = useLocation();
const isBookmarkRoute = location.pathname === ROUTES.BOOKMARK;
const { data } = useGetProfile({
enabled: isLoggedIn && !isBookmarkRoute,
});
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Description: 프로필 API 에러의 근본 원인을 파악하기 위해 관련 코드 확인

# useGetProfile 훅의 전체 구현 확인
echo "=== useGetProfile 구현 ==="
fd "use-get-my-profile" --extension ts --exec cat {}

# getProfile API 함수 확인
echo -e "\n=== getProfile API 함수 ==="
rg -n "getProfile" --type ts -A 5 -B 2

Repository: TEAM-COMFIT/COMFIT-CLIENT

Length of output: 1846


🏁 Script executed:

# 헤더 컴포넌트의 전체 코드 확인
head -100 src/widgets/header/header.tsx | cat -n

# 59번 라인 근처의 데이터 렌더링 방식 확인
sed -n '50,70p' src/widgets/header/header.tsx | cat -n

Repository: TEAM-COMFIT/COMFIT-CLIENT

Length of output: 3288


🏁 Script executed:

# 북마크 라우트가 왜 프로필 조회를 비활성화하는지 확인
rg -n "ROUTES\.BOOKMARK|bookmark" src/widgets/header/header.tsx -B 3 -A 3

# header.tsx 파일 전체 크기 확인
wc -l src/widgets/header/header.tsx

# 프로필 데이터 사용 패턴 확인 (오류 처리, 로딩 상태 등)
grep -n "data\|error\|loading\|isLoading" src/widgets/header/header.tsx

Repository: TEAM-COMFIT/COMFIT-CLIENT

Length of output: 484


북마크 라우트에서 프로필 조회 비활성화 - 근본 원인 해결 필요

현재 구현에서 몇 가지 UX 문제가 있습니다:

  1. 캐시 데이터 표시 문제: useGetProfilestaleTime: Infinity를 사용하므로, 쿼리가 비활성화되어도 캐시된 프로필 데이터가 계속 렌더링됩니다. 이는 북마크 페이지에서 이전의 캐시된 사용자명을 표시하게 됩니다.

  2. undefined님 표시: 사용자가 로그인 후 첫 번째 작업으로 북마크 페이지를 방문하는 경우, 캐시 데이터가 없어서 {data?.name}님undefined님으로 표시됩니다.

  3. 폴백 UI 부재: 현재 데이터 로딩 상태나 오류 상황에 대한 처리가 없습니다.

임시 처리로 표시하셨다면, 머지 전에 근본 원인을 해결하시기를 권장합니다. 예를 들어:

  • 북마크 라우트에서도 프로필을 표시해야 한다면, 쿼리 비활성화를 제거하고 근본 문제를 해결하세요.
  • 의도적으로 비활성화하신 거라면, 적절한 폴백 UI(예: 스켈레톤 로더, 기본값)를 추가하거나 조건부로 렌더링을 제어하세요.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/widgets/header/header.tsx` around lines 20 - 24, The profile query is
disabled on the bookmark route which, combined with useGetProfile's staleTime:
Infinity, causes stale or undefined names to render; either remove the
conditional disabling (remove the enabled: isLoggedIn && !isBookmarkRoute) so
useGetProfile always runs and fixes the stale cache, or keep it disabled but
guard rendering of {data?.name} by checking data and query states from
useGetProfile (isLoading/isError) and show a skeleton/default label (e.g.,
"회원님") or a loader when data is missing; reference useGetProfile,
isBookmarkRoute, ROUTES.BOOKMARK, isLoggedIn and data?.name to locate and
implement the change.

Copy link
Copy Markdown
Collaborator

@odukong odukong left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

일단은 가장 중요하다고 생각드는 부분에 대해 몇 가지 코드리뷰 남겨 놓았습니다
확인해보시고 필요한 부분은 수정부탁드리겠습니다! ♪(´▽`)

얼마 남지 않은 1차 스프린트!!! 화이팅입니다 ~~ 🪽

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bookmark-page.constants.ts config 파일이나 bookmark-page에서 사용되는 ui 파일들은 features
옮겨주는게 어떨까요?
아무래도 pages 폴더는 페이지 파일 자체만 위치하는 역할을 하다 보니 features 폴더로 옮긴 후
저번에 논의했던 방향대로 index.ts 진입점을 둔 뒤, pages에서 호출해 사용하도록 하면 좋을 것 같아요!

Copy link
Copy Markdown
Collaborator Author

@qowjdals23 qowjdals23 Mar 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

좋은 의견 감사합니다!

말씀해주신 것처럼 저번에 논의했던 방향대로 보면 관련 UI나 constants는 features로 분리하는 게 맞는 것 같네요..!! index.ts 진입점 두는 형태로 정리하겠습니다 !!!

"&:checked": {
backgroundImage: `url("${checkboxPressed}")`,
},
},
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

체크박스는 컴포넌트로 분리됩니다. 클릭 여부에 따라 backgroundImage를 이미지(SVG)를 변경하는 방향은 컴포넌트의 의미에 부합하지 않는다고 생각해요!
그래서 지금 아이콘 파일들도 index.ts에서 react컴포넌트로 정의한 후 사용되는게 아니라 파일을 직접적으로 참조하게 되면서 유지보수에 어려움이 생길 수도 있을 것 같아요. (아이콘 파일을 직접적으로 참조하지 않도록 icon/index.ts에 먼저 정의해주세요)

특히, icon_trashicon_trash_off는 형태는 같고, 색만 다른 아이콘입니다. 이런 경우에는 여러 케이스의 파일을 다운 받는 것보다, 하나의 svg파일만 다운해 svg파일 코드에서 fill 속성을 currentColor로 변경한 후에 style파일에서 color속성 변경을 통해 아이콘 색상을 변경해주도록 합시다 (‾◡◝)

Comment on lines +30 to +36
<div className={styles.emptySection}>
<div className={styles.emptyContent}>
<img className={styles.emptyImage} src={image} alt={alt} />
<p className={styles.emptyTitle}>{title}</p>
<p className={styles.emptyDescription}>{description}</p>
</div>
</div>
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

emptySectionbookmark-pagetableSection과 스타일을 공유할 수 있는 부분이라고 생각해요!
이미지, 타이틀, 설명을 감싸기 위해 이미 emptyContent가 존재하는데, 한번 더 emptySection으로 감싸줄 필요는 없는 것 같아요!

emptySection은 제거한 후 tableSection에서

display: "flex",
alignItems: "center",
justifyContent: "center",
height: "48.5rem",

코드를 추가해주는 것이 좋을 것 같습니다!!

Comment on lines +95 to +112
{Array.from({ length: placeholderRowCount }).map((_, idx) => (
<tr key={`placeholder-${idx}`} aria-hidden="true">
{Array.from({ length: TABLE_COLUMN_COUNT }).map((__, colIdx) => {
let alignClass = styles.centerCell;
if (colIdx === 0) alignClass = styles.checkboxCell;
if (colIdx === 1) alignClass = styles.leftCell;

return (
<td
key={`placeholder-cell-${idx}-${colIdx}`}
className={`${styles.bodyCell} ${alignClass} ${styles.placeholderCell}`}
>
&nbsp;
</td>
);
})}
</tr>
))}
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BookmarkTable에서 4개 열을 고정적으로 만들어주기 위해 구현된 코드같네요!

기능 명세서에는 정확히 명시되어 있지 않지만, 제 개인적인 의견으론 아무 데이터 값이 없는데 빈 row를 보여주기 보다 존재하는 n개의 열(n<4)에 대해서만 row을 보여주는 것이 UX적으로 더 좋지 않나..하는 생각이 들어 해당 코드는 제거해주는 것이 어떤가를.. 제안드립니다 ⚡

추가적으로 위에서 말했 듯 bookmark-empty-state.tsxemptySection을 제거하게 되면,
4개 열이 아닌 경우에는 center로 위치하게 될 텐데, 이런 경우에는 bookmark-table.css.tstable 스타일에 alignSelf:' flex-start'를 추가해주면 될 것 같습니다!

Comment on lines +174 to +179
<Modal isOpen={isDeleteModalOpen} onClose={closeDeleteModal}>
<Modal.XButton />
<Modal.Content>
<Modal.Title>선택한 북마크를 삭제할까요?</Modal.Title>
</Modal.Content>
<Modal.Buttons>
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Modal 컴포넌트는 현재 올라와있는 이슈가 머지되면 다시 한 번 수정이 되긴 해야 합니다!

그 전에 모달은 지역 state를 통해 open, close 액션이 수행되는 것이 아닌 modalStore.open()을 통해서 액션이 수행되어야 합니다. 이 부분은 이 이슈를 참고해주세요!

또한 북마크 페이지에서는 Modal 구성요소를 직접 조립하는 대신, 자주 사용되는 모달을 미리 선언해 둔ModalBasic 컴포넌트를 사용해주세요!

Comment on lines +167 to +171
<Pagination
currentPage={resolvedCurrentPage}
totalPage={paginationTotalPage}
onPageChange={handlePageChange}
/>
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

북마크 한 기업이 없다면, 페이지네이션될 데이터 리스트가 존재하지 않기 때문에 Pagination이 보여져선 안될 것 같아요! 조건문 렌더링을 추가해주세요!

그리고 검색 결과가 없는 경우에도 Pagination이 보여져선 안되는데 피그마에서는 있는 걸로 나오네요..? 그치만 마찬가지로 검색 결과가 없는 경우에도 해당 컴포넌트는 보여선 안되긴 때문에 위의 table처럼 조건문을 추가해주세요!

Comment on lines +31 to +35
<div className={styles.emptyContent}>
<img className={styles.emptyImage} src={image} alt={alt} />
<p className={styles.emptyTitle}>{title}</p>
<p className={styles.emptyDescription}>{description}</p>
</div>
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

empty 상황에서 나오는 이미지의 크기가 피그마 스펙과 다른것 같습니다! 로컬에서 실행해서 보면 코네(였던가..)가 찌그러져서 보이네요!

img에 직접적으로 width를 적용하기 보다, img를 감싸는 div태그를 추가한 후에 emptyImage 스타일을 적용해주시고, (검색 결과가 없는 경우, 북마크가 없는 경우) 두 이미지는 height는 다르지만, 같은 width를 공유하기 때문에 width: "36rem"만 적용해 비율을 맞춰주시면 될 것 같습니다!

Comment on lines +20 to +25
const [rows, setRows] = useState(BOOKMARK_MOCK_ROWS);
const [searchInput, setSearchInput] = useState("");
const [keyword, setKeyword] = useState("");
const [currentPage, setCurrentPage] = useState(1);
const [selectedIds, setSelectedIds] = useState<number[]>([]);
const [isDeleteModalOpen, setIsDeleteModalOpen] = useState(false);
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

현재 mock 데이터를 사용하고 있기 때문에 필터링 기능 구현에 있어 많은 상태들이 사용되고 있는 것 같아요.
이 부분에 대해서는 추후 API 연동이 진행되면 자연스럽게 제거될 부분이라 리뷰를 남기진 않겠습니다!

그렇지만 검색키워드현재 페이지는 state가 아닌 useSearchParams훅을 사용하여 url에 상태를 저장해둔다면, 새로고침 시에도 페이지네이션+검색결과가 유지되어 사용자 경험 개선이 도움이 될 것이라고 생각합니다!

Comment on lines +73 to +88
const toggleAll = (checked: boolean) => {
if (checked) {
setSelectedIds((prev) => Array.from(new Set([...prev, ...visibleIds])));
return;
}

setSelectedIds((prev) => prev.filter((id) => !visibleIds.includes(id)));
};

const toggleRow = (rowId: number, checked: boolean) => {
setSelectedIds((prev) =>
checked
? Array.from(new Set([...prev, rowId]))
: prev.filter((id) => id !== rowId)
);
};
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

selectedIdsnumber[] 대신 Set< number > 로 관리하는 것이 좋을 것 같습니다!

현재 toggleAll, toggleRow 두 함수 모두 북마크 id의 중복 선택을 방지하기 위해 Set을 사용하고 있는데, 배열을 spread한 뒤 Set으로 중복을 제거하고 다시 배열로 변환하는 과정을 반복하고 있어 불필요한 변환 비용이 발생한다고 생각합니다!
처음부터 Set로 state를 관리하면 변환 과정이 없어 더 코드가 간단해질 것 같아요!

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

♻️ Duplicate comments (3)
src/pages/bookmark/bookmark-page.tsx (2)

166-171: ⚠️ Potential issue | 🟡 Minor

빈 상태에서는 페이지네이션을 숨겨주세요.

BookmarkEmptyState가 렌더링되는 경우에도 항상 Pagination이 남아 있어 empty/no-result 화면에 의미 없는 1페이지가 표시됩니다. empty state일 때는 이 섹션 자체를 조건부 렌더링하는 편이 자연스럽습니다.

As per coding guidelines, src/pages/**: "UX 흐름과 접근성(a11y)을 고려한 구조인지 점검".

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/pages/bookmark/bookmark-page.tsx` around lines 166 - 171, Hide the
pagination section when the page is in the empty/no-result state by
conditionally rendering the <section className={styles.paginationSection}> with
<Pagination ...> only when the bookmarks list is non-empty (i.e., when
BookmarkEmptyState is not rendered). Locate the logic that renders
BookmarkEmptyState and use the same condition (or derived flag) to gate
rendering of Pagination (props: resolvedCurrentPage, paginationTotalPage,
handlePageChange) so the pagination row is omitted for empty results.

180-193: ⚠️ Potential issue | 🟠 Major

모달의 안전 액션과 파괴 액션 강조가 뒤바뀌어 있습니다.

지금은 삭제하기secondary, 취소하기primary라 시각적 우선순위가 반대로 보입니다. 버튼 순서도 취소하기 → 삭제하기로 두는 편이 실수 가능성을 줄입니다.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/pages/bookmark/bookmark-page.tsx` around lines 180 - 193, The modal
currently emphasizes the safe action instead of the destructive one and has the
wrong button order; swap the two Button elements so the cancel action (Button
with onClick={handleCloseDeleteModal}) appears first and the delete action
(Button with onClick={handleDeleteConfirm}) appears second, and invert their
variants so 삭제하기 uses variant="primary" (destructive emphasis) and 취소하기 uses
variant="secondary" (safe/less-emphasized); keep the existing handlers
handleDeleteConfirm and handleCloseDeleteModal unchanged.
src/pages/bookmark/ui/bookmark-table.tsx (1)

79-85: ⚠️ Potential issue | 🟠 Major

연결 상태 텍스트가 실제 값과 다르게 표시됩니다.

styles.connectionStatus({ connected: row.isConnected })는 boolean을 사용하지만 span 텍스트는 항상 "연결"이라 false 행도 오표시됩니다. 현재 mock data에도 isConnected: false 행이 있어 첫 화면부터 잘못된 상태가 노출됩니다.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/pages/bookmark/ui/bookmark-table.tsx` around lines 79 - 85, The span
always shows the literal text "연결" even when the status prop is false; update
the UI in bookmark-table.tsx so the displayed label reflects row.isConnected
(use a conditional rendering/ternary for the span children based on
row.isConnected) while keeping the existing className usage of
styles.connectionStatus({ connected: row.isConnected }); ensure you reference
row.isConnected and styles.connectionStatus to locate and fix the element so
false rows show the correct text (e.g., "미연결" or the appropriate label).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/pages/bookmark/bookmark-page.tsx`:
- Around line 69-71: Current behavior preserves selectedIds across pages causing
invisible selections to be deleted; in handlePageChange(page: number) clear
per-page selections by resetting selectedIds (call setSelectedIds([]) or
setSelectedIds(new Set()) to match your state shape) when changing pages, and
ensure any delete button state/read-only check uses the cleared selectedIds;
alternatively, if you intend cross-page selection, add a visible selection count
in the header so users see selections across pages.
- Around line 57-67: The search handler (handleSearch) trims the submitted value
but doesn't update the input state, causing the visible input and active filter
(setKeyword) to diverge; modify handleSearch to also update the input state
(call setSearchInput(trimmedValue)) — e.g., compute trimmedValue, if it's not
all-whitespace proceed to setSearchInput(trimmedValue), then
setKeyword(trimmedValue), setCurrentPage(1), and setSelectedIds([]) so the UI
input and filter stay in sync.

In `@src/pages/bookmark/ui/bookmark-empty-state.tsx`:
- Around line 11-23: The IMAGE alt text in EMPTY_STATE_CONTENT (entries bookmark
and search) is duplicating the visible heading (title), causing redundant screen
reader output; change the alt properties for decorative illustrations to an
empty string (alt: "") and keep the visible title/description as the semantic
text (title/description) so the <h2> and copy remain readable; update both
EMPTY_STATE_CONTENT.bookmark.alt and EMPTY_STATE_CONTENT.search.alt accordingly
and ensure the UI that renders image uses the alt field from this constant.

---

Duplicate comments:
In `@src/pages/bookmark/bookmark-page.tsx`:
- Around line 166-171: Hide the pagination section when the page is in the
empty/no-result state by conditionally rendering the <section
className={styles.paginationSection}> with <Pagination ...> only when the
bookmarks list is non-empty (i.e., when BookmarkEmptyState is not rendered).
Locate the logic that renders BookmarkEmptyState and use the same condition (or
derived flag) to gate rendering of Pagination (props: resolvedCurrentPage,
paginationTotalPage, handlePageChange) so the pagination row is omitted for
empty results.
- Around line 180-193: The modal currently emphasizes the safe action instead of
the destructive one and has the wrong button order; swap the two Button elements
so the cancel action (Button with onClick={handleCloseDeleteModal}) appears
first and the delete action (Button with onClick={handleDeleteConfirm}) appears
second, and invert their variants so 삭제하기 uses variant="primary" (destructive
emphasis) and 취소하기 uses variant="secondary" (safe/less-emphasized); keep the
existing handlers handleDeleteConfirm and handleCloseDeleteModal unchanged.

In `@src/pages/bookmark/ui/bookmark-table.tsx`:
- Around line 79-85: The span always shows the literal text "연결" even when the
status prop is false; update the UI in bookmark-table.tsx so the displayed label
reflects row.isConnected (use a conditional rendering/ternary for the span
children based on row.isConnected) while keeping the existing className usage of
styles.connectionStatus({ connected: row.isConnected }); ensure you reference
row.isConnected and styles.connectionStatus to locate and fix the element so
false rows show the correct text (e.g., "미연결" or the appropriate label).
🪄 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: ASSERTIVE

Plan: Pro

Run ID: 9a595a5d-aaec-4452-827a-35ba5f478c42

📥 Commits

Reviewing files that changed from the base of the PR and between 2aef8eb and 14e6a7e.

📒 Files selected for processing (5)
  • src/pages/bookmark/bookmark-page.css.ts
  • src/pages/bookmark/bookmark-page.tsx
  • src/pages/bookmark/config/bookmark-page.constant.ts
  • src/pages/bookmark/ui/bookmark-empty-state.tsx
  • src/pages/bookmark/ui/bookmark-table.tsx

Comment on lines +57 to +67
const handleSearch = (value: string) => {
const trimmedValue = value.trim();

if (value.length > 0 && trimmedValue.length === 0) {
return;
}

setKeyword(trimmedValue);
setCurrentPage(1);
setSelectedIds([]);
};
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

검색 제출 후 입력창과 실제 필터 조건이 어긋날 수 있습니다.

필터링은 trimmedValue로 수행하는데 searchInput은 그대로 둬서, 앞뒤 공백이 있는 값이나 공백-only 제출 후에 입력창과 결과 목록이 서로 다른 상태가 됩니다. 제출 시 입력값도 trim 결과로 동기화하는 편이 안전합니다.

수정 예시
 const handleSearch = (value: string) => {
   const trimmedValue = value.trim();
-
-  if (value.length > 0 && trimmedValue.length === 0) {
-    return;
-  }
+  setSearchInput(trimmedValue);
 
   setKeyword(trimmedValue);
   setCurrentPage(1);
   setSelectedIds([]);
 };
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/pages/bookmark/bookmark-page.tsx` around lines 57 - 67, The search
handler (handleSearch) trims the submitted value but doesn't update the input
state, causing the visible input and active filter (setKeyword) to diverge;
modify handleSearch to also update the input state (call
setSearchInput(trimmedValue)) — e.g., compute trimmedValue, if it's not
all-whitespace proceed to setSearchInput(trimmedValue), then
setKeyword(trimmedValue), setCurrentPage(1), and setSelectedIds([]) so the UI
input and filter stay in sync.

Comment on lines +69 to +71
const handlePageChange = (page: number) => {
setCurrentPage(page);
};
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

페이지를 넘기면 화면 밖 선택이 남아 삭제 대상이 숨겨집니다.

현재는 페이지 이동 시 selectedIds를 유지해서, 새 페이지에 체크된 행이 하나도 없어도 삭제 버튼이 활성화될 수 있습니다. 이 상태에서 삭제하면 사용자가 보고 있지 않은 행이 제거될 수 있으니, 페이지 단위 선택 UX라면 페이지 전환 때 선택을 비우거나 상단에 선택 개수를 별도로 노출해 주세요.

페이지 단위 선택 UX라면
 const handlePageChange = (page: number) => {
   setCurrentPage(page);
+  setSelectedIds([]);
 };
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/pages/bookmark/bookmark-page.tsx` around lines 69 - 71, Current behavior
preserves selectedIds across pages causing invisible selections to be deleted;
in handlePageChange(page: number) clear per-page selections by resetting
selectedIds (call setSelectedIds([]) or setSelectedIds(new Set()) to match your
state shape) when changing pages, and ensure any delete button state/read-only
check uses the cleared selectedIds; alternatively, if you intend cross-page
selection, add a visible selection count in the header so users see selections
across pages.

Comment on lines +11 to +23
const EMPTY_STATE_CONTENT = {
bookmark: {
image: ERROR,
alt: "북마크한 기업이 없습니다",
title: "북마크한 기업이 없습니다",
description: "관심 있는 기업을 탐색하고 북마크해 보세요.",
},
search: {
image: SEARCH_IMG,
alt: "검색 결과가 없습니다",
title: "검색 결과가 없습니다",
description: "다른 키워드로 다시 검색해 보세요.",
},
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

빈 상태 일러스트는 장식용으로 처리하는 편이 좋습니다.

이미지 alt<h2>가 같은 문구라 스크린리더가 상태를 중복해서 읽게 됩니다. 바로 아래 텍스트가 의미를 충분히 전달하므로 여기서는 alt=""로 두는 편이 더 적절합니다.

As per coding guidelines, src/**/ui/**/*.tsx: "접근성(a11y) 준수 여부 점검".

Also applies to: 33-33

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/pages/bookmark/ui/bookmark-empty-state.tsx` around lines 11 - 23, The
IMAGE alt text in EMPTY_STATE_CONTENT (entries bookmark and search) is
duplicating the visible heading (title), causing redundant screen reader output;
change the alt properties for decorative illustrations to an empty string (alt:
"") and keep the visible title/description as the semantic text
(title/description) so the <h2> and copy remain readable; update both
EMPTY_STATE_CONTENT.bookmark.alt and EMPTY_STATE_CONTENT.search.alt accordingly
and ensure the UI that renders image uses the alt field from this constant.

@u-zzn
Copy link
Copy Markdown
Collaborator

u-zzn commented Mar 31, 2026

체크박스 / 테이블 / 빈 상태를 페이지에서 적절히 분리해두셔서, 북마크 페이지 UI 구조가 한눈에 잘 들어오는 것 같습니다 🙂
특히 BookmarkTable, BookmarkEmptyState, BookmarkCheckbox로 역할을 나눈 덕분에 이후 API 연결하거나 상태 로직 보강할 때도 비교적 손대기 쉬운 구조로 보였어요!

수고 많으셨습니다 ☺️ 수정하면 좋을 것 같은 몇가지 부분만 아래에 comment 남겨두겠습니다 :)

Comment on lines +166 to +172
<section className={styles.paginationSection}>
<Pagination
currentPage={resolvedCurrentPage}
totalPage={paginationTotalPage}
onPageChange={handlePageChange}
/>
</section>
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

검색 결과가 empty이거나 전체 북마크가 empty인 경우에도 pagination이 항상 노출되는 구조라, '실제 화면에서 1페이지가 같이 보이면 조금 어색하지 않나?' 라고 개인적으로 생각했던 것 같아요!

isBookmarkEmpty || isSearchResultEmpty일 때는 pagination 자체를 숨기거나, 실제 row가 있을 때만 렌더링하는 쪽도 한 번 고려해보면 좋을 것 같은데, 이건 저희끼리 결정할 수 있는건 아니니 디쌤들이랑 같이 이야기해서 정해보는거 어떨까요 ?? ☺️

onClick={handleOpenDeleteModal}
aria-label="북마크 삭제"
>
<IconTrashOff className={styles.trashIcon} aria-hidden="true" />
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

아이콘 에셋에 icon_trash.svg, icon_trash_off.svg가 둘 다 있는 걸 보면 '선택 여부에 따라 아이콘도 바뀌는 UX를 의도하신 걸까?' 싶었습니다!

지금은 버튼 enabled/disabled 여부와 관계없이 IconTrashOff만 쓰고 있어서, 선택된 row가 있을 때는 active 아이콘으로 바꿔주는 게 더 자연스러울 수도 있을 것 같아요 :)

Comment on lines +20 to +24
outline: "none",
selectors: {
"&:checked": {
backgroundImage: `url("${checkboxPressed}")`,
},
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

찾아보니 outline을 제거하는 경우에는 키보드 사용자에게 포커스 위치를 명확하게 전달할 수 있도록 :focus-visible 스타일을 별도로 정의해주는 게 접근성 측면에서 권장된다고 하더라구요 🙂

custom checkbox로 잘 분리해주신 것 같은데, 현재 outline: none만 있고 :focus-visible 대응은 따로 없어서
키보드 탐색 시 포커스가 잘 안 보일 수 있을 것 같습니다!

간단하게라도 focus 스타일 하나 추가해두면 재사용 컴포넌트로서 완성도가 더 올라갈 것 같습니다 :)

Comment on lines +60 to +62
if (value.length > 0 && trimmedValue.length === 0) {
return;
}
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

공백만 입력한 검색어는 막는 의도 좋네요! :)

다만 현재 로직을 보면 태그한 이 조건 때문에 " "처럼 공백만 입력한 경우에는 keyword 상태가 갱신되지 않고 그대로 유지되는 흐름인 것 같아요.

이 상태에서 사용자는 “검색어를 지웠다”라고 인식할 가능성이 높은데, 실제로는 이전 keyword 기준으로 필터링이 계속 유지되다 보니 UI 상에서는 검색이 초기화되지 않은 것처럼 느껴질 수도 있을 것 같아요.

의도하신 UX가 아니라면, 공백만 입력된 경우에는 early return 대신

setKeyword("");

로 명시적으로 초기화해주는 쪽이 사용자 기대와 더 맞을 것 같은데, 어떻게 생각하세요 ?? ☺️

예를 들어, 검색어 전체 삭제 → 전체 리스트 다시 보이기 이런 경우가 있을 것 같아요!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

정민🍐 🌟FEAT 새 기능 추가

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Feat] 북마크 페이지 구현

3 participants