Skip to content
Open
Show file tree
Hide file tree
Changes from 7 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: "1.2rem",
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",
});
196 changes: 196 additions & 0 deletions src/pages/bookmark/bookmark-page.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,196 @@
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 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를 관리하면 변환 과정이 없어 더 코드가 간단해질 것 같아요!


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

const closeDeleteModal = () => {
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={openDeleteModal}
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={toggleAll}
onToggleRow={toggleRow}
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={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 컴포넌트를 사용해주세요!

<Button
variant="secondary"
size="large"
onClick={handleDeleteConfirm}
>
삭제하기
</Button>
<Button variant="primary" size="large" onClick={closeDeleteModal}>
취소하기
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