Skip to content
Merged
Show file tree
Hide file tree
Changes from 5 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
43 changes: 43 additions & 0 deletions src/app/providers/modal-provider.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
import { useEffect, useState, type ReactNode } from "react";
import { useLocation } from "react-router-dom";

import { modalStore } from "@/shared/model/store";

import { Modal } from "../../shared/ui/modal/modal";

interface ModalItem {
id: string;
content: ReactNode;
autoPlay?: number;
}

export const ModalProvider = () => {
const { pathname } = useLocation();
const [modals, setModals] = useState<ModalItem[]>([]);

useEffect(() => {
modalStore.subscribe(setModals);
return () => modalStore.unsubscribe();
}, []);
Comment on lines +16 to +21
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

외부 store 구독은 useSyncExternalStore로 바꾸는 편이 더 견고합니다.

지금 패턴은 useState([])로 시작한 뒤 effect에서 subscribe만 등록하므로, 현재 snapshot 동기화와 concurrent rendering 안정성을 React가 직접 보장해주지 않습니다. modalStore가 외부 store라면 getSnapshot을 제공하고 useSyncExternalStore로 연결하는 쪽이 더 안전합니다.

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

In `@src/app/providers/modal-provider.tsx` around lines 16 - 21, The current modal
subscription uses useState + useEffect (modalStore.subscribe -> setModals) which
is not safe for concurrent rendering; replace this pattern by implementing a
getSnapshot function that reads current modalStore state and subscribe function
that delegates to modalStore.subscribe, then call React's
useSyncExternalStore(subscribe, getSnapshot) instead of useState/useEffect so
the component reads synchronized snapshots from modalStore; update references to
useSyncExternalStore, modalStore, and remove the useEffect/setModals logic
accordingly.


useEffect(() => {
modalStore.reset();
}, [pathname]);

Comment on lines +23 to +26
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.

지금 reset 기준이 pathname만 보고 있어서 같은 페이지 안에서 query string이나 hash만 바뀌는 이동에서는 모달이 그대로 남을 수도 있을 것 같아요 ! 혹시 이 부분은 의도적으로 두신 건지 궁금합니다 ! 😉

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

지금 당장은 query를 통해 url이 바뀌는 페이지에서 모달이 사용되는 경우가 없어
주요하게 생각하지 않은 부분이었어요 히힣.
추후에 다른 페이지에서 모달이 쓰일 때 충분히 발생할 수 있는 이슈인 것 같아요. 해당 부분 관련해서 pathname이 아닌 location.key를 활용해 수정하도록 하겠습니다 !

return (
<>
{modals.map((modal) => {
return (
<Modal
key={modal.id}
isOpen={true}
autoPlay={modal.autoPlay}
onClose={() => modalStore.close(modal.id)}
>
{modal.content}
</Modal>
);
})}
</>
);
};
2 changes: 2 additions & 0 deletions src/app/routes/root-layout.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,13 @@ import { Outlet } from "react-router-dom";

import { Header } from "@widgets/index";

import { ModalProvider } from "../providers/modal-provider";
import { StoreResetListener } from "../providers/store-reset-listener";

export const RootLayout = () => {
return (
<>
<ModalProvider />
<StoreResetListener />
<Header />
<main>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import { useEffect, useCallback, useRef } from "react";
import { useEffect, useCallback } from "react";
import { useBlocker } from "react-router-dom";

import { useModal } from "@/shared/ui/modal/use-modal";
import { modalStore } from "@/shared/model/store";
import { ModalBasic } from "@/shared/ui";

import {
initialDraft,
Expand All @@ -24,11 +25,10 @@ const isDraftDirty = (draft: ExperienceUpsertBody): boolean => {
};

export const useLeaveConfirm = () => {
const LEAVE_MODAL_ID = "leave-confirm-modal";
const mode = useExperienceDetailStore((s) => s.mode);
const draft = useExperienceDetailStore((s) => s.draft);

const { isOpen, openModal, closeModal } = useModal();

const shouldBlock =
(mode === "create" || mode === "edit") && isDraftDirty(draft);

Expand All @@ -46,17 +46,36 @@ export const useLeaveConfirm = () => {
return shouldBlockNow && !isSubmitting && !isTransitioning;
});

const prevBlockerStateRef = useRef(blocker.state);
const confirmLeave = useCallback(() => {
if (blocker.state === "blocked") {
blocker.proceed();
}
}, [blocker]);

useEffect(() => {
const wasBlocked = prevBlockerStateRef.current === "blocked";
const nowBlocked = blocker.state === "blocked";
prevBlockerStateRef.current = blocker.state;
const cancelLeave = useCallback(() => {
if (blocker.state === "blocked") {
blocker.reset();
}
modalStore.close(LEAVE_MODAL_ID);
}, [blocker]);

if (nowBlocked && !wasBlocked && !isOpen) {
openModal();
useEffect(() => {
if (blocker.state === "blocked") {
modalStore.open(
<ModalBasic
title={`작성중인 내용이 있습니다.\n정말 나가시겠습니까?`}
subTitle="저장하지 않으면 내용이 사라져요."
closeText="이어서 작성"
confirmText="나가기"
onClose={cancelLeave}
onConfirm={confirmLeave}
/>,
0,
undefined,
LEAVE_MODAL_ID
);
}
}, [blocker.state, isOpen, openModal]);
}, [blocker.state, cancelLeave, confirmLeave]);

useEffect(() => {
const handleBeforeUnload = (e: BeforeUnloadEvent) => {
Expand All @@ -70,23 +89,5 @@ export const useLeaveConfirm = () => {
return () => window.removeEventListener("beforeunload", handleBeforeUnload);
}, [shouldBlock]);

const confirmLeave = useCallback(() => {
if (blocker.state === "blocked") {
blocker.proceed();
}
closeModal();
}, [blocker, closeModal]);

const cancelLeave = useCallback(() => {
if (blocker.state === "blocked") {
blocker.reset();
}
closeModal();
}, [blocker, closeModal]);

return {
isOpen,
confirmLeave,
cancelLeave,
};
return {};
};
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

실제로 리팩토링된 부분을 보니 바로 이해가 되네요 !!! useModal 없이 모달을 관리할 수 있게 되면서, jsx 코드가 분리되고 별도의 선언 없이 store를 사용해 모달을 열 수 있는 점도 너무 좋습니다! 모달에 들어갈 내용들을 사용처에서 입력해주는 것도 응집도 측면에서 정말 좋은 거 같아요 천재 개발자 오수빈답습니다 👍👍

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

계속해서 모달에 대한 상태 관리가 use-modal로 인해 분산되어 있다는 생각을 해서 왜 인지 모르게 찜찜했는데
use-modal 없이 modal을 관리할 수 있게 되어서 햅삐하고, 거대칭찬을 받아서 또 햅삐하네요 //
부끄_하치와레

Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import { EXPERIENCE_TYPE } from "@/shared/config/experience";
import { parseYMD } from "@/shared/lib/format-date";
import { modalStore } from "@/shared/model/store";
import { ModalBasic, Tooltip } from "@/shared/ui";
import { Button } from "@/shared/ui/button/button";
import { useModal } from "@/shared/ui/modal/use-modal";
import { Tag } from "@/shared/ui/tag/tag";
import { Textfield } from "@/shared/ui/textfield/textfield";
import { HELP_TOOLTIP_CONTENT } from "@/shared/ui/tooltip/tooltip.content";
Expand All @@ -24,9 +24,6 @@ const ExperienceViewer = () => {
const { showEditDelete, onClickEdit, onClickDelete, onToggleDefault } =
useExperienceHeaderActions();

const { isOpen: isDeleteModalOpen, handleModal: toggleDeleteModal } =
useModal();

const startDate = current?.startAt ? parseYMD(current.startAt) : null;
const endDate = current?.endAt ? parseYMD(current.endAt) : null;

Expand All @@ -42,6 +39,22 @@ const ExperienceViewer = () => {

const typeLabel = current.type ? EXPERIENCE_TYPE[current.type] : "미지정";

const handleOpenDeleteModal = () => {
modalStore.open(
<ModalBasic
title="이 경험을 삭제하시겠습니까?"
subTitle="작성한 내용은 즉시 제거되며, 복구할 수 없습니다."
closeText="취소"
confirmText="삭제"
onClose={() => modalStore.reset()} // 취소 시 닫기
Copy link
Copy Markdown
Collaborator

@u-zzn u-zzn Mar 16, 2026

Choose a reason for hiding this comment

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

현재 store는 modalList 기반으로 여러 모달을 관리할 수 있게 설계되어 있는 것 같은데, 여기서는 특정 삭제 확인 모달만 닫는 목적임에도 reset()으로 전체 모달을 비우고 있어요. 지금은 문제 없을 수 있지만, 이후 다른 전역 모달이 함께 열리는 경우 의도치 않게 전부 닫히는 부작용이 생길 수도 있을 것 같아요!

store에서 close(id)reset()의 책임이 분리되어 있는 만큼, 여기서는 reset()보다는 close(id)로 해당 모달만 닫는 쪽이 설계 의도와도 더 잘 맞고 안전할 것 같은데, 어떻게 생각하시나요? ☺️

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

말씀해주신대로 close(id)로 모달을 개별적으로 닫는 것이 안전한 방향이지만,
experience-viewer 경험/등록/삭제에서의 모달 관련한 수정 코드는 이어 해당 기능들이 리팩토링이 진행될 부분임을 감안해서 모달 동작이 정상적으로 이루어지게끔만 수정 코드를 최대한 줄여 임시적인 느낌으로 수정한 상태입니다! 해당 부분 관련해서는 리팩토링 하면서 함께 반영해주시면 좋을 것 같아요! 🫰🏻

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.

넵 이해했습니다!! 말씀해주신 부분은 제가 추후 리팩토링 진행하면서 close(id) 기반으로 정리해보겠습니다. 감사합니다 🙂

onConfirm={() => {
onClickDelete(); // 실제 삭제 동작
modalStore.reset(); // 모달 닫기
}}
/>
);
Comment on lines +42 to +55
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

이 확인창에서 reset()을 쓰면 다른 전역 모달까지 같이 닫힙니다.

onCloseonConfirm이 모두 modalStore.reset()을 호출해서, 이 삭제 모달과 무관한 다른 모달도 함께 사라집니다. 여기서는 고정 ID를 넘기고 modalStore.close(id)로 자기 자신만 닫도록 분리하는 편이 안전합니다.

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

In `@src/features/experience-detail/ui/experience-viewer/experience-viewer.tsx`
around lines 42 - 55, The delete modal currently calls modalStore.reset() in
handleOpenDeleteModal which closes all global modals; change it to open the
ModalBasic with a fixed unique id (e.g. const id = 'experience-delete') passed
to modalStore.open and then replace modalStore.reset() in both onClose and
onConfirm with modalStore.close(id) so only this modal is closed (keep onConfirm
calling onClickDelete() before modalStore.close(id)). Ensure you pass the id
when opening and use the same id in onClose/onConfirm.

};

return (
<main className={s.page}>
<StickyHeader
Expand All @@ -53,7 +66,7 @@ const ExperienceViewer = () => {
<Button
variant="secondary"
size="small"
onClick={toggleDeleteModal}
onClick={handleOpenDeleteModal}
>
삭제하기
</Button>
Expand Down Expand Up @@ -130,19 +143,6 @@ const ExperienceViewer = () => {
</div>
</div>
</section>

<ModalBasic
title="이 경험을 삭제하시겠습니까?"
subTitle="작성한 내용은 즉시 제거되며, 복구할 수 없습니다."
closeText="취소"
confirmText="삭제"
isOpen={isDeleteModalOpen}
onClose={toggleDeleteModal}
onConfirm={() => {
toggleDeleteModal();
onClickDelete();
}}
/>
</main>
);
};
Expand Down
110 changes: 44 additions & 66 deletions src/features/experience-matching/ui/select-company/select-company.tsx
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import { useEffect, useRef, useState } from "react";
import { useEffect, useState } from "react";
import { useNavigate } from "react-router-dom";

import { ROUTES } from "@/app/routes/paths";
import { Button, Modal, useModal } from "@/shared/ui";
import { modalStore } from "@/shared/model/store";
import { Button, Modal } from "@/shared/ui";
import {
useGetExperience,
useGetCompanyList,
Expand All @@ -19,53 +20,61 @@ export const SelectCompany = ({ onClick }: { onClick: () => void }) => {
const navigate = useNavigate();
const { data } = useGetExperience(); // 경험 조회 API

/** Report 전역 상태 */
// AI-Report 입력 단계 저장을 위한 전역 상태
const setCompany = useReportStore((state) => state.setCompany);
const company = useReportStore((state) => state.company);

/** 모달 상태 관리 */
const { autoPlay, isOpen, handleModal } = useModal(3000); // 몇 초 뒤 닫히게 할 건지 정의
const alertModal = useModal(); // 경험 등록 여부 확인 모달

/** 입력 데이터 상태 관리 */
// 기업 입력 데이터 상태 관리
const [inputValue, setInputValue] = useState(""); // 실시간 입력 상태
const [searchKeyword, setSearchKeyword] = useState(""); // 디바운스된 키워드 상태
const [selectedCompany, setSelectedCompany] = useState<Company | null>(
company
);
const { data: searchResults = [] } = useGetCompanyList(searchKeyword); // 기업 검색 API

// 경험 등록 여부 확인 모달 오픈
// 경험 등록 여부 확인 모달
useEffect(() => {
if (data?.totalElements === 0) {
if (!alertModal.isOpen) {
alertModal.openModal();
}
modalStore.open(
<>
Comment on lines +35 to +39
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.

data?.totalElements === 0일 때마다 바로 modalStore.open()을 호출하고 있어서, 쿼리 refetch가 일어나거나 컴포넌트가 다시 마운트되는 경우 동일한 안내 모달이 한 번 더 쌓일 수 있을 것 같아요 !

이전에 open 여부를 한 번 체크해주는 흐름이 있었던 것 처럼 여기서도 고정 id를 넘겨서 중복 노출을 방지하는건 어떻게 생각하시나요 !? 👀

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

쿼리 refetch가 일어나는 경우는 다른 페이지에서 현재 페이지로 다시 포커스가 되거나(refetchOnWindowFocus), query-key가 변경될 때일텐데, 전역적으로 refetchOnWindowFocus 옵션을 false로 설정해두었고, query-key가 변경될 가능성이 낮다고 생각했습니다!
또한 컴포넌트의 마운트의 경우에도 사실상 modal-provider의 useEffect에서 location.key(url이 변경될 때)가 변경됨에 따라 modal.reset()이 실행되어 modal_list가 한 번 비워지기 때문에, 중복 모달이 띄워질 가능성은 낮은 구조이기도 합니다.
하지만 개발 환경의 Strict Mode나 예외적인 리마운트 상황까지 고려했을 때, 고정 ID를 설정하는 것이 안정성을 보장하는 방안이라고 생각이 들어 해당 부분 함께 반영하도록 하겠습니다!

<Modal.Content>
<Modal.Title>아직 등록된 경험이 없습니다</Modal.Title>
<Modal.SubTitle>지금 바로 경험을 등록하러 가볼까요?</Modal.SubTitle>
</Modal.Content>
<Modal.Buttons>
<Button variant="secondary" onClick={() => navigate(ROUTES.HOME)}>
나가기
</Button>
<Button
variant="primary"
onClick={() => navigate(ROUTES.EXPERIENCE_CREATE)}
>
이동하기
</Button>
</Modal.Buttons>
</>
);
}
}, [data, alertModal]);
}, [data, navigate]);

// useEffect(() => {
// if (company?.id) {
// const temp={
// id:company.id,
// name:company.name
// }
// setSelectedCompany(temp);
// }
// }, [company]);

const { data: searchResults = [] } = useGetCompanyList(searchKeyword); // 기업 검색 API

// 모달 닫힘 여부 확인 후 페이지 이동
const prevIsOpen = useRef(isOpen);
useEffect(() => {
if (prevIsOpen.current === true && isOpen === false) {
if (selectedCompany) {
setCompany(selectedCompany); // 선택된 기업 데이터 저장
const handleSearch = () => {
if (!selectedCompany) return;
// 기업 선택 후, 대기하는 모달
modalStore.open(
<>
<Modal.Content type="auto">
<Modal.Title>{selectedCompany.name}을 선택하셨습니다</Modal.Title>
<Modal.SubTitle>기업분석 내용을 불러오는 중입니다.</Modal.SubTitle>
</Modal.Content>
<Modal.Image />
</>,
3000,
() => {
setCompany(selectedCompany);
onClick();
}
}
prevIsOpen.current = isOpen;
}, [isOpen, selectedCompany, onClick, setCompany]);
);
Comment on lines +66 to +81
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

기업 선택 모달에도 고정 ID 사용을 권장해요.

handleSearch에서 열리는 모달은 고정 ID가 없어서 new Date().toString() 기본값이 사용돼요. 재진입 방지와 함께 "SELECT-COMPANY-LOADING"과 같은 고정 ID를 사용하면 더 안정적이에요.

🔧 제안 코드
     modalStore.open(
       <>
         <Modal.Content type="auto">
           <Modal.Title>
             {josa(selectedCompany.name, "을/를")} 선택하셨습니다
           </Modal.Title>
           <Modal.SubTitle>기업분석 내용을 불러오는 중입니다.</Modal.SubTitle>
         </Modal.Content>
         <Modal.Image />
       </>,
       3000,
       () => {
         setCompany(selectedCompany);
         onClick();
-      }
+      },
+      "SELECT-COMPANY-LOADING"
     );
📝 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
modalStore.open(
<>
<Modal.Content type="auto">
<Modal.Title>
{josa(selectedCompany.name, "을/를")} 선택하셨습니다
</Modal.Title>
<Modal.SubTitle>기업분석 내용을 불러오는 중입니다.</Modal.SubTitle>
</Modal.Content>
<Modal.Image />
</>,
3000,
() => {
setCompany(selectedCompany);
onClick();
}
}
prevIsOpen.current = isOpen;
}, [isOpen, selectedCompany, onClick, setCompany]);
);
modalStore.open(
<>
<Modal.Content type="auto">
<Modal.Title>
{josa(selectedCompany.name, "을/를")} 선택하셨습니다
</Modal.Title>
<Modal.SubTitle>기업분석 내용을 불러오는 중입니다.</Modal.SubTitle>
</Modal.Content>
<Modal.Image />
</>,
3000,
() => {
setCompany(selectedCompany);
onClick();
},
"SELECT-COMPANY-LOADING"
);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/features/experience-matching/ui/select-company/select-company.tsx` around
lines 66 - 81, The modal opened in select-company.tsx via modalStore.open(...)
uses the default ephemeral ID (new Date().toString()), causing re-entry issues;
update the call to pass a fixed, descriptive ID (e.g. "SELECT-COMPANY-LOADING")
to modalStore.open so the modal is identified consistently and re-entry is
prevented, keeping the existing content, duration (3000) and completion callback
that calls setCompany(selectedCompany) and onClick(); locate the modalStore.open
invocation in the select-company component to make this change.

};
Comment on lines +63 to +82
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

이 플로우는 재진입 방지가 필요합니다.

handleSearch가 클릭마다 새 3초 타이머를 등록하는데, 선택 버튼은 onSearch?.()를 동기적으로 바로 호출합니다. 그래서 빠른 더블클릭이면 setCompany(selectedCompany)onClick()이 여러 번 실행될 수 있습니다. 이 구간은 pending 플래그로 한 번만 실행되게 잠그거나, 버튼을 즉시 비활성화하는 방어가 필요합니다.

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

In `@src/features/experience-matching/ui/select-company/select-company.tsx` around
lines 60 - 79, handleSearch can be re-entered on rapid clicks causing
setCompany(selectedCompany) and onClick() to run multiple times; add a pending
guard (e.g., a useRef or state boolean like isPending) checked at the top of
handleSearch to return early if true, set isPending = true immediately before
calling modalStore.open, and clear/reset isPending in the modalStore.open
completion callback (the function that currently calls setCompany and onClick)
or after the 3s timeout so the flow only executes once per selection;
alternatively disable the related button when isPending is true. Ensure you
reference and update the same pending flag around handleSearch, modalStore.open,
setCompany and onClick to prevent double execution.


return (
<div className={styles.layout}>
Expand All @@ -77,39 +86,8 @@ export const SelectCompany = ({ onClick }: { onClick: () => void }) => {
onDebounceChange={setSearchKeyword}
selectedItem={selectedCompany}
onSelect={setSelectedCompany}
onSearch={handleModal}
onSearch={handleSearch}
/>
{/** 경험 등록 여부 확인 모달 */}
<Modal isOpen={alertModal.isOpen} onClose={alertModal.closeModal}>
<Modal.Content>
<Modal.Title>아직 등록된 경험이 없습니다</Modal.Title>
<Modal.SubTitle>지금 바로 경험을 등록하러 가볼까요?</Modal.SubTitle>
</Modal.Content>
<Modal.Buttons>
<Button
variant="secondary"
size="large"
onClick={() => navigate(ROUTES.HOME)}
>
나가기
</Button>
<Button
variant="primary"
size="large"
onClick={() => navigate(ROUTES.EXPERIENCE_CREATE)}
>
이동하기
</Button>
</Modal.Buttons>
</Modal>
{/** 기업 선택 후, 대기 모달 */}
<Modal autoPlay={autoPlay} isOpen={isOpen} onClose={handleModal}>
<Modal.Content type="auto">
<Modal.Title>{selectedCompany?.name}을 선택하셨습니다</Modal.Title>
<Modal.SubTitle>기업분석 내용을 불러오는 중입니다.</Modal.SubTitle>
</Modal.Content>
<Modal.Image />
</Modal>
</div>
);
};
Loading
Loading