Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/app/routes/paths.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,4 +16,5 @@ export const ROUTES = {
EXPERIENCE_EDIT: (id = ":id") => `/experience/${id}/edit`, // 경험 수정

MYPAGE: "/mypage",
BOOKMARK: "/bookmark",
};
7 changes: 7 additions & 0 deletions src/app/routes/public-routes.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,18 @@ const CompanyDetailPage = lazy(() =>
}))
);

const BookmarkPage = lazy(() =>
import("@/pages/bookmark/bookmark-page").then((module) => ({
default: module.BookmarkPage,
}))
);

export const guestRoutes = [{ path: ROUTES.LOGIN, element: <LoginPage /> }];

export const publicRoutes = [
{ path: ROUTES.LOGIN_AUTH, element: <KakaoLoginPage /> },
{ path: ROUTES.LANDING, element: <LandingPage /> },
{ path: ROUTES.HOME, element: <HomePage /> },
{ path: ROUTES.COMPANY(), element: <CompanyDetailPage /> },
{ path: ROUTES.BOOKMARK, element: <BookmarkPage /> },
];
104 changes: 104 additions & 0 deletions src/pages/bookmark/bookmark-page.css.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,104 @@
import { globalStyle, style } from "@vanilla-extract/css";

import { themeVars } from "@/app/styles";

export const page = style({
width: "100%",
maxWidth: "106rem",
margin: "0 auto",
paddingTop: `calc(${themeVars.height.header} + 8rem)`,
paddingBottom: "8rem",
});

export const topRow = style({
display: "flex",
alignItems: "center",
justifyContent: "space-between",
gap: "2rem",
});

export const headerSection = style({
display: "flex",
alignItems: "center",
gap: "1.6rem",
});

export const titleIcon = style({
width: "6.4rem",
height: "6.4rem",
flexShrink: 0,
});

export const titleWrap = style({
display: "flex",
flexDirection: "column",
gap: "0.4rem",
});

export const title = style({
color: themeVars.color.black,
...themeVars.fontStyles.title_b_24,
});

export const subtitle = style({
color: themeVars.color.gray500,
...themeVars.fontStyles.body_m_16,
});

export const actionSection = style({
display: "flex",
alignItems: "center",
gap: "1.2rem",
});

export const searchWrap = style({
display: "flex",
alignItems: "center",
});

export const deleteButtonWrap = style({
display: "inline-flex",
});

export const trashIcon = style({
width: "2.4rem",
height: "2.4rem",
flexShrink: 0,
});

globalStyle(`${deleteButtonWrap} > button`, {
width: "4.8rem",
minWidth: "4.8rem",
height: "4.8rem",
padding: 0,
borderRadius: "12px",
backgroundColor: themeVars.color.blue600,
borderColor: themeVars.color.blue600,
color: themeVars.color.white,
});

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,
});
Comment on lines +80 to +94
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.


export const tableSection = style({
marginTop: "5rem",
});

export const paginationSection = style({
marginTop: "4.8rem",
display: "flex",
justifyContent: "center",
});
200 changes: 200 additions & 0 deletions src/pages/bookmark/bookmark-page.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,200 @@
import { useMemo, useState } from "react";
import { useNavigate } from "react-router-dom";

import { ROUTES } from "@/app/routes/paths";
import IconBookmarkBefore from "@/shared/assets/icons/icon_bookmark_before.svg?react";
import IconTrashOff from "@/shared/assets/icons/icon_trash_off.svg?react";
import { Button, Modal, Pagination, Search } from "@/shared/ui";

import * as styles from "./bookmark-page.css";
import {
BOOKMARK_MOCK_ROWS,
BOOKMARK_PAGE_SIZE,
} from "./config/bookmark-page.constant";
import { BookmarkEmptyState } from "./ui/bookmark-empty-state";
import { BookmarkTable } from "./ui/bookmark-table";

const BookmarkPage = () => {
const navigate = useNavigate();

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);
Comment on lines +20 to +25
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에 상태를 저장해둔다면, 새로고침 시에도 페이지네이션+검색결과가 유지되어 사용자 경험 개선이 도움이 될 것이라고 생각합니다!


const filteredRows = useMemo(() => {
if (!keyword) return rows;

const normalizedKeyword = keyword.toLowerCase();
return rows.filter((row) =>
row.companyName.toLowerCase().includes(normalizedKeyword)
);
}, [keyword, rows]);

const totalPage = Math.ceil(filteredRows.length / BOOKMARK_PAGE_SIZE);
const paginationTotalPage = Math.max(totalPage, 1);
const resolvedCurrentPage = Math.min(currentPage, paginationTotalPage);

const currentPageRows = useMemo(() => {
const startIndex = (resolvedCurrentPage - 1) * BOOKMARK_PAGE_SIZE;
return filteredRows.slice(startIndex, startIndex + BOOKMARK_PAGE_SIZE);
}, [filteredRows, resolvedCurrentPage]);

const visibleIds = useMemo(
() => currentPageRows.map((row) => row.id),
[currentPageRows]
);

const isAllSelected =
visibleIds.length > 0 && visibleIds.every((id) => selectedIds.includes(id));

const isDeleteDisabled = selectedIds.length === 0;
const isBookmarkEmpty = rows.length === 0;
const isSearchResultEmpty = rows.length > 0 && filteredRows.length === 0;

const handleSearch = (value: string) => {
const trimmedValue = value.trim();

if (value.length > 0 && trimmedValue.length === 0) {
return;
}
Comment on lines +60 to +62
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("");

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

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


setKeyword(trimmedValue);
setCurrentPage(1);
setSelectedIds([]);
};
Comment on lines +57 to +67
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.


const handlePageChange = (page: number) => {
setCurrentPage(page);
};
Comment on lines +69 to +71
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.


const handleToggleAll = (checked: boolean) => {
if (checked) {
setSelectedIds((prev) => Array.from(new Set([...prev, ...visibleIds])));
return;
}

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

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

const handleOpenDeleteModal = () => {
if (isDeleteDisabled) return;
setIsDeleteModalOpen(true);
};

const handleCloseDeleteModal = () => {
setIsDeleteModalOpen(false);
};

const handleDeleteConfirm = () => {
setRows((prev) => prev.filter((row) => !selectedIds.includes(row.id)));
setSelectedIds([]);
setIsDeleteModalOpen(false);
};

const handleClickCompany = (companyId: number) => {
navigate(ROUTES.COMPANY(String(companyId)));
};

return (
<main className={styles.page}>
<section className={styles.topRow}>
<div className={styles.headerSection}>
<IconBookmarkBefore className={styles.titleIcon} aria-hidden="true" />
<div className={styles.titleWrap}>
<h1 className={styles.title}>기업 북마크</h1>
<p className={styles.subtitle}>
최근 6개월 이내에 스크랩한 기업정보 입니다
</p>
</div>
</div>

<div className={styles.actionSection}>
<div className={styles.searchWrap}>
<Search
size="small"
value={searchInput}
onChange={setSearchInput}
onSearch={handleSearch}
placeholder="기업명 검색"
inputAriaLabel="기업명 검색"
/>
</div>

<div className={styles.deleteButtonWrap}>
<Button
variant="secondary"
size="medium"
disabled={isDeleteDisabled}
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 아이콘으로 바꿔주는 게 더 자연스러울 수도 있을 것 같아요 :)

</Button>
</div>
</div>
</section>

<section className={styles.tableSection}>
{isBookmarkEmpty ? (
<BookmarkEmptyState type="bookmark" />
) : isSearchResultEmpty ? (
<BookmarkEmptyState type="search" />
) : (
<BookmarkTable
rows={currentPageRows}
pageSize={BOOKMARK_PAGE_SIZE}
selectedIds={selectedIds}
isAllSelected={isAllSelected}
onToggleAll={handleToggleAll}
onToggleRow={handleToggleRow}
onClickCompany={handleClickCompany}
/>
)}
</section>

<section className={styles.paginationSection}>
<Pagination
currentPage={resolvedCurrentPage}
totalPage={paginationTotalPage}
onPageChange={handlePageChange}
/>
Comment on lines +167 to +171
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처럼 조건문을 추가해주세요!

</section>
Comment on lines +166 to +172
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가 있을 때만 렌더링하는 쪽도 한 번 고려해보면 좋을 것 같은데, 이건 저희끼리 결정할 수 있는건 아니니 디쌤들이랑 같이 이야기해서 정해보는거 어떨까요 ?? ☺️


<Modal isOpen={isDeleteModalOpen} onClose={handleCloseDeleteModal}>
<Modal.XButton />
<Modal.Content>
<Modal.Title>선택한 북마크를 삭제할까요?</Modal.Title>
</Modal.Content>
<Modal.Buttons>
<Button
variant="secondary"
size="large"
onClick={handleDeleteConfirm}
>
삭제하기
</Button>
<Button
variant="primary"
size="large"
onClick={handleCloseDeleteModal}
>
취소하기
Comment on lines +180 to +192
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.

</Button>
</Modal.Buttons>
</Modal>
</main>
);
};

export { BookmarkPage };
Loading
Loading