[Feat] 북마크 페이지 구현#163
Merged
Merged
Conversation
|
Caution Review failedAn error occurred during the review process. Please try again later. 📝 WalkthroughWalkthrough북마크 기능을 추가하는 PR로, 새로운 Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant BookmarkPage as BookmarkPage<br/>(State Management)
participant BookmarkTable as BookmarkTable<br/>(Display)
participant Pagination as Pagination<br/>(Navigation)
participant ModalStore as ModalStore<br/>(Delete Modal)
User->>BookmarkPage: 페이지 로드
BookmarkPage->>BookmarkPage: 검색 params 읽기<br/>(keyword, page)
BookmarkPage->>BookmarkPage: 필터링 & 페이징 계산
User->>BookmarkTable: 행 선택/체크박스
BookmarkTable->>BookmarkPage: onToggleRow(rowId)
BookmarkPage->>BookmarkPage: selectedIds 업데이트
User->>BookmarkPage: 검색 입력
BookmarkPage->>BookmarkPage: URL params 업데이트
BookmarkPage->>BookmarkPage: rows 필터링
BookmarkPage->>BookmarkTable: filtered rows 전달
User->>Pagination: 페이지 변경
Pagination->>BookmarkPage: page 쿼리 param 변경
BookmarkPage->>BookmarkTable: 현재 페이지 행 렌더링
User->>BookmarkPage: 삭제 버튼 클릭
BookmarkPage->>ModalStore: 삭제 모달 표시
User->>ModalStore: 삭제 확인
ModalStore->>BookmarkPage: 삭제 이벤트
BookmarkPage->>BookmarkPage: 선택된 행 제거<br/>rows 업데이트
BookmarkPage->>BookmarkTable: 업데이트된 rows 렌더링
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45분 이 PR은 여러 새로운 컴포넌트, 복잡한 상태 관리 로직(필터링, 페이징, 선택, 삭제), 스타일 모듈, 라우팅 통합을 포함하는 실질적인 새 기능으로, 다양한 관심사를 다루고 있어 신중한 검토가 필요합니다. Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ 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. Comment |
🚀 빌드 결과✅ 린트 검사 완료 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 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
⛔ Files ignored due to path filters (8)
src/shared/assets/icons/checkbox_small_default.svgis excluded by!**/*.svgand included bysrc/**src/shared/assets/icons/checkbox_small_disabled.svgis excluded by!**/*.svgand included bysrc/**src/shared/assets/icons/checkbox_small_pressed.svgis excluded by!**/*.svgand included bysrc/**src/shared/assets/icons/icon_bookmark.svgis excluded by!**/*.svgand included bysrc/**src/shared/assets/icons/icon_bookmark_before.svgis excluded by!**/*.svgand included bysrc/**src/shared/assets/icons/icon_trash.svgis excluded by!**/*.svgand included bysrc/**src/shared/assets/icons/icon_trash_off.svgis excluded by!**/*.svgand included bysrc/**src/shared/assets/images/search_img.pngis excluded by!**/*.pngand included bysrc/**
📒 Files selected for processing (13)
src/app/routes/paths.tssrc/app/routes/public-routes.tsxsrc/pages/bookmark/bookmark-page.css.tssrc/pages/bookmark/bookmark-page.tsxsrc/pages/bookmark/config/bookmark-page.constant.tssrc/pages/bookmark/ui/bookmark-checkbox.css.tssrc/pages/bookmark/ui/bookmark-checkbox.tsxsrc/pages/bookmark/ui/bookmark-empty-state.css.tssrc/pages/bookmark/ui/bookmark-empty-state.tsxsrc/pages/bookmark/ui/bookmark-table.css.tssrc/pages/bookmark/ui/bookmark-table.tsxsrc/shared/assets/images/index.tssrc/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, | ||
| }); |
There was a problem hiding this comment.
🧹 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
+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}")`, | ||
| }, | ||
| }, | ||
| }); |
There was a problem hiding this comment.
접근성: 포커스 상태 스타일이 필요해요.
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> |
There was a problem hiding this comment.
연결 상태 텍스트가 고정되어 있어 오표시됩니다.
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.
odukong
requested changes
Mar 27, 2026
Collaborator
There was a problem hiding this comment.
bookmark-page.constants.ts config 파일이나 bookmark-page에서 사용되는 ui 파일들은 features로
옮겨주는게 어떨까요?
아무래도 pages 폴더는 페이지 파일 자체만 위치하는 역할을 하다 보니 features 폴더로 옮긴 후
저번에 논의했던 방향대로 index.ts 진입점을 둔 뒤, pages에서 호출해 사용하도록 하면 좋을 것 같아요!
Collaborator
Author
There was a problem hiding this comment.
좋은 의견 감사합니다!
말씀해주신 것처럼 저번에 논의했던 방향대로 보면 관련 UI나 constants는 features로 분리하는 게 맞는 것 같네요..!! index.ts 진입점 두는 형태로 정리하겠습니다 !!!
| "&:checked": { | ||
| backgroundImage: `url("${checkboxPressed}")`, | ||
| }, | ||
| }, |
Collaborator
There was a problem hiding this comment.
체크박스는 컴포넌트로 분리됩니다. 클릭 여부에 따라 backgroundImage를 이미지(SVG)를 변경하는 방향은 컴포넌트의 의미에 부합하지 않는다고 생각해요!
그래서 지금 아이콘 파일들도 index.ts에서 react컴포넌트로 정의한 후 사용되는게 아니라 파일을 직접적으로 참조하게 되면서 유지보수에 어려움이 생길 수도 있을 것 같아요. (아이콘 파일을 직접적으로 참조하지 않도록 icon/index.ts에 먼저 정의해주세요)
특히, icon_trash나 icon_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> |
Collaborator
There was a problem hiding this comment.
emptySection은 bookmark-page의 tableSection과 스타일을 공유할 수 있는 부분이라고 생각해요!
이미지, 타이틀, 설명을 감싸기 위해 이미 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}`} | ||
| > | ||
| | ||
| </td> | ||
| ); | ||
| })} | ||
| </tr> | ||
| ))} |
Collaborator
There was a problem hiding this comment.
BookmarkTable에서 4개 열을 고정적으로 만들어주기 위해 구현된 코드같네요!
기능 명세서에는 정확히 명시되어 있지 않지만, 제 개인적인 의견으론 아무 데이터 값이 없는데 빈 row를 보여주기 보다 존재하는 n개의 열(n<4)에 대해서만 row을 보여주는 것이 UX적으로 더 좋지 않나..하는 생각이 들어 해당 코드는 제거해주는 것이 어떤가를.. 제안드립니다 ⚡
추가적으로 위에서 말했 듯 bookmark-empty-state.tsx의 emptySection을 제거하게 되면,
4개 열이 아닌 경우에는 center로 위치하게 될 텐데, 이런 경우에는 bookmark-table.css.ts의 table 스타일에 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> |
Collaborator
There was a problem hiding this comment.
Modal 컴포넌트는 현재 올라와있는 이슈가 머지되면 다시 한 번 수정이 되긴 해야 합니다!
그 전에 모달은 지역 state를 통해 open, close 액션이 수행되는 것이 아닌 modalStore.open()을 통해서 액션이 수행되어야 합니다. 이 부분은 이 이슈를 참고해주세요!
또한 북마크 페이지에서는 Modal 구성요소를 직접 조립하는 대신, 자주 사용되는 모달을 미리 선언해 둔ModalBasic 컴포넌트를 사용해주세요!
Comment on lines
+167
to
+171
| <Pagination | ||
| currentPage={resolvedCurrentPage} | ||
| totalPage={paginationTotalPage} | ||
| onPageChange={handlePageChange} | ||
| /> |
Collaborator
There was a problem hiding this comment.
북마크 한 기업이 없다면, 페이지네이션될 데이터 리스트가 존재하지 않기 때문에 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> |
Collaborator
There was a problem hiding this comment.
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); |
Collaborator
There was a problem hiding this comment.
현재 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) | ||
| ); | ||
| }; |
Collaborator
There was a problem hiding this comment.
selectedIds는 number[] 대신 Set< number > 로 관리하는 것이 좋을 것 같습니다!
현재 toggleAll, toggleRow 두 함수 모두 북마크 id의 중복 선택을 방지하기 위해 Set을 사용하고 있는데, 배열을 spread한 뒤 Set으로 중복을 제거하고 다시 배열로 변환하는 과정을 반복하고 있어 불필요한 변환 비용이 발생한다고 생각합니다!
처음부터 Set로 state를 관리하면 변환 과정이 없어 더 코드가 간단해질 것 같아요!
There was a problem hiding this comment.
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
📒 Files selected for processing (5)
src/pages/bookmark/bookmark-page.css.tssrc/pages/bookmark/bookmark-page.tsxsrc/pages/bookmark/config/bookmark-page.constant.tssrc/pages/bookmark/ui/bookmark-empty-state.tsxsrc/pages/bookmark/ui/bookmark-table.tsx
Collaborator
|
체크박스 / 테이블 / 빈 상태를 페이지에서 적절히 분리해두셔서, 북마크 페이지 UI 구조가 한눈에 잘 들어오는 것 같습니다 🙂 수고 많으셨습니다 |
u-zzn
reviewed
Mar 31, 2026
Comment on lines
+166
to
+172
| <section className={styles.paginationSection}> | ||
| <Pagination | ||
| currentPage={resolvedCurrentPage} | ||
| totalPage={paginationTotalPage} | ||
| onPageChange={handlePageChange} | ||
| /> | ||
| </section> |
Collaborator
There was a problem hiding this comment.
검색 결과가 empty이거나 전체 북마크가 empty인 경우에도 pagination이 항상 노출되는 구조라, '실제 화면에서 1페이지가 같이 보이면 조금 어색하지 않나?' 라고 개인적으로 생각했던 것 같아요!
isBookmarkEmpty || isSearchResultEmpty일 때는 pagination 자체를 숨기거나, 실제 row가 있을 때만 렌더링하는 쪽도 한 번 고려해보면 좋을 것 같은데, 이건 저희끼리 결정할 수 있는건 아니니 디쌤들이랑 같이 이야기해서 정해보는거 어떨까요 ??
| onClick={handleOpenDeleteModal} | ||
| aria-label="북마크 삭제" | ||
| > | ||
| <IconTrashOff className={styles.trashIcon} aria-hidden="true" /> |
Collaborator
There was a problem hiding this comment.
아이콘 에셋에 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}")`, | ||
| }, |
Collaborator
There was a problem hiding this comment.
찾아보니 outline을 제거하는 경우에는 키보드 사용자에게 포커스 위치를 명확하게 전달할 수 있도록 :focus-visible 스타일을 별도로 정의해주는 게 접근성 측면에서 권장된다고 하더라구요 🙂
custom checkbox로 잘 분리해주신 것 같은데, 현재 outline: none만 있고 :focus-visible 대응은 따로 없어서
키보드 탐색 시 포커스가 잘 안 보일 수 있을 것 같습니다!
간단하게라도 focus 스타일 하나 추가해두면 재사용 컴포넌트로서 완성도가 더 올라갈 것 같습니다 :)
Comment on lines
+60
to
+62
| if (value.length > 0 && trimmedValue.length === 0) { | ||
| return; | ||
| } |
Collaborator
There was a problem hiding this comment.
공백만 입력한 검색어는 막는 의도 좋네요! :)
다만 현재 로직을 보면 태그한 이 조건 때문에 " "처럼 공백만 입력한 경우에는 keyword 상태가 갱신되지 않고 그대로 유지되는 흐름인 것 같아요.
이 상태에서 사용자는 “검색어를 지웠다”라고 인식할 가능성이 높은데, 실제로는 이전 keyword 기준으로 필터링이 계속 유지되다 보니 UI 상에서는 검색이 초기화되지 않은 것처럼 느껴질 수도 있을 것 같아요.
의도하신 UX가 아니라면, 공백만 입력된 경우에는 early return 대신
setKeyword("");로 명시적으로 초기화해주는 쪽이 사용자 기대와 더 맞을 것 같은데, 어떻게 생각하세요 ??
예를 들어, 검색어 전체 삭제 → 전체 리스트 다시 보이기 이런 경우가 있을 것 같아요!
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
src/pages/bookmark/bookmark-page.tsx (2)
100-110: 🧹 Nitpick | 🔵 Trivial공백만 입력 후 제출 시 UX가 다소 혼란스러울 수 있어요.
현재 로직은 공백만 입력하면 이전
keyword로 입력창을 되돌려요 (line 104). 사용자 관점에서는 "검색어를 지웠다"고 인식할 수 있는데, 실제로는 이전 필터가 유지되어 혼란을 줄 수 있어요.공백만 입력된 경우 검색을 초기화하는 방향도 고려해 보세요.
♻️ 대안: 공백 입력 시 검색 초기화
const handleSearch = (value: string) => { const trimmedValue = value.trim(); if (value.length > 0 && trimmedValue.length === 0) { - setSearchInput(keyword); - return; + // 공백만 입력한 경우 검색 초기화 + updateSearchParams("", 1); + setSelectedIds(new Set()); + return; } updateSearchParams(trimmedValue, 1); setSelectedIds(new Set()); };🤖 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 100 - 110, handleSearch에서 사용자가 공백만 입력하면 현재는 이전 keyword로 입력창을 복원(setSearchInput(keyword))해 UX가 혼란스러우니, 공백만 입력된 경우 검색을 초기화하도록 변경하세요: handleSearch 내부에서 trimmedValue가 빈 문자열일 때 updateSearchParams('', 1)로 검색 파라미터를 초기화하고 setSearchInput('')으로 입력창을 비우며 필요하면 setSelectedIds(new Set())로 선택 상태도 리셋하도록 수정하세요; 관련 식별자는 handleSearch, setSearchInput, keyword, updateSearchParams, setSelectedIds입니다.
209-217: 🧹 Nitpick | 🔵 Trivial선택 여부에 따라 휴지통 아이콘을 구분하면 더 직관적이에요.
현재
IconTrash만 사용하고 있는데, 에셋에icon_trash.svg와icon_trash_off.svg가 있다면 선택된 항목이 있을 때와 없을 때 아이콘을 구분하면 시각적 피드백이 개선될 거예요.♻️ 아이콘 분기 예시
+import { IconTrash, IconTrashOff } from "@/shared/assets/icons"; // ... <Button variant="secondary" size="medium" disabled={isDeleteDisabled} onClick={handleOpenDeleteModal} aria-label="북마크 삭제" > - <IconTrash className={styles.trashIcon} aria-hidden="true" /> + {isDeleteDisabled ? ( + <IconTrashOff className={styles.trashIcon} aria-hidden="true" /> + ) : ( + <IconTrash className={styles.trashIcon} aria-hidden="true" /> + )} </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 209 - 217, Replace the single IconTrash render with a conditional that switches between the active and disabled trash assets based on isDeleteDisabled: when isDeleteDisabled is false render the regular trash icon (icon_trash.svg) and when true render the “off” variant (icon_trash_off.svg); update the JSX inside the Button (where IconTrash is used) to choose the correct asset and keep the same className (styles.trashIcon) and aria-hidden prop, and ensure the Button still uses onClick={handleOpenDeleteModal} and disabled={isDeleteDisabled} so behavior is unchanged.
🤖 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/app/routes/public-routes.tsx`:
- Line 54: The bookmark route (ROUTES.BOOKMARK) is currently listed in the
public routes array and must be moved to the protected/private routing before
merge; remove the { path: ROUTES.BOOKMARK, element: <BookmarkPage /> } entry
from publicRoutes and add it to your authenticated/protected route configuration
(or wrap BookmarkPage with your auth guard/PrivateRoute component used
elsewhere) so that access requires a logged-in user and proper authorization
checks; update imports if needed and run route-related tests to confirm the
BookmarkPage is no longer reachable publicly.
In `@src/features/bookmark/ui/bookmark-checkbox.css.ts`:
- Around line 5-43: Extract the repeated "2.4rem" into a top-level constant
named CHECKBOX_SIZE and replace the literal in the style objects: use
CHECKBOX_SIZE for checkbox.width, checkbox.height, icon.width, icon.height, and
iconSvg.width, iconSvg.height; keep all other properties and the focus-visible
selector intact so the selectors referencing input and icon still work. Ensure
the constant uses UPPER_SNAKE_CASE (CHECKBOX_SIZE) and is defined before the
exported styles so it can be reused.
In `@src/features/bookmark/ui/bookmark-table.css.ts`:
- Around line 71-75: The companyButton style currently only defines a hover
state; add a matching :focus-visible rule so keyboard users get the same visual
feedback. Inside the selectors object for companyButton (next to "&:hover"), add
a "&:focus-visible" entry that applies the same textDecoration: "underline" (or
an accessible outline if preferred) to mirror the hover behavior and ensure
keyboard focus is visible.
---
Duplicate comments:
In `@src/pages/bookmark/bookmark-page.tsx`:
- Around line 100-110: handleSearch에서 사용자가 공백만 입력하면 현재는 이전 keyword로 입력창을
복원(setSearchInput(keyword))해 UX가 혼란스러우니, 공백만 입력된 경우 검색을 초기화하도록 변경하세요:
handleSearch 내부에서 trimmedValue가 빈 문자열일 때 updateSearchParams('', 1)로 검색 파라미터를
초기화하고 setSearchInput('')으로 입력창을 비우며 필요하면 setSelectedIds(new Set())로 선택 상태도 리셋하도록
수정하세요; 관련 식별자는 handleSearch, setSearchInput, keyword, updateSearchParams,
setSelectedIds입니다.
- Around line 209-217: Replace the single IconTrash render with a conditional
that switches between the active and disabled trash assets based on
isDeleteDisabled: when isDeleteDisabled is false render the regular trash icon
(icon_trash.svg) and when true render the “off” variant (icon_trash_off.svg);
update the JSX inside the Button (where IconTrash is used) to choose the correct
asset and keep the same className (styles.trashIcon) and aria-hidden prop, and
ensure the Button still uses onClick={handleOpenDeleteModal} and
disabled={isDeleteDisabled} so behavior is unchanged.
🪄 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: 48ba71b5-fd78-439f-88cd-f9d113d3e22a
📒 Files selected for processing (15)
src/app/routes/paths.tssrc/app/routes/public-routes.tsxsrc/features/bookmark/.gitkeepsrc/features/bookmark/config/bookmark-page.constants.tssrc/features/bookmark/index.tssrc/features/bookmark/ui/bookmark-checkbox.css.tssrc/features/bookmark/ui/bookmark-checkbox.tsxsrc/features/bookmark/ui/bookmark-empty-state.css.tssrc/features/bookmark/ui/bookmark-empty-state.tsxsrc/features/bookmark/ui/bookmark-table.css.tssrc/features/bookmark/ui/bookmark-table.tsxsrc/features/experience-detail/ui/experience-form/experience-form.tsxsrc/pages/bookmark/bookmark-page.css.tssrc/pages/bookmark/bookmark-page.tsxsrc/shared/assets/icons/index.ts
| { path: ROUTES.LANDING, element: <LandingPage /> }, | ||
| { path: ROUTES.HOME, element: <HomePage /> }, | ||
| { path: ROUTES.COMPANY(), element: <CompanyDetailPage /> }, | ||
| { path: ROUTES.BOOKMARK, element: <BookmarkPage /> }, |
There was a problem hiding this comment.
북마크 페이지를 publicRoutes에 두는 임시 처리 상태를 머지 전에 정리해 주세요.
북마크는 사용자별 데이터 접근이 필요한 화면이라 공개 라우트에 남아 있으면 인증/인가 누락 리스크가 있습니다. PR 설명에 “temporary routing”으로 명시된 만큼, 머지 전 protected/private 라우트로 이동이 필요합니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/app/routes/public-routes.tsx` at line 54, The bookmark route
(ROUTES.BOOKMARK) is currently listed in the public routes array and must be
moved to the protected/private routing before merge; remove the { path:
ROUTES.BOOKMARK, element: <BookmarkPage /> } entry from publicRoutes and add it
to your authenticated/protected route configuration (or wrap BookmarkPage with
your auth guard/PrivateRoute component used elsewhere) so that access requires a
logged-in user and proper authorization checks; update imports if needed and run
route-related tests to confirm the BookmarkPage is no longer reachable publicly.
Comment on lines
+5
to
+43
| export const checkbox = style({ | ||
| position: "relative", | ||
| display: "inline-flex", | ||
| alignItems: "center", | ||
| justifyContent: "center", | ||
| width: "2.4rem", | ||
| height: "2.4rem", | ||
| cursor: "pointer", | ||
| verticalAlign: "middle", | ||
| }); | ||
|
|
||
| export const input = style({ | ||
| position: "absolute", | ||
| inset: 0, | ||
| margin: 0, | ||
| opacity: 0, | ||
| cursor: "pointer", | ||
| }); | ||
|
|
||
| export const icon = style({ | ||
| width: "2.4rem", | ||
| height: "2.4rem", | ||
| display: "inline-flex", | ||
| alignItems: "center", | ||
| justifyContent: "center", | ||
| pointerEvents: "none", | ||
| selectors: { | ||
| [`${input}:focus-visible + &`]: { | ||
| outline: `0.2rem solid ${themeVars.color.blue400}`, | ||
| outlineOffset: "0.2rem", | ||
| }, | ||
| }, | ||
| }); | ||
|
|
||
| export const iconSvg = style({ | ||
| width: "2.4rem", | ||
| height: "2.4rem", | ||
| flexShrink: 0, | ||
| }); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
반복되는 체크박스 크기 값을 상수로 추출해 주세요.
2.4rem이 여러 셀렉터에 중복되어 있어, 디자인 변경 시 수정 누락 가능성이 있습니다.
♻️ 제안 diff
+const CHECKBOX_SIZE = "2.4rem";
+
export const checkbox = style({
position: "relative",
display: "inline-flex",
alignItems: "center",
justifyContent: "center",
- width: "2.4rem",
- height: "2.4rem",
+ width: CHECKBOX_SIZE,
+ height: CHECKBOX_SIZE,
cursor: "pointer",
verticalAlign: "middle",
});
@@
export const icon = style({
- width: "2.4rem",
- height: "2.4rem",
+ width: CHECKBOX_SIZE,
+ height: CHECKBOX_SIZE,
display: "inline-flex",
@@
export const iconSvg = style({
- width: "2.4rem",
- height: "2.4rem",
+ width: CHECKBOX_SIZE,
+ height: CHECKBOX_SIZE,
flexShrink: 0,
});As per coding guidelines, src/**/*.{ts,tsx}: "Use UPPER_SNAKE_CASE for constants (e.g., VITE_API_KEY, ROTATE_DELAY)".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/features/bookmark/ui/bookmark-checkbox.css.ts` around lines 5 - 43,
Extract the repeated "2.4rem" into a top-level constant named CHECKBOX_SIZE and
replace the literal in the style objects: use CHECKBOX_SIZE for checkbox.width,
checkbox.height, icon.width, icon.height, and iconSvg.width, iconSvg.height;
keep all other properties and the focus-visible selector intact so the selectors
referencing input and icon still work. Ensure the constant uses UPPER_SNAKE_CASE
(CHECKBOX_SIZE) and is defined before the exported styles so it can be reused.
Comment on lines
+71
to
+75
| selectors: { | ||
| "&:hover": { | ||
| textDecoration: "underline", | ||
| }, | ||
| }, |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
companyButton에 :focus-visible 스타일도 추가해 주세요.
현재 hover만 정의되어 있어 키보드 탐색 시 상호작용 피드백이 약합니다. hover와 동일한 underline(또는 outline) 상태를 :focus-visible에도 부여하는 것을 권장합니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/features/bookmark/ui/bookmark-table.css.ts` around lines 71 - 75, The
companyButton style currently only defines a hover state; add a matching
:focus-visible rule so keyboard users get the same visual feedback. Inside the
selectors object for companyButton (next to "&:hover"), add a "&:focus-visible"
entry that applies the same textDecoration: "underline" (or an accessible
outline if preferred) to mirror the hover behavior and ensure keyboard focus is
visible.
odukong
approved these changes
Apr 4, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
✏️ Summary
close [Feat] 북마크 페이지 구현 #154
체크박스 컴포넌트 구현
북마크 페이지 UI 구현
📑 Tasks
체크박스 컴포넌트 구현
bookmark-checkbox.tsxbookmark-checkbox.css.ts북마크 페이지에서 사용하는 체크박스 UI 컴포넌트를 분리했습니다.
전체 선택/개별 선택 모두 같은 컴포넌트로 재사용되도록 구현했습니다.
bookmark-table.tsxbookmark-table.css.ts테이블 마크업과 스타일은 페이지 파일에서 분리해
BookmarkTable로 컴포넌트화했습니다 !북마크 페이지 UI 구현
북마크 페이지의 전체 레이아웃과 테이블 UI를 구현습니다.
북마크 목록 화면에서 빈 상태를 2가지로 분기해서 처리했습니다.
👀 To Reviewer
path.ts와public-routes.tsx에서의 북마크 라우팅은 임시 라우팅입니다. 머지 전에 최종 위치로 수정하겠습니다 !!header.tsx에서도 에러 이슈 때문에 프로필 api를 호출하지 않도록 임시로 처리해뒀는데, 머지 전에 수정하겠습니다 !!!!!📸 Screenshot
🔔 ETC
Summary by CodeRabbit
새로운 기능