Update after demo #55
Conversation
포크한 레포에 작동 테스틀 위해 연결 스토리북 형태도 세팅함
구조 수정 및 tailwind prefix에 맞게 mapping
이전 패키지 버전에선 빌드가 안 됐음.
httpClient에서 게스트모드에서도 format을 가져서 api 콜을 할 수 있도록 코드 수정
WalkthroughAdds a generic bottom-sheet system (store, hook, renderer) and integrates it throughout the app; introduces guest-mode support with IndexedDB-backed MSW handlers; debounces and types the API client; converts Drawer-based bottom sheets to UI-only components and hook-managed flows; mounts guest and bottom-sheet handlers in layout; and rewrites README and Storybook manager head. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as Component
participant Hook as useBottomSheet
participant Store as useBottomSheetStore
participant Renderer as BottomSheetRenderer
UI->>Hook: openBottomSheet(info)
Hook->>Store: push {id, info} to history
Renderer->>Store: read top history
Renderer-->>UI: render ContentComponent with contentProps
UI->>Hook: updateBottomSheet(nextInfo)
Hook->>Store: update top history (by id)
UI->>Hook: closeBottomSheet()
Hook->>Store: remove entry by id
Renderer-->>Store: no top -> unmount content
sequenceDiagram
participant Caller as API consumer
participant Debouncer as debouncedRequester
participant SW as securityWorker
participant Toast as ToastStore
Caller->>Debouncer: request(args)
Debouncer->>Debouncer: debounce window (1s)
Debouncer->>SW: prepare headers (token or guest JSON)
Debouncer->>Debouncer: perform fetch
alt error
Debouncer->>Toast: show "API ERROR"
end
Debouncer-->>Caller: response
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 27
🔭 Outside diff range comments (5)
components/group/groupRoom/GroupChatRoom/GroupChatRoom.tsx (1)
62-73: Fix dependency array: current expression can throw and breaks exhaustive-depsThe dependency expression dereferences properties on a potentially undefined page and is flagged by ESLint. It can also throw at runtime on initial load. Extract to variables and depend on a single stable value.
-useEffect(() => { - if (lastMessageRef.current) { - lastMessageRef.current.scrollIntoView({ - behavior: "smooth", - }); - } -}, [ - // 마지막 메시지 id - data?.[data?.length - 1].messages[ - data?.[data?.length - 1].messages.length - 1 - ].messageId, -]); +const lastPage = data?.[data.length - 1]; +const lastMessage = + lastPage?.messages?.[lastPage.messages.length - 1]; +const lastMessageId = lastMessage?.messageId; + +useEffect(() => { + if (lastMessageId && lastMessageRef.current) { + lastMessageRef.current.scrollIntoView({ + behavior: "smooth", + }); + } +}, [lastMessageId]);app/onboarding/_components/LoginScreen.tsx (1)
36-38: Redact sensitive tokens and URLs from logsAccess/refresh tokens and full URLs with query params are being logged. This is a security/PII risk in production and can leak secrets into logs.
Apply these changes to sanitize logs:
- console.log("현재 URL:", window.location.href); + // Avoid logging full URL as it may contain sensitive query params + console.log("현재 Pathname:", window.location.pathname);- console.log("URL 파라미터들:"); - console.log("- code:", code); - console.log("- error:", error); - console.log("- token:", token); - console.log("- state:", state); - console.log("- access_token:", accessTokenFromUrl); - console.log("- refresh_token:", refreshTokenFromUrl); + console.log("URL 파라미터들:"); + console.log("- code:", code); + console.log("- error:", error); + console.log("- token:", token ? "[REDACTED]" : null); + console.log("- state:", state); + console.log("- access_token:", accessTokenFromUrl ? "[REDACTED]" : null); + console.log("- refresh_token:", refreshTokenFromUrl ? "[REDACTED]" : null);- console.log("모든 URL 파라미터:"); - for (const [key, value] of urlParams.entries()) { - console.log(`- ${key}: ${value}`); - } + console.log("모든 URL 파라미터:"); + for (const [key, value] of urlParams.entries()) { + const safeValue = /token/i.test(key) ? "[REDACTED]" : value; + console.log(`- ${key}: ${safeValue}`); + }- console.log("✅ Access Token 저장됨:", accessTokenFromUrl); + console.log("✅ Access Token 저장됨: [REDACTED]");- console.log("✅ Refresh Token 저장됨:", refreshTokenFromUrl); + console.log("✅ Refresh Token 저장됨: [REDACTED]");Also applies to: 51-58, 60-63, 96-104
app/adding-goal/page.tsx (1)
96-118: Prevent default form submission to avoid page reload
onSubmitshould calle.preventDefault(); otherwise, the browser may attempt a navigation even in client components.Apply this diff:
- onSubmit={async () => { + onSubmit={async (e) => { + e.preventDefault(); const res = await goalApi.createGoal({ isPeriodByMonth: goalAddInfo.durationType === "month", title: goalAddInfo.goal, dueDate: goalAddInfo.durationType === "month" ? undefined : date2StringWithSpliter( goalAddInfo.durationValue as Date, "-", ), month: goalAddInfo.durationType === "month" ? (goalAddInfo.durationValue as number) : undefined, subGoals: goalAddInfo.subGoals.map((subGoalName) => ({ title: subGoalName, })), }); if (res) { router.back(); } }}components/shared/BottomSheets/TodoBottomSheet/TodoBottomSheet.tsx (1)
33-38: Type bug: TodoInfo.id references the wrong typeid should be string (same as submission id), not the todo string.
type TodoInfo = { - id?: TodoInfoForSubmission["todo"]; + id?: TodoInfoForSubmission["id"]; todo: TodoInfoForSubmission["todo"]; date?: TodoInfoForSubmission["date"]; subGoalTitle?: TodoInfoForSubmission["subGoalTitle"]; subGoalId?: TodoInfoForSubmission["subGoalId"]; };components/shared/BottomSheets/TodoResultBottomSheet/TodoResultBottomSheet.tsx (1)
29-34: Close via bottom-sheet store, not parent prop; streamline propsUse the centralized hook to close the sheet. Remove openBottomSheet/setOpenBottomSheet props and wire the close button to closeBottomSheet().
interface TodoResultBottomSheetProps { - openBottomSheet: boolean; - setOpenBottomSheet: (nextIsOpen: boolean) => void; onSubmit: (todoResult: TodoResult) => Promise<void>; - // hasBottomTabBar: boolean; }-const TodoResultBottomSheet = ({ - openBottomSheet, - setOpenBottomSheet, - onSubmit, - // hasBottomTabBar, -}: TodoResultBottomSheetProps) => { +import useBottomSheet from "@/hooks/useBottomSheet"; +import { useEffect, useState } from "react"; + +const TodoResultBottomSheet = ({ onSubmit }: TodoResultBottomSheetProps) => { + const { closeBottomSheet } = useBottomSheet(); const [todoResult, setTodoResult] = useState<TodoResult>(initTodoResult);- const resetBottomSheet = () => { - setOpenBottomSheet(false); - // setTodoResult(initTodoResult); - }; + const resetBottomSheet = () => { + closeBottomSheet(); + // setTodoResult(initTodoResult); + };- <div className="flex justify-center items-center w-6 h-6 relative overflow-hidden text-label-alternative"> + <div className="flex justify-center items-center w-6 h-6 relative overflow-hidden text-label-alternative"> <CloseSvg /> </div>Also applies to: 44-48, 51-55, 93-101
🧹 Nitpick comments (64)
components/main/GoalMenuContainer/GoalMenuContainer.tsx (1)
86-121: Commented-out list leaves dead code; memoize list and fix effect deps (ESLint warnings)With the goal list UI commented out, several measurements/drag pieces above are now effectively dead, and ESLint warns about unstable deps:
- Wrap goalMenuInfoList in useMemo to stabilize references and satisfy exhaustiveness.
- Fix the effect that updates the store to depend on goalMenuInfoList and selectedGoalIdx.
- Remove or guard the unused refs/measurements and motion usage until the list returns.
Follow-up (outside this range), suggested adjustments:
// 1) Memoize derived list const goalMenuInfoList = useMemo<GoalMenuInfo[]>( () => rawGoalData?.goals?.map((goalInfo) => ({ goal: goalInfo.title ?? "", percentage: goalInfo.progress ?? 0, goalId: goalInfo.id ?? "", })) ?? [], [rawGoalData?.goals], ); // 2) Correct effect deps useEffect(() => { updateGoalId(goalMenuInfoList[selectedGoalIdx]?.goalId ?? null); }, [goalMenuInfoList, selectedGoalIdx, updateGoalId]); // 3) Optionally remove/guard the sizing logic while list is disabled useEffect(() => { if (!containerRef.current || !contentRef.current) return; setContainerW(containerRef.current.offsetWidth); setContentW(contentRef.current.scrollWidth); }, [goalMenuInfoList.length]);Consider removing motion/useMotionValue imports and the related refs until the carousel returns, to keep the file lean.
components/shared/TodoItem/TodoItem.tsx (1)
118-135: Avoid hard-coded SVG stroke color; inherit from text color (tokens/Tailwind)Use currentColor for the stroke and drive color via Tailwind class using your design tokens. Also mark as decorative if it has no semantic meaning.
- <div className="self-stretch w-6 h-6 flex justify-center items-center relative bg-background-assistive rounded-full overflow-hidden"> + <div className="self-stretch w-6 h-6 flex justify-center items-center relative bg-background-assistive text-label-assistive rounded-full overflow-hidden" aria-hidden="true"> <svg - width="18" - height="18" + width="18" + height="18" viewBox="0 0 18 18" - fill="none" + fill="none" xmlns="http://www.w3.org/2000/svg" > <path d="M9 6.00009L3 12.0001V15.0001L6 15.0001L12 9.00008M9 6.00009L11.1515 3.8486L11.1528 3.84732C11.4489 3.55116 11.5973 3.40282 11.7683 3.34725C11.9189 3.29831 12.0812 3.29831 12.2318 3.34725C12.4027 3.40278 12.5509 3.55095 12.8466 3.84669L14.1515 5.15154C14.4485 5.44855 14.597 5.59713 14.6527 5.76837C14.7016 5.91901 14.7016 6.08127 14.6527 6.2319C14.5971 6.40302 14.4487 6.55137 14.1521 6.84796L14.1515 6.8486L12 9.00008M9 6.00009L12 9.00008" - stroke="#8A949E" + stroke="currentColor" strokeWidth="1.5" strokeLinecap="round" strokeLinejoin="round" /> </svg> </div>README.md (1)
50-66: Heading levels jump (MD001) and emphasis-as-heading (MD036)Use H3 under H2 for the feature subsections, not H4, and keep headings as actual headings (not bold). This improves accessibility and satisfies markdownlint.
-#### 🌟 **중장기 목표 설정** +### 🌟 중장기 목표 설정 ... -#### 🔥 **기록 남기기** +### 🔥 기록 남기기 ... -#### 💡 **그룹** +### 💡 그룹 ... -#### 🎯 **피드** +### 🎯 피드components/main/GoalCard/GoalCard.tsx (5)
198-201: Remove debug logging or guard itConsole logs in production spam the console. Remove or guard with a dev flag.
- //test - console.log( - "shouldTodoResultBottomSheetOpened: ", - shouldTodoResultBottomSheetOpened, - ); + // console.debug('[GoalCard] shouldTodoResultBottomSheetOpened:', shouldTodoResultBottomSheetOpened);
184-191: Effect deps incomplete (ESLint) and potentially stale closuresThe effect manipulates the TodoBottomSheet via functions returned by useBottomSheet, but they’re omitted from the deps. Either:
- Add all referenced functions/values to dependencies, or
- Wrap them in refs or integrate the open/update/close into a single stable callback.
At minimum, include: checkRendered, openTodoBottomSheet, updateTodoBottomSheet, closeTodoBottomSheet, and the subGoals reference you actually use (a memoized projection is safest).
129-139: Backdrops: state consistency and UX
- You set hasBackdrop to isActive initially, but you also flip hasBackdrop to false inside backdrop onClick. On the next render, the effect will overwrite hasBackdrop back to isActive, which may cause a flicker/inconsistency.
- Consider driving hasBackdrop solely from state (e.g., isActive) and let backdrop onClick only toggle the state (not hasBackdrop directly).
Also, inline style bottomSheetFixerStyle: { bottom: "56px" } violates the “Tailwind for styling” guideline. Prefer a CSS var or Tailwind utility:
- If BottomSheet accepts className, use bottom-14.
- If it requires a style, set a tokenized CSS var: style={{ bottom: "var(--bottom-tabbar-height)" }} and define the var in tokens.css.
Also applies to: 170-173
146-149: Animation timing via setTimeout: add cleanupsetTimeout without cleanup can update state after unmount. Track the timer and clear it.
// example pattern: const timeoutRef = useRef<number | null>(null); setIsActivated: (newState: boolean) => { if (timeoutRef.current) clearTimeout(timeoutRef.current); timeoutRef.current = window.setTimeout(() => setIsActive(newState), 400); }, useEffect(() => { return () => { if (timeoutRef.current) clearTimeout(timeoutRef.current); }; }, []);
386-414: Harden API submit: handle errors and type outcomes explicitlyTreat API failures explicitly and surface errors. swagger-typescript-api clients often throw on non-2xx; a try/catch yields clearer semantics.
-const handleTodoBottomSheetSubmit: ( +const handleTodoBottomSheetSubmit: ( newTodoInfo: TodoInfoForSubmission, afterSubmit: () => void, ) => Promise<boolean> = async (newTodoInfo, afterSubmit: () => void) => { - const isCreating = newTodoInfo.id ? false : true; - let fetchRes; - if (isCreating) { - fetchRes = await subGoalApi.createTodo(newTodoInfo.subGoalId, { - title: newTodoInfo.todo, - date: newTodoInfo?.date - ? date2StringWithSpliter(newTodoInfo?.date, "-") - : undefined, - }); - } else { - fetchRes = await todoApi.updateTodo(newTodoInfo.id ?? "", { - date: newTodoInfo.date - ? date2StringWithSpliter(newTodoInfo.date, "-") - : undefined, - title: newTodoInfo.todo, - }); - } - - const isFetchOk = fetchRes ? true : false; - if (isFetchOk) { - afterSubmit(); - } - - return isFetchOk; + try { + const isCreating = !newTodoInfo.id; + if (isCreating) { + await subGoalApi.createTodo(newTodoInfo.subGoalId, { + title: newTodoInfo.todo, + date: newTodoInfo?.date + ? date2StringWithSpliter(newTodoInfo?.date, "-") + : undefined, + }); + } else { + await todoApi.updateTodo(newTodoInfo.id ?? "", { + date: newTodoInfo.date + ? date2StringWithSpliter(newTodoInfo.date, "-") + : undefined, + title: newTodoInfo.todo, + }); + } + afterSubmit(); + return true; + } catch (e) { + // TODO: surface toast or error boundary if desired + return false; + } };mocks/guestMode/types.ts (3)
42-51: Remove redundant| undefinedin optional fieldIn TypeScript,
todoResultId?: IDalready impliesID | undefined. The explicit union is redundant.export interface DBTodo { id: ID; subGoalId?: ID; title: string; date?: string; // YYYY-MM-DD (optional) status?: TodoStatus; - todoResultId?: ID | undefined; + todoResultId?: ID; ownerId?: ID; // 소유자 (예: "me") createdAt?: string; }
81-86: Avoidanyinmessage.content; prefer a discriminated union
anydefeats type safety. If possible, modelmessageas a discriminated union keyed bytype, or useunknownand narrow where used. This will catch shape regressions at compile time.Example shape:
type GroupMessageContent = | { type: "JOIN"; content: { userName: string } } | { type: "LEAVE"; content: { userName: string } } | { type: "TODO_RESULT_SUBMIT"; content: TodoResultSubmittedContent } | { type: "TODO_COMPLETE"; content: TodoCompletedContent } | { type: "MESSAGE_REACTION"; content: MessageReactionContent }; export interface DBGroupMessage { // ... message?: GroupMessageContent; // ... }
22-27: Introduce an explicit date-string alias for consistencyMultiple fields carry date strings. Consider a reusable alias to document the expected format and ease future validation/migration.
export type ISODateString = string; // e.g., "YYYY-MM-DD" or ISO 8601 // Examples: dueDate?: ISODateString; createdAt?: ISODateString; date?: ISODateString; joinedAt?: ISODateString;Also applies to: 46-51, 59-60, 66-67, 74-75, 86-87, 103-104, 110-111
components/main/GoalData/GoalData.tsx (1)
31-38: Clamp progress between 0 and 100 to prevent overflow/negative widthsProtect against invalid values to avoid visual glitches and potential layout overflow.
<div className="flex-1 h-2 relative bg-background-normal rounded-[999px] overflow-hidden"> <div className={` h-2 left-0 top-[0.50px] absolute bg-background-primary rounded-[999px]`} - style={{ width: `${progress}%` }} + style={{ width: `${Math.min(100, Math.max(0, progress))}%` }} ></div> </div> <p className="flex justify-center text-label-alternative text-sm font-medium font-['SUIT_Variable'] leading-none"> - {`${progress}%`} + {`${Math.min(100, Math.max(0, progress))}%`} </p>components/group/groupRoom/GroupChatRoom/GroupChatRoom.tsx (3)
239-255: Handle API errors when leaving a reactionWrap the mutation in try/catch to avoid unhandled rejections and improve UX on failure.
- onLeaveReaction={async (selectedType) => { - const emotionEnumVal = Object.entries( - reactionTypeMaps, - ).find( - ([key, val]) => val === selectedType, - )![0] as unknown as UpsertGroupReactionParamsTypeEnum; - - const res = await groupApi.upsertGroupReaction( - messageInfo.messageId, - { - type: emotionEnumVal, - }, - ); - if (res) { - closeModal(); - mutate(); - } - }} + onLeaveReaction={async (selectedType) => { + try { + const emotionEnumVal = Object.entries(reactionTypeMaps).find( + ([, val]) => val === selectedType, + )![0] as unknown as UpsertGroupReactionParamsTypeEnum; + + const res = await groupApi.upsertGroupReaction( + messageInfo.messageId, + { type: emotionEnumVal }, + ); + if (res) { + closeModal(); + mutate(); + } + } catch (e) { + // TODO: surface an error toast + console.error("upsertGroupReaction failed", e); + } + }}
53-54: Type theobservingFirstRefrefAvoid
anyby specifying the element type.- const observingFirstRef = useRef(null); + const observingFirstRef = useRef<HTMLDivElement | null>(null);
98-107: Avoid O(n·m) search forreferUserNameif reactions are frequentThe reduce + findIndex scans all pages per reaction. If this runs often or pages are large, consider precomputing a Map<messageId, userName> once per data fetch.
components/group/GroupChatItem.tsx (2)
51-58: Replace hardcoded hex colors with design tokens (tokens.css)Per guidelines, prefer tokens over raw hex (e.g., #33363D, #F7F7F8). You’re already using tokens like bg-background-assistive and text-label-normal elsewhere—apply them consistently here for theming parity and maintainability.
If you share a mapping from your design tokens to these shades, I can generate a targeted diff replacing each hex with the closest token utility.
Also applies to: 153-168, 174-180
211-212: Unify font utility usageUse the same
font-SUIT_Variableutility you use elsewhere instead of inline font family syntax for consistency.- <p className="font-['SUIT_Variable']"> Invalid URL path</p> + <p className="font-SUIT_Variable">Invalid URL path</p>mocks/server.ts (1)
3-5: Don’t hard-switch to guest handlers for all Node/MSW contexts; gate or compose handlers.Switching to
guestHandlersglobally will break tests or dev flows that expect the non-guest mocks. Prefer an env-gated selection (or compose both) so you can run either profile without editing code.Apply this diff:
-// import { handlers } from "./handlers"; +import { handlers } from "./handlers"; import guestHandlers from "./guestMode/handlers"; -export const server = setupServer(...guestHandlers); +const isGuestMSW = process.env.NEXT_PUBLIC_MSW_PROFILE === "guest"; +const allHandlers = isGuestMSW ? guestHandlers : handlers; + +export const server = setupServer(...allHandlers);Ensure your test/dev scripts set
NEXT_PUBLIC_MSW_PROFILE=guestwhen you want guest-mode mocks. Otherwise, the default remains the full/auth handlers.mocks/browser.ts (1)
3-5: Mirror Node-side gating for browser worker to avoid locking into guest-only mocks.Like the Node server, prefer choosing the handler set via an env profile (or compose them) so you can run either mode without editing code.
Apply this diff:
-// import { handlers } from "./handlers"; +import { handlers } from "./handlers"; import guestHandlers from "./guestMode/handlers"; -export const worker = setupWorker(...guestHandlers); +const isGuestMSW = process.env.NEXT_PUBLIC_MSW_PROFILE === "guest"; +const allHandlers = isGuestMSW ? guestHandlers : handlers; + +export const worker = setupWorker(...allHandlers);Note: If you eventually need both sets active simultaneously, ensure route patterns don’t collide or control precedence explicitly.
Confirm your dev startup path exports/uses
NEXT_PUBLIC_MSW_PROFILE=guestwhen testing guest mode (and omits it otherwise), to align withGuestModeHandler’s conditional start/stop.app/page.tsx (1)
10-10: GoalDataContainer integration and layout adjustments look good.Replacing
GoalCardwithGoalDataContainerand addingmin-h-screen pb-14resolves the bottom bar overlap and maintains scroll height. Consider removing the commentedGoalCardonce the new container is fully vetted.Also applies to: 34-36
app/onboarding/_components/LoginScreen.tsx (2)
174-180: Guest browse likely shouldn’t call login(); consider removing to avoid inconsistent auth stateCalling login() in the browse path can flip isLoggedIn to true while simultaneously marking the session as guest, which may confuse downstream code and trigger authenticated API calls without tokens.
If guest mode should be unauthenticated, remove the login() call:
const handleBrowse = () => { - // TODO: Handle browse without login + // Guest browse: clear auth state and mark guest reset(); - login(); setIsGuest(true); onNext(); };If login() intentionally enables certain UI while in guest mode, please confirm the store semantics (isLoggedIn vs isGuest) so we can harden checks where needed.
154-156: Use a single source of truth for FRONTEND_BASE_URLYou import FRONTEND_BASE_URL but use process.env.NEXT_PUBLIC_FRONTEND_BASE_URL inline. Prefer one consistent constant.
- const redirect_uri = `${process.env.NEXT_PUBLIC_FRONTEND_BASE_URL}/onboarding`; + const redirect_uri = `${FRONTEND_BASE_URL}/onboarding`;- const redirect_uri = `${process.env.NEXT_PUBLIC_FRONTEND_BASE_URL}/onboarding`; + const redirect_uri = `${FRONTEND_BASE_URL}/onboarding`;Also applies to: 170-172
app/group/page.tsx (3)
26-27: Trim unused store actions from destructuresetIsGuest and logout aren’t used here.
- const { isGuest, setIsGuest, logout, reset } = useAuthStore(); + const { isGuest, reset } = useAuthStore();
43-44: Avoid hardcoding the persist storage keyHardcoding "auth-storage" couples this page to the store’s internal key. Prefer a store helper to purge/persist-reset, or export the key from the store and import it here.
Would you like me to add an explicit purge method to useAuthStore (e.g., purgePersist()) so pages don’t reach into localStorage directly?
36-54: Suppress SWR fetches in guest mode by null-key gatingSWR doesn’t support a React-Query-style
enabledflag—instead it skips fetching whenever the key isnull. You can expose anenabledoption in your hooks (or add an internal guard) so that whenisGuestis true, you passparamsasnull, e.g.:• In api/hooks.ts, around your
useQueryobject entries forgoalsNotInGroupandjoinedGroups, change:// before goalsNotInGroup: (config?: SWRConfiguration) => useApiQuery("목표Api", "getGoalsNotInGroup", [], undefined, config), joinedGroups: (config?: SWRConfiguration) => useApiQuery("그룹Api", "joinedGroups", [], undefined, config),to:
// after goalsNotInGroup: ({ enabled = true, ...config }: { enabled?: boolean } & SWRConfiguration = {}) => useApiQuery( "목표Api", "getGoalsNotInGroup", enabled ? [] : null, undefined, config, ), joinedGroups: ({ enabled = true, ...config }: { enabled?: boolean } & SWRConfiguration = {}) => useApiQuery( "그룹Api", "joinedGroups", enabled ? [] : null, undefined, config, ),• In app/group/page.tsx, invoke with your guard:
const { isGuest } = useAuthStore(); const { data: joinedGroups } = useJoinedGroups({ enabled: !isGuest }); const { data: pendingGroups } = useGoalsNotInGroup({ enabled: !isGuest });This leverages SWR’s null-key behavior to prevent any network calls (and avoid 401s) when in guest mode.
components/main/GoalDataContainer/GoalDataContainer.tsx (3)
56-63: Use valid Tailwind width classes (w-82 isn’t standard)w-82 is not a default Tailwind utility and will be ignored unless extended. Use w-full or an arbitrary value like w-[328px].
- <div className="w-82 h-[130px] bg-background-alternative rounded-lg outline-1 outline-offset-[-1px] outline-line-normal inline-flex flex-col justify-center items-center gap-3"> + <div className="w-full h-[130px] bg-background-alternative rounded-lg outline-1 outline-offset-[-1px] outline-line-normal inline-flex flex-col justify-center items-center gap-3">If you intended a specific pixel width, replace w-full with w-[].
51-51: Decide on a user-friendly D-day fallbackPassing NaN to GoalData may render oddly. Consider a semantic fallback (e.g., undefined or 0) and have GoalData render “—” or hide D-day when dueDate is absent.
Do you want me to update GoalData to handle undefined dDay gracefully and adjust this component accordingly?
1-68: Add a Storybook story for GoalDataContainerPer team conventions, new components should ship with stories. A simple “Primary” story rendering a few mocked goals would help visual regression and design review.
I can scaffold components/main/GoalDataContainer/GoalDataContainer.stories.tsx with controls and mocked data. Want me to open a PR chunk for this?
components/details/BottomSheets/GoalDurationBottomSheet/GoalDurationBottomSheet.stories.tsx (2)
27-36: Make close button actually hide the component in Storybook (wrap with conditional rendering)Currently the close button calls setOpen(false) but the component remains mounted unconditionally. Wrap the component with a local open state so the close button affects visibility. Also, type args without referring to Primary to avoid circular type usage.
-import { useState } from "react"; +import { useState, type ComponentProps } from "react"; @@ -const Wrapper = ({ args }: { args: typeof Primary.args }) => { - const [open, setOpen] = useState(true); - return ( - <GoalDurationBottomSheet - {...args} - // openBottomSheet={open} - setopenBottomSheet={setOpen} - /> - ); -}; +const Wrapper = ({ + args, +}: { + args: ComponentProps<typeof GoalDurationBottomSheet>; +}) => { + const [open, setOpen] = useState(true); + return open ? ( + <GoalDurationBottomSheet {...args} setopenBottomSheet={setOpen} /> + ) : ( + <button type="button" onClick={() => setOpen(true)}>Open Bottom Sheet</button> + ); +};Also applies to: 5-5
55-58: Drop redundant setopenBottomSheet default in argsWrapper explicitly overrides setopenBottomSheet, so the default here is unused and can confuse readers.
args: { onEdit: () => {}, - // openBottomSheet: true, - setopenBottomSheet: () => {}, // Primary 스토리에만 적용될 Props initDate: undefined, initMonth: undefined, },components/shared/BottomSheets/TodoBottomSheet/TodoBottomSheet.stories.tsx (1)
12-17: Update docs description to reflect portal-free rendering in this storyThe story now renders the component directly (without a Drawer/portal), so the warning that Docs won’t display correctly is outdated.
parameters: { // Canvas 레이아웃을 중앙으로 정렬하거나 패딩을 추가할 수 있습니다. layout: "centered", docs: { description: { - component: - "docs에서는 제대로 보이지 않습니다. portal을 사용해서요. 직접 스토리에 들어가서 확인하세요.", + component: + "이 스토리는 포털 없이 직접 렌더링합니다. Docs와 Canvas 모두에서 확인할 수 있습니다.", }, }, },components/details/BottomSheets/GoalDurationBottomSheet/GoalDurationBottomSheet.tsx (3)
105-110: Close the sheet after applying editsIf the desired UX is to close after saving, call setopenBottomSheet(false) after onEdit. Keeps behavior consistent with other bottom sheets.
<button onClick={() => { onEdit({ type: durationType, value: durationType === "month" ? selectedMonth : selectedDate, }); + setopenBottomSheet(false); }} type="button"
33-41: Remove deprecated Drawer remnants and unused importLarge commented blocks and an unused Drawer import add noise and risk confusion during maintenance. Since orchestration moved to a central renderer, this component should be lean content only.
-import { Drawer } from "vaul"; @@ - {/* <Drawer.Root open={openBottomSheet}> - <Drawer.Portal> - <Drawer.Overlay - className="fixed inset-0 z-20 bg-neutral-700/50 " - onClick={() => { - setopenBottomSheet(false); - }} - /> - <div className="flex justify-center w-[100vw] relative"> */} - <div - // <Drawer.Content - className=" - z-30 - - w-[360px] h-[585px] bg-background-alternative rounded-tl-lg rounded-tr-lg shadow-[0px_0px_4px_0px_rgba(0,0,0,0.24)] overflow-hidden - flex flex-col items-center - pt-5 pl-4 pr-4 pb-6 - " - // className=" - // z-30 - // bottom-0 - // w-[360px] h-[585px] bg-background-alternative rounded-tl-lg rounded-tr-lg shadow-[0px_0px_4px_0px_rgba(0,0,0,0.24)] overflow-hidden - // flex flex-col items-center - // pt-5 pl-4 pr-4 pb-6 - // fixed - // " - > - {/* <Drawer.Title className="invisible"></Drawer.Title> */} + <div + className=" + z-30 + w-[360px] h-[585px] bg-background-alternative rounded-tl-lg rounded-tr-lg shadow-[0px_0px_4px_0px_rgba(0,0,0,0.24)] overflow-hidden + flex flex-col items-center + pt-5 pl-4 pr-4 pb-6 + " + > @@ - {/* </Drawer.Content> */} </div> - {/* </div> - </Drawer.Portal> - </Drawer.Root> */}Also applies to: 42-59, 60-61, 120-125, 1-1
13-15: Clean up commented prop remnantsRemove commented openBottomSheet references to keep the public API clear.
interface GoalDurationBottomSheetProps { initMonth?: number; initDate?: Date; - // openBottomSheet: boolean; setopenBottomSheet: (newOpenBottomSheetStatus: boolean) => void; @@ initDate, initMonth, - // openBottomSheet, setopenBottomSheet, onEdit, }: GoalDurationBottomSheetProps) => {Also applies to: 24-26
components/details/Modals/ModalAddingTodo/ModalAddingTodo.tsx (1)
9-11: Verify ModalCommon typing import or ambient declaration
ModalCommonis referenced but not imported. If it’s not globally declared, TypeScript will error. Import the type from its source or switch to an explicit props shape.Do you want me to scan the repo and wire the correct import?
app/adding-goal/page.tsx (2)
29-35: Remove unusedcheckRenderedto satisfy lint and reduce noise
checkRenderedis destructured but never used.Apply this diff:
const { checkOpened, - checkRendered, closeBottomSheet, openBottomSheet: openGoalDurationBottomSheet, updateBottomSheet: updateGoalDurationBottomSheet, } = useBottomSheet<typeof GoalDurationBottomSheet>();
235-247: Dead/commented bottom-sheet JSX can be removedThis commented block is superseded by the hook-driven sheet. Consider removing to keep the file lean.
components/group/Guest/GuestGroup/GuestGroup.tsx (5)
2-2: Add PanInfo typing for drag handlerAvoid
anyand usePanInfofor stronger type-safety.Apply this diff:
-import { motion, AnimatePresence } from "motion/react"; +import { motion, AnimatePresence } from "motion/react"; +import type { PanInfo } from "motion";
80-95: Type the drag handler and drop the unused event paramThis reduces implicit any usage and tightens intent.
Apply this diff:
- const handleDragEnd = (event: any, info: any) => { + const handleDragEnd = (_: MouseEvent | TouchEvent | PointerEvent, info: PanInfo) => { const threshold = 50; // 드래그 거리 기준을 조금 늘림 const { offset } = info;
121-129: Use design tokens instead of raw color classesReplace
text-blackwith tokenized text color (e.g.,text-label-strong) to align with tokens.css usage across components.Apply this diff:
- <h2 className="whitespace-pre-wrap w-80 text-center justify-center text-black text-xl font-bold font-['SUIT_Variable'] leading-normal"> + <h2 className="whitespace-pre-wrap w-80 text-center justify-center text-label-strong text-xl font-bold font-['SUIT_Variable'] leading-normal">
149-151: Tokenize secondary text colorSame rationale as the title: avoid raw
text-black.Apply this diff:
- <p className="whitespace-pre-wrap text-center self-stretch justify-center text-black text-base font-bold font-['SUIT_Variable'] leading-tight"> + <p className="whitespace-pre-wrap text-center self-stretch justify-center text-label-strong text-base font-bold font-['SUIT_Variable'] leading-tight">
127-127: Verify custom Tailwind classes exist (h-100,w-50)These aren’t default Tailwind utilities. If not defined in tailwind.config, they’ll be no-ops. Prefer arbitrary values (e.g.,
h-[100px],w-[200px]) or tokens-based sizing.Also applies to: 168-168
app/details/edit/[goal_id]/_components/GoalEdit/GoalEdit.tsx (2)
49-57: Align useBottomSheet typing and remove unusedcheckRenderedUse the same typing pattern as other usages and drop dead code to avoid type/lint drift.
Apply this diff:
// GoalDurationBottomSheet 관리 const { checkOpened, - checkRendered, closeBottomSheet, openBottomSheet: openGoalDurationBottomSheet, updateBottomSheet: updateGoalDurationBottomSheet, - } = useBottomSheet<Parameters<typeof GoalDurationBottomSheet>[0]>(); + } = useBottomSheet<typeof GoalDurationBottomSheet>();
170-183: Remove legacy commented JSX to keep component leanHook-based bottom sheet fully replaces this block.
mocks/guestMode/handlers.ts (2)
122-124: Remove debug logging from handlersLeaking console logs from MSW handlers is noisy and can pollute test output.
Apply this diff:
- //test - console.log("goal in handler: ", goals);
517-524: Optional: return stored points if presentYou’re reading
pointsbut always returning{ point: 0 }. Consider returning the first stored value when available.Apply this diff:
- return HttpResponse.json( - { point: 0 } as PointRs, - // (points[0] as PointRs) || ({ point: 0 } as PointRs), - ); + return HttpResponse.json((points[0] as PointRs) || ({ point: 0 } as PointRs));app/_components/BottomSheetRenderer.tsx (2)
11-17: Stabilize keys and fix typo; use top entry id for consistent remount behavior
- Typo: keyCandinates → keyCandidates.
- Use the top history entry’s id as the Drawer.Root key for consistent remounting when pushing/popping sheets.
Apply this diff:
- const { + const { + id: topId, hasBackdrop, backdropProps, ContentComponent, contentProps, bottomSheetFixerStyle, } = bottomSheetInfoHistory?.[bottomSheetInfoHistory.length - 1] ?? {}; - const keyCandinates = ["one", "other"]; + const keyCandidates = ["one", "other"]; const [keyIdx, setKeyIdx] = useState(0); const isBottomSheetOpen = !!ContentComponent;- <Drawer.Root - key={`${ContentComponent}`} + <Drawer.Root + key={topId ?? "no-sheet"} open={isBottomSheetOpen} defaultOpen={false} handleOnly disablePreventScroll={isBottomSheetOpen && !hasBackdrop} >- <ContentComponent - key={keyCandinates[keyIdx]} + <ContentComponent + key={keyCandidates[keyIdx]} {...contentProps} />Also applies to: 19-25, 63-69, 82-85
63-69: Optional: close on swipe/backdrop via onOpenChange and pop historyHook Drawer.Root’s onOpenChange to pop the top of BottomSheetInfoHistory when it becomes false. This keeps gestures/backdrop interactions in sync with the store.
Example patch (requires adding the updater selector):
- const bottomSheetInfoHistory = useBottomSheetStore( - (state) => state.BottomSheetInfoHistory, - ); + const bottomSheetInfoHistory = useBottomSheetStore( + (state) => state.BottomSheetInfoHistory, + ); + const updateBottomSheetInfoHistory = useBottomSheetStore( + (state) => state.updateBottomSheetInfoHistory, + );<Drawer.Root key={topId ?? "no-sheet"} open={isBottomSheetOpen} defaultOpen={false} handleOnly disablePreventScroll={isBottomSheetOpen && !hasBackdrop} + onOpenChange={(next) => { + if (!next) { + updateBottomSheetInfoHistory((prev) => + prev.length > 0 ? prev.slice(0, -1) : prev, + ); + } + }} >hooks/useBottomSheet.ts (2)
58-63: Doc vs behavior mismatch: updateBottomSheet only updates the top sheetThe comment says it updates even if not rendering, but the code requires the item to be the top of the stack. Align the comment or relax the condition.
Apply this diff to align the comment with current behavior:
/** - * 바텀시트를 업데이트 합니다. 해당 바텀시트가 렌더링 되고 있지 않아도 props등은 업데이트 됩니다. - * @param newBottomSheetInfo 업데이트할 bottomsheetinfo. id값이 필요합니다. + * 바텀시트를 업데이트 합니다. 현재 렌더링(스택 top) 중인 항목에만 적용됩니다. + * @param newBottomSheetInfo 업데이트할 bottomsheetinfo * @returns id에 매칭되는 바텀시트가 존재하면 업데이트 후 true반환. 아니면 false 반환. */Also applies to: 71-80
42-56: Return the created id from openBottomSheet (or update the JSDoc)Callers may need the id; the comment promises it. Return it to avoid confusion.
- const openBottomSheet = ( + const openBottomSheet = ( newBottomSheetInfo: NewBottomSheetInfo<TContentProps>, - ) => { + ) => { // // const newBottomSheetId = crypto.randomUUID(); const newBottomSheetId = uuidV4(); updateBottomSheetInfoHistory((prev) => { const newHistory = [ ...prev, { ...newBottomSheetInfo, id: newBottomSheetId }, ]; return newHistory; }); setId(newBottomSheetId); + return newBottomSheetId; };components/shared/BottomSheets/TodoBottomSheet/TodoBottomSheet.tsx (2)
3-3: Remove unused importDrawer is no longer used in this component.
-import { Drawer } from "vaul";
134-143: Replace inline styles with Tailwind classes for consistency with guidelinesAvoid inline style for height/transition. Use Tailwind conditional classes.
- <div - // <Drawer.Content - className={` - w-[360px] - z-30 pl-4 pr-4 bg-white flex flex-col justify-start rounded-t-[10px]`} - style={{ - height: isActivated ? (showDate ? "382px" : "100px") : "66px", // 고정값 강제 - transition: "height 0.3s ", - bottom: 0, - }} - > + <div + className={`w-[360px] z-30 pl-4 pr-4 bg-white flex flex-col justify-start rounded-t-[10px] transition-[height] duration-300 ${ + isActivated ? (showDate ? "h-[382px]" : "h-[100px]") : "h-[66px]" + }`} + >components/shared/BottomSheets/TodoResultBottomSheet/TodoResultBottomSheet.tsx (2)
1-1: Remove unused Drawer importThis component no longer uses Drawer.
-import { Drawer } from "vaul";
156-166: Revoke object URLs to avoid memory leaksWhen previewing images via URL.createObjectURL, revoke them on change/unmount.
Add this effect near component bottom:
useEffect(() => { return () => { if (todoResult.imagePreviewUrl?.startsWith("blob:")) { URL.revokeObjectURL(todoResult.imagePreviewUrl); } }; }, [todoResult.imagePreviewUrl]);api/service.ts (1)
20-24: Guest mode behavior OK; consider always setting format to 'json'For consistency, you can set format: "json" in the default branch to avoid response type surprises.
- return {}; + return { format: "json" };stores/useBottomSheetStore.ts (2)
25-31: Prefer using Zustand'sgetinstead of referencing the store inside its own initializerAvoids potential circular references and is the conventional pattern with Zustand. This also improves type-safety.
-const useBottomSheetStore = create<BottomSheetStore<any>>((set) => ({ +const useBottomSheetStore = create<BottomSheetStore<any>>((set, get) => ({ BottomSheetInfoHistory: [], updateBottomSheetInfoHistory: (newBottomSheetInfoHistoryOrFunc) => { if (typeof newBottomSheetInfoHistoryOrFunc === "function") { - const prev = useBottomSheetStore.getState().BottomSheetInfoHistory; + const prev = get().BottomSheetInfoHistory; const newHistory = newBottomSheetInfoHistoryOrFunc(prev); set({ BottomSheetInfoHistory: newHistory }); return; } set({ BottomSheetInfoHistory: newBottomSheetInfoHistoryOrFunc }); }, }));
7-15: TypeContentComponentas a React componentUsing
React.ComponentType<TProps>better communicates intent and integrates with JSX tooling.interface BottomSheetStore<TContentProps> { BottomSheetInfoHistory: { id: string; - ContentComponent: (params: TContentProps) => ReactNode; + ContentComponent: React.ComponentType<TContentProps>; contentProps: TContentProps; hasBackdrop: boolean; backdropProps: Parameters<typeof Overlay>[0]; bottomSheetFixerStyle?: HTMLAttributes<HTMLDivElement>["style"]; }[];components/details/ListCard/ListCard.tsx (4)
96-146: Fix missing dependencies inuseEffectto avoid stale closures and ESLint warningsThe effect captures several functions/values but doesn't list them in the dependency array. This can lead to stale references and is flagged by ESLint.
- }, [openBottomSheet, todoIdForResult]); + }, [ + openBottomSheet, + todoIdForResult, + checkRendered, + checkOpened, + openTodoRootBottomSheet, + updateTodoResultBottomSheet, + closeBottomSheet, + mutate, + ]);
102-113: Backdrop props are provided whilehasBackdropis falseIf you expect the overlay to render and be clickable,
hasBackdropshould be true; otherwisebackdropPropsare inert. Confirm intended behavior.
242-281: Use the imported PlusSvg or remove the unused import; avoid inline SVG duplicationKeeps icon usage consistent and reduces inline SVG duplication.
- <div className="w-8 h-8"> + <div className="w-8 h-8"> <button onClick={() => { openModal( <ModalAddingTodo onAddTodo={async (newTodo) => { if (!subGoalInfo.id) return; const res = await subGoalApi.createTodo(subGoalInfo.id, { title: newTodo, }); if (res) { closeModal(); mutate(); } }} onClose={closeModal} />, ); }} className="w-full h-full rounded-full bg-background-normal flex justify-center items-center" type="button" > - <svg - width="20" - height="20" - viewBox="0 0 20 20" - fill="none" - xmlns="http://www.w3.org/2000/svg" - > - <path - d="M5 10H10M10 10H15M10 10V15M10 10V5" - stroke="#464C53" - strokeWidth="1.5" - strokeLinecap="round" - strokeLinejoin="round" - /> - </svg> + <PlusSvg width={20} height={20} /> </button> </div>
150-231: Remove or consolidate large commented blocksThe commented sections add noise and make maintenance harder. Consider removing them or moving them to a doc/notes if needed.
Also applies to: 329-356
app/details/[goal_id]/_components/DetailsBody/DetailBody.tsx (3)
91-95: IncludeopenModalCompletingGoalin the dependency arrayPrevents stale reference to the callback and addresses the ESLint warning.
- }, [allSubGoalCompleted, goalDetail?.isCompleted]); + }, [allSubGoalCompleted, goalDetail?.isCompleted, openModalCompletingGoal]);
108-166: Fix missing dependencies and avoid closing the bottom sheet on every rerun
- Add all referenced functions/values to the dependency array to avoid stale closures.
- The cleanup unconditionally closing the bottom sheet can cause flicker; the effect already handles closing when
shouldBottomSheetOpenedis false. Remove the cleanup unless you truly want to force-close on unmount only.useEffect(() => { const isRendered = checkRendered(); if (!shouldBottomSheetOpened && isRendered) { closeBottomSheet(); return; } const bottomSheetInfo: Parameters<typeof openBottomSheet>[0] = { backdropProps: { onClick: () => { // 내용물을 초기화 해야 함. -> key값 바꿔도 애니메이션이나 바텀시트 높이 정상적일까? setIsActive(false); updateBottomSheet((prev) => { return { ...prev, hasBackdrop: false }; }); }, className: "fixed inset-0 bg-black/20 z-20", }, ContentComponent: TodoBottomSheet, contentProps: { isActivated: isActive, initTodoInfo: initContent, setIsActivated: setIsActive, subGoals: data.subGoals?.map((subGoalInfo) => ({ id: subGoalInfo.subGoalId ?? "", title: subGoalInfo.subGoal ?? "", })) ?? [], onSubmitTodo: async (newTodoInfo) => { const afterSubmit = () => { mutate(); // 바텀시트 리셋 setIsActive(false); }; const res = await handleTodoBottomSheetSubmit( newTodoInfo, afterSubmit, ); return res; }, }, hasBackdrop: isActive, bottomSheetFixerStyle: { bottom: "0px" }, }; if (shouldBottomSheetOpened && !isRendered) { openBottomSheet(bottomSheetInfo); return; } updateBottomSheet(bottomSheetInfo); - return () => { - closeBottomSheet(); - }; - }, [isActive, initContent, data.subGoals?.length, shouldBottomSheetOpened]); + }, [ + isActive, + initContent, + shouldBottomSheetOpened, + // functions/values used inside effect + checkRendered, + openBottomSheet, + updateBottomSheet, + closeBottomSheet, + setIsActive, + mutate, + data.subGoals, + ]);
69-72: Guard against NaN in D-day calculation to avoid showing “D+NaN”When
dueDateis absent,leftDaybecomesNaN. Provide a safe label.const leftDay = goalDetail?.dueDate?.dueDate ? calcLeftDay(goalDetail?.dueDate.dueDate) : NaN; + + const ddayLabel = Number.isFinite(leftDay) + ? `D${leftDay >= 0 ? "-" : "+"}${leftDay !== 0 ? Math.abs(leftDay) : "Day"}` + : "D-";- <p className="justify-center text-label-primary text-base font-bold font-['SUIT_Variable'] leading-tight">{`D${leftDay >= 0 ? "-" : "+"}${leftDay !== 0 ? Math.abs(leftDay) : "Day"}`}</p> + <p className="justify-center text-label-primary text-base font-bold font-['SUIT_Variable'] leading-tight"> + {ddayLabel} + </p>Also applies to: 177-183
| managerHead: (head) => ` | ||
| ${head} | ||
| <link rel="icon" type="image/png" href="/app/icon.png" /> | ||
| `, |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Favicon path likely 404s in Storybook; point to /public instead of /app
Storybook serves static assets from staticDirs = ../public, so /app/icon.png won’t be resolvable. Use /icon.png or add the app folder to staticDirs.
managerHead: (head) => `
${head}
- <link rel="icon" type="image/png" href="/app/icon.png" />
+ <link rel="icon" type="image/png" href="/icon.png" />
`,Run this to verify where icon.png lives:
Expected: public/icon.png exists; if it’s instead at app/icon.png, either move it to public or add path.resolve(__dirname, "../app") to staticDirs.
🏁 Script executed:
#!/bin/bash
fd -a --strip-cwd-prefix -t f -g "icon.png"Length of output: 75
Fix broken Storybook favicon by updating staticDirs and link path
Icon.png lives in app/icon.png, but Storybook only serves ../public by default. You’ll need to include the app folder in staticDirs and point the link at /icon.png (or configure a custom mount if you want to preserve /app/icon.png).
• File: .storybook/main.ts
• Add your app directory to staticDirs and change the link href:
--- a/.storybook/main.ts
+++ b/.storybook/main.ts
@@ export default {
- staticDirs: [path.resolve(__dirname, '../public')],
+ staticDirs: [
+ path.resolve(__dirname, '../public'),
+ path.resolve(__dirname, '../app'),
+ ],
@@
- managerHead: (head) => `
- ${head}
- <link rel="icon" type="image/png" href="/app/icon.png" />
- `,
+ managerHead: (head) => `
+ ${head}
+ <link rel="icon" type="image/png" href="/icon.png" />
+ `,If you prefer to keep the /app prefix, you can instead use:
staticDirs: [
path.resolve(__dirname, '../public'),
{ from: path.resolve(__dirname, '../app'), to: '/app' },
],
// then keep href="/app/icon.png"🤖 Prompt for AI Agents
In .storybook/main.ts around lines 7 to 10 the Storybook managerHead injects a
link to /app/icon.png but Storybook only serves ../public by default so the
favicon is broken; update the Storybook config to add the app directory to
staticDirs and change the link href to /icon.png (or alternatively add a
staticDirs mapping from the app folder to the /app mount and keep
href="/app/icon.png"); specifically, add the app path to staticDirs (or a {
from: appPath, to: '/app' } entry) and update the managerHead link to point to
the served path accordingly.
| const debounceer = <T, E>(apiRequest: typeof httpClient.request<T, E>) => { | ||
| const timeLimit = 1000; | ||
| let timer: number; | ||
| let rejectTimer: (reason?: any) => void; | ||
| return ( | ||
| requestParams: Parameters<typeof httpClient.request<T, E>>[0], | ||
| ): ReturnType<typeof httpClient.request<T>> => { | ||
| if (timer) { | ||
| clearTimeout(timer); | ||
| rejectTimer("debouncing"); | ||
| } | ||
| const apiRes: Promise<T> = new Promise((resolve, reject) => { | ||
| rejectTimer = reject; | ||
| timer = Number( | ||
| setTimeout(async () => { | ||
| try { | ||
| const res = apiRequest(requestParams); | ||
| resolve(res); | ||
| } catch (error) { | ||
| console.error(error); | ||
| showToast(`API ERROR`, new Date()); | ||
| } | ||
| }, timeLimit), | ||
| ); | ||
| }); | ||
| return apiRes; | ||
| }; | ||
| }; | ||
|
|
||
| httpClient.request = debounceer(httpClient.request); | ||
|
|
There was a problem hiding this comment.
💡 Verification agent
❓ Verification inconclusive
Debouncer breaks typing, loses this context, and doesn’t catch async errors; fix implementation
- TypeScript: typeof httpClient.request<T, E> is invalid; generics can’t be applied to typeof result like that.
- Context: calling apiRequest(...) may lose HttpClient’s this. Use the client instance.
- Error handling: try/catch around a non-awaited promise won’t catch network errors.
- Global debounce drops every request in the window; verify this is intended. Consider scoping by method+URL.
Apply this diff to fix typing, context, and error handling:
-// Debouncer 감싸도 될 것 같은데?
-const debounceer = <T, E>(apiRequest: typeof httpClient.request<T, E>) => {
- const timeLimit = 1000;
- let timer: number;
- let rejectTimer: (reason?: any) => void;
- return (
- requestParams: Parameters<typeof httpClient.request<T, E>>[0],
- ): ReturnType<typeof httpClient.request<T>> => {
- if (timer) {
- clearTimeout(timer);
- rejectTimer("debouncing");
- }
- const apiRes: Promise<T> = new Promise((resolve, reject) => {
- rejectTimer = reject;
- timer = Number(
- setTimeout(async () => {
- try {
- const res = apiRequest(requestParams);
- resolve(res);
- } catch (error) {
- console.error(error);
- showToast(`API ERROR`, new Date());
- }
- }, timeLimit),
- );
- });
- return apiRes;
- };
-};
-
-httpClient.request = debounceer(httpClient.request);
+// Global debouncer (verify this behavior is desired)
+const debounceRequests = (client: HttpClient, ms = 1000): HttpClient["request"] => {
+ let timer: ReturnType<typeof setTimeout> | undefined;
+ let rejectTimer: ((reason?: unknown) => void) | undefined;
+
+ return async function request<T, E>(
+ params: Parameters<HttpClient["request"]>[0],
+ ): ReturnType<HttpClient["request"]> {
+ if (timer) {
+ clearTimeout(timer);
+ rejectTimer?.(new Error("debounced"));
+ }
+ return await new Promise<T>((resolve, reject) => {
+ rejectTimer = reject;
+ timer = setTimeout(async () => {
+ try {
+ const res = await client.request<T, E>(params);
+ resolve(res);
+ } catch (error) {
+ console.error(error);
+ showToast("API ERROR", new Date());
+ reject(error);
+ } finally {
+ timer = undefined;
+ rejectTimer = undefined;
+ }
+ }, ms);
+ });
+ };
+};
+
+httpClient.request = debounceRequests(httpClient);If the intent is to debounce only identical or mutating requests, I can provide a keyed debouncer (method+URL+[body]) to avoid dropping unrelated calls. Do you want that?
Fix Debouncer in api/service.ts: correct typing, preserve this, and handle async errors
The current implementation introduces three critical issues:
- Typing is invalid:
typeof httpClient.request<T, E>cannot accept generics. - Context is lost: calling the unbound
apiRequest(...)strips away theHttpClientinstance’sthis. - Async errors aren’t caught: wrapping a non-awaited call in
try/catchwon’t capture promise rejections.
Additionally, the global debounce drops every request in the window. If you only intend to debounce identical requests, consider a key-based approach (e.g., by URL+params).
Apply this diff to fix all three issues:
--- a/api/service.ts
+++ b/api/service.ts
@@ -35,7 +35,7 @@
-const debounceer = <T, E>(apiRequest: typeof httpClient.request<T, E>) => {
+const debounceRequests = (client: HttpClient, ms = 1000): HttpClient["request"] => {
let timer: ReturnType<typeof setTimeout> | undefined;
let rejectTimer: ((reason?: unknown) => void) | undefined;
- return (requestParams: Parameters<typeof httpClient.request<T, E>>[0]): ReturnType<typeof httpClient.request<T>> => {
+ return async function request<T, E>(
+ params: Parameters<HttpClient["request"]>[0]
+ ): ReturnType<HttpClient["request"]> {
if (timer) {
clearTimeout(timer);
rejectTimer?.(new Error("debounced"));
@@ -45,20 +45,32 @@
- const apiRes: Promise<T> = new Promise((resolve, reject) => {
- rejectTimer = reject;
- timer = Number(
- setTimeout(async () => {
- try {
- const res = apiRequest(requestParams);
- resolve(res);
- } catch (error) {
- console.error(error);
- showToast(`API ERROR`, new Date());
- }
- }, timeLimit),
- );
- });
- return apiRes;
+ return new Promise<T>((resolve, reject) => {
+ rejectTimer = reject;
+ timer = setTimeout(async () => {
+ try {
+ // Preserve context and catch async errors
+ const res = await client.request<T, E>(params);
+ resolve(res);
+ } catch (error) {
+ console.error(error);
+ showToast("API ERROR", new Date());
+ reject(error);
+ } finally {
+ timer = undefined;
+ rejectTimer = undefined;
+ }
+ }, ms);
+ });
};
};
-httpClient.request = debounceer(httpClient.request);
+// Override with the debounced version of request
+httpClient.request = debounceRequests(httpClient, 1000);Let me know if you’d like a keyed-debouncer (method+URL+body) instead of a global one.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In api/service.ts around lines 35 to 65, the debouncer loses the original this,
uses invalid generic typing for typeof httpClient.request, and won’t catch async
rejections; fix it by typing debounceer to accept and return the concrete
function signature (avoid using typeof httpClient.request with generics — use
the actual parameter/return types or a generic function type like <T>(fn: (this:
any, params: RequestParams) => Promise<T>) => (this: any, params: RequestParams)
=> Promise<T>), implement the returned function as a regular function(this: any,
requestParams) so you can call apiRequest.call(this, requestParams) to preserve
context, and await the apiRequest call inside the timer’s async handler within
try/catch (resolve(await apiRequest.call(this, requestParams)) / reject(error))
so promise rejections are caught; also make timer and reject handler optional
(number | undefined) and consider switching to per-request-key timers if you
only want to debounce identical requests.
| const handleMobileKeyboardResize = () => { | ||
| const bottomSheet = document.querySelector( | ||
| // "#fixer", | ||
| "#bottom-sheet", | ||
| ) as HTMLDivElement; | ||
| if (!bottomSheet) return; | ||
| const keyboardHeight = window.innerHeight - window.visualViewport!.height; | ||
|
|
||
| // 키보드가 올라온 경우 | ||
| if (keyboardHeight > 0) { | ||
| bottomSheet.style.transform = `translateY(-${keyboardHeight}px)`; | ||
| } | ||
| // 키보드가 내려간 경우 | ||
| else { | ||
| bottomSheet.style.transform = "translateY(0px)"; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid overriding Drawer.Content transform; move keyboard offset to the wrapper to prevent conflicts with Vaul animations
Overwriting style.transform on Drawer.Content will fight Vaul’s own transform-based animations, causing visual glitches and broken interactions (especially on Android). Apply the translateY on the fixed wrapper instead.
Apply this diff:
- const handleMobileKeyboardResize = () => {
- const bottomSheet = document.querySelector(
- // "#fixer",
- "#bottom-sheet",
- ) as HTMLDivElement;
+ const handleMobileKeyboardResize = () => {
+ const bottomSheetWrapper = document.querySelector(
+ "#bottom-sheet-fixer",
+ ) as HTMLDivElement;
- if (!bottomSheet) return;
+ if (!bottomSheetWrapper) return;
const keyboardHeight = window.innerHeight - window.visualViewport!.height;
// 키보드가 올라온 경우
if (keyboardHeight > 0) {
- bottomSheet.style.transform = `translateY(-${keyboardHeight}px)`;
+ bottomSheetWrapper.style.transform = `translateY(-${keyboardHeight}px)`;
}
// 키보드가 내려간 경우
else {
- bottomSheet.style.transform = "translateY(0px)";
+ bottomSheetWrapper.style.transform = "translateY(0px)";
}
};- <div
+ <div
+ id="bottom-sheet-fixer"
className={
`flex justify-center w-[100vw] fixed bottom-14 z-30` /* 바텀탭바 따라 높이 조절은 일단 나중에..*/
}
style={bottomSheetFixerStyle}
>
- <Drawer.Content id="bottom-sheet">
+ <Drawer.Content>Also applies to: 73-79
🤖 Prompt for AI Agents
In app/_components/BottomSheetRenderer.tsx around lines 30 to 45 (and similarly
lines 73 to 79), the code sets style.transform directly on the Drawer.Content
element (#bottom-sheet), which conflicts with Vaul's transform-based animations;
instead, move the keyboard translateY to the fixed wrapper element that contains
the Drawer.Content. Update the selector to target the wrapper (e.g., the fixed
container ID/class) and apply bottomSheetWrapper.style.transform =
`translateY(-${keyboardHeight}px)` when keyboardHeight > 0 and reset to
"translateY(0px)" when not, leaving Drawer.Content transform untouched so Vaul
animations remain unaffected.
| const GuestModeHandler = () => { | ||
| const { isGuest } = useAuthStore(); | ||
| return <>{isGuest && <MSWComponent />}</>; | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Stop MSW when leaving guest mode to avoid intercepting real requests.
As written, the worker starts when isGuest becomes true but never stops when it flips back to false. The worker will keep intercepting calls post-login. Add a small effect to stop MSW on exit from guest mode.
Apply this diff:
import useAuthStore from "@/stores/useAuthStore";
import { MSWComponent } from "@/components/_mocks/MSWComponent";
+import { useEffect } from "react";
const GuestModeHandler = () => {
const { isGuest } = useAuthStore();
+ useEffect(() => {
+ if (isGuest) return;
+ // best-effort stop; no-op if stop API is not exposed
+ const stop = async () => {
+ try {
+ const { stopMsw } = await import("@/mocks/index");
+ await stopMsw?.();
+ } catch {
+ // ignore when not available (e.g., test/server)
+ }
+ };
+ stop();
+ }, [isGuest]);
return <>{isGuest && <MSWComponent />}</>;
};If stopMsw isn’t available yet, I can help wire it up in mocks/index.ts (export the worker and a stopMsw() helper).
📝 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.
| const GuestModeHandler = () => { | |
| const { isGuest } = useAuthStore(); | |
| return <>{isGuest && <MSWComponent />}</>; | |
| }; | |
| import useAuthStore from "@/stores/useAuthStore"; | |
| import { MSWComponent } from "@/components/_mocks/MSWComponent"; | |
| import { useEffect } from "react"; | |
| const GuestModeHandler = () => { | |
| const { isGuest } = useAuthStore(); | |
| useEffect(() => { | |
| if (isGuest) return; | |
| // best‐effort stop; no‐op if stop API is not exposed | |
| const stop = async () => { | |
| try { | |
| const { stopMsw } = await import("@/mocks/index"); | |
| await stopMsw?.(); | |
| } catch { | |
| // ignore when not available (e.g., test/server) | |
| } | |
| }; | |
| stop(); | |
| }, [isGuest]); | |
| return <>{isGuest && <MSWComponent />}</>; | |
| }; | |
| export default GuestModeHandler; |
🤖 Prompt for AI Agents
In app/_components/GuestModeHandler.tsx around lines 5 to 8, the component
starts MSW when isGuest is true but never stops it when isGuest flips back to
false, causing the worker to continue intercepting requests after login; update
the component to import stopMsw (from mocks/index.ts), add a useEffect that
watches isGuest and calls stopMsw() when isGuest becomes false (and optionally
calls stopMsw on unmount), and guard the call so you only attempt to stop if the
worker was started or stopMsw is defined.
| const newResult = { | ||
| todoResultId: genId(), | ||
| todoId: String(todoId), | ||
| ...(body.request as any), | ||
| createdAt: new Date().toISOString(), | ||
| } as TodoResultRs; | ||
| await dbService.add("todoResults", newResult); | ||
| return HttpResponse.json({ | ||
| todoResultId: newResult.todoResultId, | ||
| } as TodoResultIdRs); | ||
| } catch (e) { | ||
| return HttpResponse.json( |
There was a problem hiding this comment.
Store todoResults with id and delete by that id
You’re storing todoResultId instead of id, but later deleting by id. This will make deletions fail. Normalize to id and return todoResultId from the response.
Apply this diff:
- const newResult = {
- todoResultId: genId(),
- todoId: String(todoId),
- ...(body.request as any),
- createdAt: new Date().toISOString(),
- } as TodoResultRs;
- await dbService.add("todoResults", newResult);
- return HttpResponse.json({
- todoResultId: newResult.todoResultId,
- } as TodoResultIdRs);
+ const newResult: DBTodoResult = {
+ id: genId(),
+ todoId: String(todoId),
+ ...(body.request as any),
+ createdAt: new Date().toISOString(),
+ };
+ await dbService.add("todoResults", newResult);
+ return HttpResponse.json({
+ todoResultId: newResult.id,
+ } as TodoResultIdRs);📝 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.
| const newResult = { | |
| todoResultId: genId(), | |
| todoId: String(todoId), | |
| ...(body.request as any), | |
| createdAt: new Date().toISOString(), | |
| } as TodoResultRs; | |
| await dbService.add("todoResults", newResult); | |
| return HttpResponse.json({ | |
| todoResultId: newResult.todoResultId, | |
| } as TodoResultIdRs); | |
| } catch (e) { | |
| return HttpResponse.json( | |
| const newResult: DBTodoResult = { | |
| id: genId(), | |
| todoId: String(todoId), | |
| ...(body.request as any), | |
| createdAt: new Date().toISOString(), | |
| }; | |
| await dbService.add("todoResults", newResult); | |
| return HttpResponse.json({ | |
| todoResultId: newResult.id, | |
| } as TodoResultIdRs); |
🤖 Prompt for AI Agents
In mocks/guestMode/handlers.ts around lines 430 to 441, the code saves the new
todo result with a property named todoResultId but later deletions expect an id
field; change the stored object to include id: newResultId (i.e., set id:
genId() or map todoResultId to id) so the DB record uses id, keep returning
todoResultId in the HttpResponse payload (generate/assign todoResultId, set id
equal to it in the saved object), and ensure any typing reflects TodoResultRs
has id while the response remains TodoResultIdRs with todoResultId.
| - [🚀 Moiomo 디자인 시스템 둘러보기](#-motimo-디자인-시스템-둘러보기) | ||
| - [🤝 기여하기](#-기여하기) |
There was a problem hiding this comment.
Typo in ToC: “Moiomo” → “Motimo”
Rename for consistency and to avoid broken ToC expectations.
-- [🚀 Moiomo 디자인 시스템 둘러보기](#-motimo-디자인-시스템-둘러보기)
+- [🚀 Motimo 디자인 시스템 둘러보기](#-motimo-디자인-시스템-둘러보기)📝 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.
| - [🚀 Moiomo 디자인 시스템 둘러보기](#-motimo-디자인-시스템-둘러보기) | |
| - [🤝 기여하기](#-기여하기) | |
| - [🚀 Motimo 디자인 시스템 둘러보기](#-motimo-디자인-시스템-둘러보기) | |
| - [🤝 기여하기](#-기여하기) |
🤖 Prompt for AI Agents
In README.md around lines 30 to 31, fix the ToC typo by renaming the link label
"Moiomo" to "Motimo" (i.e., change "[🚀 Moiomo 디자인 시스템 둘러보기]" to "[🚀 Motimo 디자인
시스템 둘러보기]") and ensure the anchor target (#-motimo-디자인-시스템-둘러보기) matches the
header ID if needed so the Table of Contents link works correctly.
| <a href="10th-motimo-storybook.vercel.app">10th-motimo-storybook.vercel.app</a> | ||
|
|
||
| <br/><br/> |
There was a problem hiding this comment.
External link missing protocol; make it clickable
Add https:// to the Storybook link so GitHub renders it correctly.
-<a href="10th-motimo-storybook.vercel.app">10th-motimo-storybook.vercel.app</a>
+<a href="https://10th-motimo-storybook.vercel.app">10th-motimo-storybook.vercel.app</a>📝 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.
| <img width="1894" height="945" alt="image" src="https://github.com/user-attachments/assets/0154c0fc-1dcf-4f58-b415-591a75fd751e" /> | |
| <a href="10th-motimo-storybook.vercel.app">10th-motimo-storybook.vercel.app</a> | |
| <img width="1894" height="945" alt="image" src="https://github.com/user-attachments/assets/0154c0fc-1dcf-4f58-b415-591a75fd751e" /> | |
| <a href="https://10th-motimo-storybook.vercel.app">10th-motimo-storybook.vercel.app</a> |
🤖 Prompt for AI Agents
In README.md around lines 186 to 188, the Storybook anchor uses a bare URL
without protocol so GitHub won't render it as a clickable link; update the
anchor href and visible link text to include the full protocol
(https://10th-motimo-storybook.vercel.app) so the link becomes clickable and
renders correctly on GitHub.
| - [Motion](https://github.com/motiondivision/motion) - 애니메이션에 사용된 라이브러리 | ||
| - [Contributors](https://github.com/prography/10th-Motimo-FE/contributors) - 모든 기여자분들께 감사드립니다 | ||
| - [Prography](https://prography.org/) - 활발한 개발 환경 지원 | ||
| - Motimo 팀원들과 Progarphy 10기 분들 | ||
|
|
||
| <br/><br/> |
There was a problem hiding this comment.
Spelling: “Progarphy” → “Prography”
Minor typo fix in acknowledgements.
-- Motimo 팀원들과 Progarphy 10기 분들
+- Motimo 팀원들과 Prography 10기 분들📝 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.
| - [Vaul](https://github.com/emilkowalski/vaul) - 바텀 시트에 사용된 라이브러리 | |
| - [Motion](https://github.com/motiondivision/motion) - 애니메이션에 사용된 라이브러리 | |
| - [Contributors](https://github.com/prography/10th-Motimo-FE/contributors) - 모든 기여자분들께 감사드립니다 | |
| - [Prography](https://prography.org/) - 활발한 개발 환경 지원 | |
| - Motimo 팀원들과 Progarphy 10기 분들 | |
| - [Vaul](https://github.com/emilkowalski/vaul) - 바텀 시트에 사용된 라이브러리 | |
| - [Motion](https://github.com/motiondivision/motion) - 애니메이션에 사용된 라이브러리 | |
| - [Contributors](https://github.com/prography/10th-Motimo-FE/contributors) - 모든 기여자분들께 감사드립니다 | |
| - [Prography](https://prography.org/) - 활발한 개발 환경 지원 | |
| - Motimo 팀원들과 Prography 10기 분들 |
🤖 Prompt for AI Agents
In README.md around lines 232 to 237, there's a typo "Progarphy" in the
acknowledgements; update the spelling to "Prography" so the link text and any
plain occurrences read correctly, and verify there are no other instances of the
misspelling elsewhere in the file.
| // Guest | ||
| setIsGuest: (newVal) => set({ isGuest: newVal }), | ||
| }), |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Enforce mutual exclusivity between guest and logged-in states in the setter.
To avoid inconsistent UI/logic, ensure setIsGuest(true) forces isLoggedIn to false.
Apply this diff:
- // Guest
- setIsGuest: (newVal) => set({ isGuest: newVal }),
+ // Guest
+ setIsGuest: (newVal) =>
+ set((state) => ({
+ isGuest: newVal,
+ // ensure mutual exclusivity
+ isLoggedIn: newVal ? false : state.isLoggedIn,
+ })),Optionally, also clear tokens when entering guest mode if your network layer relies on token presence to branch behavior. If desired, I can provide a minimal safe change for that as well.
[suggest_optional_refactor]
Run this to check call sites set the expected values across flows:
🏁 Script executed:
#!/bin/bash
# Inspect how guest mode and login are toggled throughout the app
rg -n -C3 -P --type tsx --type ts --type tsx '\bsetIsGuest\s*\('
rg -n -C3 -P --type tsx --type ts --type tsx '\blogin\s*\('Length of output: 184
Let’s broaden the search to all .ts/.tsx files without relying on ripgrep file-type definitions:
🏁 Script executed:
#!/bin/bash
# Inspect how guest mode and login are toggled throughout the app
rg -n -C3 -g '*.ts' -g '*.tsx' '\bsetIsGuest\s*\(' .
rg -n -C3 -g '*.ts' -g '*.tsx' '\blogin\s*\(' .Length of output: 2712
Enforce mutual exclusivity in setIsGuest
We’ve identified flows (e.g. the handleBrowse flow in
app/onboarding/_components/LoginScreen.tsx) that call login() then
setIsGuest(true), which currently leaves both isLoggedIn and isGuest
set to true. To avoid inconsistent state, update the setIsGuest setter so
that turning on guest mode automatically clears isLoggedIn.
Files to update:
- stores/useAuthStore.ts (setter for
setIsGuest)
Apply this change:
--- a/stores/useAuthStore.ts
@@ lines 111-113
- // Guest
- setIsGuest: (newVal) => set({ isGuest: newVal }),
+ // Guest
+ setIsGuest: (newVal) =>
+ set((state) => ({
+ isGuest: newVal,
+ // ensure mutual exclusivity
+ isLoggedIn: newVal ? false : state.isLoggedIn,
+ })),Optional cleanup (if your network layer uses tokens to branch behavior), clear tokens when entering guest mode:
- setIsGuest: (newVal) =>
- set((state) => ({
- isGuest: newVal,
- isLoggedIn: newVal ? false : state.isLoggedIn,
- })),
+ setIsGuest: (newVal) =>
+ set((state) => ({
+ isGuest: newVal,
+ isLoggedIn: newVal ? false : state.isLoggedIn,
+ authToken: newVal ? null : state.authToken,
+ })),📝 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.
| // Guest | |
| setIsGuest: (newVal) => set({ isGuest: newVal }), | |
| }), | |
| // Guest | |
| setIsGuest: (newVal) => | |
| set((state) => ({ | |
| isGuest: newVal, | |
| // ensure mutual exclusivity | |
| isLoggedIn: newVal ? false : state.isLoggedIn, | |
| })), | |
| }), |
🤖 Prompt for AI Agents
In stores/useAuthStore.ts around lines 111 to 113, the setIsGuest setter should
enforce mutual exclusivity by ensuring when isGuest is set to true it also
clears isLoggedIn (and optionally clears any auth tokens); update the setter so
that set({ isGuest: newVal, isLoggedIn: newVal ? false : get().isLoggedIn }) (or
equivalent) is used, and if your network layer relies on tokens also clear them
when turning on guest mode.
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (2)
app/adding-goal/page.tsx (2)
96-118: Add error handling for API callThe form submission doesn't handle potential API failures, which could leave users confused if the request fails.
Add try-catch error handling:
onSubmit={async () => { + try { const res = await goalApi.createGoal({ isPeriodByMonth: goalAddInfo.durationType === "month", title: goalAddInfo.goal, dueDate: goalAddInfo.durationType === "month" ? undefined : date2StringWithSpliter( goalAddInfo.durationValue as Date, "-", ), month: goalAddInfo.durationType === "month" ? (goalAddInfo.durationValue as number) : undefined, subGoals: goalAddInfo.subGoals.map((subGoalName) => ({ title: subGoalName, })), }); if (res) { router.back(); } + } catch (error) { + console.error("Failed to create goal:", error); + // Consider showing a user-friendly error message + } }}
96-118: Prevent default form submission behaviorThe form submission handler doesn't prevent the default browser behavior, which could cause page refresh.
- onSubmit={async () => { + onSubmit={async (e) => { + e.preventDefault(); const res = await goalApi.createGoal({
♻️ Duplicate comments (1)
app/adding-goal/page.tsx (1)
37-82: Fix missing dependencies in useEffect hookThe ESLint warning indicates missing dependencies in the useEffect. This can lead to stale closures and unexpected behavior.
Apply this diff to fix the dependency array and simplify the logic:
useEffect(() => { if (!openBottomSheet) { closeBottomSheet(); + return; } const bottomSheetInfo: Parameters<typeof openGoalDurationBottomSheet>[0] = { backdropProps: { className: "fixed inset-0 z-20 bg-neutral-700/50 ", onClick: () => { setOpenBottomSheet(false); }, }, ContentComponent: GoalDurationBottomSheet, hasBackdrop: true, bottomSheetFixerStyle: { bottom: 0, }, contentProps: { setopenBottomSheet: setOpenBottomSheet, onEdit: async ({ type, value }) => { setGoalAddInfo((prev) => ({ ...prev, durationType: type, durationValue: value, })); // 닫기 setOpenBottomSheet(false); }, }, }; - const isOpended = checkOpened(); - if (openBottomSheet && !isOpended) { + const isOpened = checkOpened(); + if (!isOpened) { openGoalDurationBottomSheet(bottomSheetInfo); - return; - } - - if (openBottomSheet) { + } else { updateGoalDurationBottomSheet(bottomSheetInfo); } return () => { - if (!openBottomSheet) { - closeBottomSheet(); - } + closeBottomSheet(); }; - }, [openBottomSheet]); + }, [ + openBottomSheet, + checkOpened, + closeBottomSheet, + openGoalDurationBottomSheet, + updateGoalDurationBottomSheet, + ]);
🧹 Nitpick comments (5)
components/details/ListCard/ListCard.stories.tsx (2)
38-40: Remove stale commented-out props to prevent driftThese callbacks were removed from the component API; keeping commented placeholders adds noise and risks confusion.
Apply this minimal cleanup:
- // applyOnGoalData: () => {}, - // onLeft: () => {}, - // onRight: () => {},
42-43: Drop commented subGoalInfo fields that no longer existStories should mirror the current public props; if you need to document prior fields, move that into Docs descriptions instead of commented code.
Apply this minimal cleanup:
- // idx: 0, - // totalSubGoalsLen: 4,app/adding-goal/page.tsx (3)
67-67: Fix typo in variable nameThe variable name has a typo that should be corrected for consistency.
- const isOpended = checkOpened(); + const isOpened = checkOpened();
180-194: Add unique keys for list itemsUsing array indices as keys can cause issues with React's reconciliation when the list changes. Consider using a more stable identifier.
Consider using a unique identifier for each sub-goal instead of the array index. You could generate IDs when adding new sub-goals:
- {goalAddInfo.subGoals.map((subGoal, idx) => ( + {goalAddInfo.subGoals.map((subGoal, idx) => ( <TextField - key={`${idx}`} + key={`subgoal-${idx}-${subGoal}`} isError={false}Or better yet, modify the data structure to include unique IDs for each sub-goal.
235-247: Remove commented-out codeThe commented-out JSX for the old bottom sheet implementation should be removed as it's no longer needed with the new hook-based approach.
- {/* <GoalDurationBottomSheet - openBottomSheet={openBottomSheet} - setopenBottomSheet={setOpenBottomSheet} - onEdit={async ({ type, value }) => { - setGoalAddInfo((prev) => ({ - ...prev, - durationType: type, - durationValue: value, - })); - // 닫기 - setOpenBottomSheet(false); - }} - /> */}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
app/adding-goal/page.tsx(4 hunks)components/details/BottomSheets/GoalDurationBottomSheet/GoalDurationBottomSheet.stories.tsx(2 hunks)components/details/ListCard/ListCard.stories.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- components/details/BottomSheets/GoalDurationBottomSheet/GoalDurationBottomSheet.stories.tsx
🧰 Additional context used
📓 Path-based instructions (4)
components/**/*.{js,jsx,ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/global-rule.mdc)
For every styling, use tailwind css.
Files:
components/details/ListCard/ListCard.stories.tsx
{components/**/*.{js,jsx,ts,tsx},**/*.css}
📄 CodeRabbit Inference Engine (.cursor/rules/global-rule.mdc)
For tokens, use tokens.css (mdc:app/tokens.css).
Files:
components/details/ListCard/ListCard.stories.tsx
components/**/*
📄 CodeRabbit Inference Engine (.cursor/rules/global-rule.mdc)
Component should be created in components/
Files:
components/details/ListCard/ListCard.stories.tsx
components/**/*.stories.{js,jsx,ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/global-rule.mdc)
components/**/*.stories.{js,jsx,ts,tsx}: If Component is created, add storybook files as well.
TheDefaultexample of storybook should be namedPrimary
Files:
components/details/ListCard/ListCard.stories.tsx
🪛 GitHub Actions: CI Pipeline
app/adding-goal/page.tsx
[warning] 82-82: React Hook useEffect has missing dependencies: 'checkOpened', 'closeBottomSheet', 'openGoalDurationBottomSheet', and 'updateGoalDurationBottomSheet'. Either include them or remove the dependency array. (react-hooks/exhaustive-deps)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: chromatic-test
DoneItemDetail에서 useGoalDetail훅을 사용하는데, 이는 스토리북에서 사용하려면 따로 render에서 불러와야 함. 그러나 컴포넌트 내부에서 사용되지 않아 주석처리함.
There was a problem hiding this comment.
Actionable comments posted: 3
🔭 Outside diff range comments (1)
app/details/[goal_id]/_components/DetailsBody/DetailBody.tsx (1)
45-47: Fix possible runtime crash:datacan be undefined when dereferencing
datamay be undefined on initial render, causingCannot read properties of undefined (reading 'subGoals')in the hook argument.Apply this diff:
- const { mutate } = useSubGoalTodosAllInfinite( - data.subGoals?.[targetSubGoalIdx]?.subGoalId ?? "", - ); + const { mutate } = useSubGoalTodosAllInfinite( + data?.subGoals?.[targetSubGoalIdx]?.subGoalId ?? "", + );
♻️ Duplicate comments (1)
app/details/[goal_id]/_components/DetailsBody/DetailBody.tsx (1)
248-262: Provide a stable fallback for React list keys (align with previous review)Keys can be undefined if
subGoalIdis missing, leading to unstable reconciliation. UsesubGoalIdfirst, and fall back to index.Apply this diff:
- {data.subGoals?.map((subGoalInfo) => ( + {data.subGoals?.map((subGoalInfo, idx) => ( <ListCard - key={subGoalInfo.subGoalId} + key={subGoalInfo?.subGoalId ?? `subgoal-${idx}`} initTodoInfoList={subGoalInfo?.initTodoItemsInfo} subGoalInfo={{ id: subGoalInfo?.subGoalId, // idx: targetSubGoalIdx, name: subGoalInfo?.subGoal, // totalSubGoalsLen: data.subGoals?.length ?? 0, isCompleted: subGoalInfo?.isCompleted, }} /> ))}
🧹 Nitpick comments (2)
app/details/[goal_id]/_components/DetailsBody/DetailBody.tsx (2)
187-193: Use the already-roundedfixedProgressconsistentlyYou compute
fixedProgressbut render rawgoalDetail?.progress, creating rounding inconsistencies between bar width and label when decimals exist.Apply this diff:
- style={{ width: `${goalDetail?.progress ?? 0}%` }} + style={{ width: `${fixedProgress}%` }} ... - {`${goalDetail?.progress ?? 0}%`} + {`${fixedProgress}%`}
345-373: Add error handling in submission helperAPI calls may throw/return errors. Wrap in try/catch and surface failure toasts or logs. This will also make
isFetchOkreflect actual success/failure rather than truthiness of the response.Example:
const handleTodoBottomSheetSubmit = async (newTodoInfo, afterSubmit) => { try { const isCreating = !newTodoInfo.id; const fetchRes = isCreating ? await subGoalApi.createTodo(newTodoInfo.subGoalId, { title: newTodoInfo.todo, date: newTodoInfo?.date ? date2StringWithSpliter(newTodoInfo?.date, "-") : undefined, }) : await todoApi.updateTodo(newTodoInfo.id!, { date: newTodoInfo.date ? date2StringWithSpliter(newTodoInfo.date, "-") : undefined, title: newTodoInfo.todo, }); const ok = !!fetchRes; if (ok) afterSubmit(); return ok; } catch (e) { // e.g., setToast("할 일 저장에 실패했어요. 잠시 후 다시 시도해주세요."); return false; } };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
app/details/[goal_id]/_components/DetailsBody/DetailBody.tsx(7 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: chromatic-test
| const leftDay = goalDetail?.dueDate?.dueDate | ||
| ? calcLeftDay(goalDetail?.dueDate.dueDate) | ||
| : NaN; | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Guard against NaN left-day and ensure D-Day formatting is correct when 0
- When
goalDetail?.dueDate?.dueDateis missing,leftDaybecomesNaN, renderingD+NaN. - For
leftDay === 0, you currently renderD-Day(which is fine if intended), but the logic is easier to read if we avoid numeric ops onnull/NaN.
Apply these diffs:
Set a null fallback instead of NaN:
- const leftDay = goalDetail?.dueDate?.dueDate
- ? calcLeftDay(goalDetail?.dueDate.dueDate)
- : NaN;
+ const leftDay = goalDetail?.dueDate?.dueDate
+ ? calcLeftDay(goalDetail?.dueDate.dueDate)
+ : null;Render label defensively:
- <p className="justify-center text-label-primary text-base font-bold font-['SUIT_Variable'] leading-tight">{`D${leftDay >= 0 ? "-" : "+"}${leftDay !== 0 ? Math.abs(leftDay) : "Day"}`}</p>
+ <p className="justify-center text-label-primary text-base font-bold font-['SUIT_Variable'] leading-tight">
+ {leftDay === null
+ ? "-"
+ : `D${leftDay === 0 ? "-Day" : `${leftDay > 0 ? "-" : "+"}${Math.abs(leftDay)}`}`}
+ </p>Also applies to: 178-194
🤖 Prompt for AI Agents
In app/details/[goal_id]/_components/DetailsBody/DetailBody.tsx around lines
69-72 (and similarly 178-194), replace the NaN fallback with a null fallback
when dueDate is missing (e.g., const leftDay = goalDetail?.dueDate?.dueDate ?
calcLeftDay(...) : null), and update rendering logic to treat null defensively:
if leftDay === null show a safe placeholder (e.g., '-' or empty), if leftDay ===
0 render "D-Day", otherwise render "D‑N" for future days and "D+N" for past days
using Math.abs for sign handling; apply the same null-aware checks in the other
block at lines 178-194.
| const afterSubmit = () => { | ||
| mutate(); | ||
| // 바텀시트 리셋 | ||
| setIsActive(false); | ||
| }; | ||
| const res = await handleTodoBottomSheetSubmit( | ||
| newTodoInfo, | ||
| afterSubmit, | ||
| ); |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Refetch likely targets the wrong sub-goal after submit
afterSubmit calls the mutate returned from a hook that is bound to data?.subGoals?.[targetSubGoalIdx]?.subGoalId (with targetSubGoalIdx fixed at 0). If the user creates/edits a todo for a different sub-goal, you’re invalidating/refetching the wrong list.
Consider invalidating/refetching by the sub-goal ID contained in newTodoInfo.subGoalId using your query client (React Query) or global mutate (SWR). For example with React Query:
// inside onSubmitTodo -> after successful submission
const queryClient = useQueryClient();
await queryClient.invalidateQueries({
queryKey: ['subGoalTodosAllInfinite', { subGoalId: newTodoInfo.subGoalId }],
});
setIsActive(false);If you prefer not to couple to the query key here, you can expose an invalidator function from the todos query module that takes subGoalId.
#!/bin/bash
echo "Locating DetailBody.tsx..."
file=$(fd DetailBody.tsx)
echo "File found at: $file"
echo -e "\n📄 Displaying lines 100–180 of DetailBody.tsx"
sed -n '100,180p' "$file"
echo -e "\n🔍 Searching for targetSubGoalIdx usage in DetailBody.tsx"
rg -n "targetSubGoalIdx" -C5 "$file"
echo -e "\n🔍 Searching for useSWR or useSWRInfinite across the repo"
rg -n --type ts,tsx "useSWR" -C3 .
rg -n --type ts,tsx "useSWRInfinite" -C3 .
echo -e "\n🔍 Searching for mutate( calls under app/details"
rg -n --type ts,tsx "mutate(" -C5 app/details
🏁 Script executed:
#!/bin/bash
echo "Locating DetailBody.tsx..."
file=$(fd DetailBody.tsx)
echo "Found file: $file"
echo -e "\nDisplaying lines 100–200 of DetailBody.tsx:"
sed -n '100,200p' "$file"
echo -e "\nSearching for mutate destructuring in DetailBody.tsx:"
rg -n "const\s+{[^}]*mutate" -C3 "$file"
echo -e "\nSearching for useSWR or useSWRInfinite in DetailBody.tsx:"
rg -n "useSWR" -C3 "$file"
rg -n "useSWRInfinite" -C3 "$file"
echo -e "\nSearching for targetSubGoalIdx definitions and usage:"
rg -n "targetSubGoalIdx" -C3 "$file"
echo -e "\nSearching for handleTodoBottomSheetSubmit import and call:"
rg -n "handleTodoBottomSheetSubmit" -C3 "$file"Length of output: 7144
Ensure Todos are Refetched for the Submitted Sub-goal
The current afterSubmit callback always calls the mutate function from
useSubGoalTodosAllInfinite bound to data.subGoals[targetSubGoalIdx] (which remains at index 0). If a todo is created or edited for any other sub-goal, that sub-goal’s list won’t be updated.
• File: app/details/[goal_id]/_components/DetailsBody/DetailBody.tsx
• Affected lines: the afterSubmit definition and its invocation of mutate() around lines 141–147
Replace:
const afterSubmit = () => {
mutate(); // always invalidates subGoals[0]
setIsActive(false);
};With (React Query example):
const queryClient = useQueryClient();
const afterSubmit = async (submittedInfo: TodoInfoForSubmission) => {
await queryClient.invalidateQueries({
queryKey: [
'subGoalTodosAllInfinite',
{ subGoalId: submittedInfo.subGoalId },
],
});
setIsActive(false);
};
// …
const res = await handleTodoBottomSheetSubmit(
newTodoInfo,
() => afterSubmit(newTodoInfo),
);If you’d rather not reference the raw query key here, expose a helper (e.g. invalidateSubGoalTodos(subGoalId)) from your todos-query module and call that instead.
🤖 Prompt for AI Agents
In app/details/[goal_id]/_components/DetailsBody/DetailBody.tsx around lines 139
to 147, the afterSubmit callback calls mutate() which always targets the bound
subGoals[0], so todos for other sub-goals aren’t refetched; replace this with
logic that invalidates/refetches only the specific sub-goal's todos (use
useQueryClient().invalidateQueries with the subGoalId-specific query key or call
an exported helper like invalidateSubGoalTodos(subGoalId)), change afterSubmit
to accept the submitted todo info (or subGoalId) and call the invalidate helper
with that id, and pass a wrapper to handleTodoBottomSheetSubmit so afterSubmit
is invoked with the newTodoInfo before closing the bottom sheet.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (7)
app/onboarding/page.tsx (3)
35-40: Prefer optional chaining over nested default destructuring to preserve types (and enable isValidating).Nested default destructuring (
data: { goals } = {}) erases the hook’s response type and can turngoalsintoany. It also makes it harder to include additional flags likeisValidatingfor better loading states.Apply this diff to keep type information intact (and expose isValidating for the spinner logic below):
- const { - data: { goals } = {}, - isLoading, - error, - mutate, - } = useGoals({ + const { + data, + isLoading, + isValidating, + error, + mutate, + } = useGoals({Then, right below the hook call, derive goals with proper optional chaining:
// Right below the hook call const goals = data?.goals;
63-63: Redirect now gated by isLoggedIn; verify guest-mode semantics.Given guest mode was added in this PR, confirm whether guests who’ve completed onboarding should also be redirected to “/”. If yes, include isGuest in the condition.
- if (hasCompletedOnboarding && isLoggedIn) { + if (hasCompletedOnboarding && (isLoggedIn || isGuest)) {And ensure isGuest is pulled from the store:
// Above const { setHasCompletedOnboarding, isLoggedIn, hasCompletedOnboarding, isGuest } = useAuthStore();
118-123: Avoid UI flicker on login-triggered refetch; tie the spinner to revalidation.With the current condition, during a post-login refetch the presence of a previous error can suppress the spinner momentarily. Consider also showing the spinner during revalidation.
If your hook exposes isValidating (SWR v2), fold it into the condition:
- if ( - hasHydrated && - isLoggedIn && - (isLoading || (!goals && !error) || (goals && goals.length > 0)) - ) { + if ( + hasHydrated && + isLoggedIn && + (isLoading || isValidating || (!goals && !error) || (goals && goals.length > 0)) + ) {Additionally, your login-retry effect (Lines 47-52) can loop if the error persists after login (each mutate produces a new error object). Guard it to run once per login transition:
// Above effects const retriedAfterLoginRef = useRef(false); // Replace the login-retry effect body: useEffect(() => { if (hasHydrated && isLoggedIn && error && !retriedAfterLoginRef.current) { retriedAfterLoginRef.current = true; mutate(); } if (!isLoggedIn) { // reset guard when user logs out retriedAfterLoginRef.current = false; } }, [hasHydrated, isLoggedIn, error, mutate]);components/mypage/DoneItemDetail.stories.tsx (4)
5-20: Solid Story meta; consider “satisfies Meta” for stronger typing and clarify goalDetail description
- Using the satisfies operator preserves arg inference and catches excess props without widening types.
- The description can reflect both todos/completedTodos used across stories.
Apply this diff:
-const meta: Meta<typeof DoneItemDetail> = { +const meta = { title: "mypage/DoneItemDetail", component: DoneItemDetail, parameters: { layout: "centered", }, tags: ["autodocs"], argTypes: { goalDetail: { - description: "Complete goal detail with sub-goals and completed todos", + description: "Complete goal detail with sub-goals and items (todos/completedTodos)", }, className: { control: "text", description: "Additional CSS classes", }, }, -}; +} satisfies Meta<typeof DoneItemDetail>;
26-47: Standardize date string format (ISO 8601) for consistencyPrimary uses dueDate as YYYY-MM-DD but todos use dots. Prefer ISO 8601 across stories to avoid confusion (and to be friendlier to parsers if reused).
Apply this diff:
- date: "2025.01.15", + date: "2025-01-15",
106-136: Standardize completedDate strings to ISO 8601Keeps story data consistent and predictable.
Apply this diff:
- completedDate: "2025.01.20", + completedDate: "2025-01-20", - completedDate: "2025.01.21", + completedDate: "2025-01-21", - completedDate: "2025.01.22", + completedDate: "2025-01-22",
106-136: Optional: add a variant showcasing className customizationA small story demonstrating custom className (e.g., padding or background) can help consumers understand styling hooks.
You can add:
export const WithCustomClassName: Story = { args: { className: "p-4 bg-Color-gray-50", goalDetail: { id: "2", title: "ClassName Showcase", subGoals: [{ id: "1", title: "예시", completedTodos: [] }], }, }, };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
app/onboarding/page.tsx(4 hunks)components/mypage/DoneItemDetail.stories.tsx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
components/**/*.{js,jsx,ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/global-rule.mdc)
For every styling, use tailwind css.
Files:
components/mypage/DoneItemDetail.stories.tsx
{components/**/*.{js,jsx,ts,tsx},**/*.css}
📄 CodeRabbit Inference Engine (.cursor/rules/global-rule.mdc)
For tokens, use tokens.css (mdc:app/tokens.css).
Files:
components/mypage/DoneItemDetail.stories.tsx
components/**/*
📄 CodeRabbit Inference Engine (.cursor/rules/global-rule.mdc)
Component should be created in components/
Files:
components/mypage/DoneItemDetail.stories.tsx
components/**/*.stories.{js,jsx,ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/global-rule.mdc)
components/**/*.stories.{js,jsx,ts,tsx}: If Component is created, add storybook files as well.
TheDefaultexample of storybook should be namedPrimary
Files:
components/mypage/DoneItemDetail.stories.tsx
🧠 Learnings (1)
📚 Learning: 2025-07-21T07:45:37.405Z
Learnt from: CR
PR: prography/10th-Motimo-FE#0
File: .cursor/rules/global-rule.mdc:0-0
Timestamp: 2025-07-21T07:45:37.405Z
Learning: Applies to components/**/*.stories.{js,jsx,ts,tsx} : The `Default` example of storybook should be named `Primary`
Applied to files:
components/mypage/DoneItemDetail.stories.tsx
🧬 Code Graph Analysis (2)
app/onboarding/page.tsx (1)
api/hooks.ts (1)
useGoals(174-174)
components/mypage/DoneItemDetail.stories.tsx (1)
components/mypage/DoneItemDetail.tsx (1)
DoneItemDetail(44-74)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: chromatic-test
🔇 Additional comments (2)
app/onboarding/page.tsx (1)
74-81: Deps array LGTM.Explicitly listing all used values keeps the effect predictable. No functional concerns here.
components/mypage/DoneItemDetail.stories.tsx (1)
26-47: Primary story name LGTM and aligns with our guidelineYou adhered to our learning that the default example should be named “Primary.” Nice.
| args: { | ||
| goalDetail: { | ||
| id: "1", | ||
| title: "독서 습관 만들기", | ||
| subGoals: [ | ||
| { | ||
| id: "1", | ||
| title: "매일 30분 책 읽기", | ||
| completedTodos: [ | ||
| { | ||
| id: "1", | ||
| title: "소설 '데미안' 읽기", | ||
| completedDate: "2025.01.15", | ||
| attachment: { | ||
| type: "image", | ||
| url: "https://picsum.photos/200", | ||
| }, | ||
| }, | ||
| { | ||
| id: "2", | ||
| title: "독서 노트 작성하기", | ||
| completedDate: "2025.01.16", | ||
| attachment: { | ||
| type: "file", | ||
| url: "/reading-note.pdf", | ||
| name: "독서노트_데미안.pdf", | ||
| }, | ||
| }, | ||
| { | ||
| id: "3", | ||
| title: "독서 후기 SNS 게시", | ||
| completedDate: "2025.01.17", | ||
| }, | ||
| ], | ||
| }, | ||
| { | ||
| id: "2", | ||
| title: "독서 리스트 작성", | ||
| completedTodos: [ | ||
| { | ||
| id: "4", | ||
| title: "올해 읽을 책 10권 선정", | ||
| completedDate: "2024.12.31", | ||
| attachment: { | ||
| type: "file", | ||
| url: "/book-list.pdf", | ||
| name: "2025년_독서목록.pdf", | ||
| }, | ||
| }, | ||
| ], | ||
| }, | ||
| ], | ||
| }, | ||
| }, | ||
| }; |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Make story data deterministic and verify local asset paths
- Consider using a deterministic placeholder image to avoid flaky network calls in Storybook (e.g., seed-based picsum).
- Local PDF paths may 404 in Storybook unless served from public/static. Verify their presence or switch to a known-accessible URL.
- Also, standardize completedDate to ISO 8601.
[suggest_nitpick]
Apply this diff:
- completedDate: "2025.01.15",
+ completedDate: "2025-01-15",
- attachment: {
- type: "image",
- url: "https://picsum.photos/200",
- },
+ attachment: {
+ type: "image",
+ url: "https://picsum.photos/seed/done-item/200",
+ },- completedDate: "2025.01.16",
+ completedDate: "2025-01-16",- completedDate: "2024.12.31",
+ completedDate: "2024-12-31",Run this script to confirm the static files actually exist (adjust if they live under a different path like public/ or static/):
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Searching for referenced PDFs..."
fd -t f -i 'reading-note.pdf' -uu || true
fd -t f -i 'book-list.pdf' -uu || true
echo "If no results above, consider placing PDFs under public/ and referencing them as /reading-note.pdf or using a remote URL."Length of output: 412
Ensure deterministic assets and accessible PDFs in Storybook
- The placeholder image URL should be seed-based to avoid flaky network calls.
- Standardize all
completedDatevalues to ISO 8601 (YYYY-MM-DD). - The script confirmed that
reading-note.pdfandbook-list.pdfare not in the repo. Place them underpublic/(orstatic/) so Storybook can serve them at/reading-note.pdfand/book-list.pdf, or switch to a publicly hosted URL.
Apply these changes:
- completedDate: "2025.01.15",
+ completedDate: "2025-01-15",
- attachment: {
- type: "image",
- url: "https://picsum.photos/200",
- },
+ attachment: {
+ type: "image",
+ url: "https://picsum.photos/seed/done-item/200",
+ },- completedDate: "2025.01.16",
+ completedDate: "2025-01-16",- completedDate: "2025.01.17",
+ completedDate: "2025-01-17",- completedDate: "2024.12.31",
+ completedDate: "2024-12-31",📝 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.
| args: { | |
| goalDetail: { | |
| id: "1", | |
| title: "독서 습관 만들기", | |
| subGoals: [ | |
| { | |
| id: "1", | |
| title: "매일 30분 책 읽기", | |
| completedTodos: [ | |
| { | |
| id: "1", | |
| title: "소설 '데미안' 읽기", | |
| completedDate: "2025.01.15", | |
| attachment: { | |
| type: "image", | |
| url: "https://picsum.photos/200", | |
| }, | |
| }, | |
| { | |
| id: "2", | |
| title: "독서 노트 작성하기", | |
| completedDate: "2025.01.16", | |
| attachment: { | |
| type: "file", | |
| url: "/reading-note.pdf", | |
| name: "독서노트_데미안.pdf", | |
| }, | |
| }, | |
| { | |
| id: "3", | |
| title: "독서 후기 SNS 게시", | |
| completedDate: "2025.01.17", | |
| }, | |
| ], | |
| }, | |
| { | |
| id: "2", | |
| title: "독서 리스트 작성", | |
| completedTodos: [ | |
| { | |
| id: "4", | |
| title: "올해 읽을 책 10권 선정", | |
| completedDate: "2024.12.31", | |
| attachment: { | |
| type: "file", | |
| url: "/book-list.pdf", | |
| name: "2025년_독서목록.pdf", | |
| }, | |
| }, | |
| ], | |
| }, | |
| ], | |
| }, | |
| }, | |
| }; | |
| args: { | |
| goalDetail: { | |
| id: "1", | |
| title: "독서 습관 만들기", | |
| subGoals: [ | |
| { | |
| id: "1", | |
| title: "매일 30분 책 읽기", | |
| completedTodos: [ | |
| { | |
| id: "1", | |
| title: "소설 '데미안' 읽기", | |
| completedDate: "2025-01-15", | |
| attachment: { | |
| type: "image", | |
| url: "https://picsum.photos/seed/done-item/200", | |
| }, | |
| }, | |
| { | |
| id: "2", | |
| title: "독서 노트 작성하기", | |
| completedDate: "2025-01-16", | |
| attachment: { | |
| type: "file", | |
| url: "/reading-note.pdf", | |
| name: "독서노트_데미안.pdf", | |
| }, | |
| }, | |
| { | |
| id: "3", | |
| title: "독서 후기 SNS 게시", | |
| completedDate: "2025-01-17", | |
| }, | |
| ], | |
| }, | |
| { | |
| id: "2", | |
| title: "독서 리스트 작성", | |
| completedTodos: [ | |
| { | |
| id: "4", | |
| title: "올해 읽을 책 10권 선정", | |
| completedDate: "2024-12-31", | |
| attachment: { | |
| type: "file", | |
| url: "/book-list.pdf", | |
| name: "2025년_독서목록.pdf", | |
| }, | |
| }, | |
| ], | |
| }, | |
| ], | |
| }, | |
| }, | |
| }; |
🤖 Prompt for AI Agents
In components/mypage/DoneItemDetail.stories.tsx around lines 50 to 104, make
assets and dates deterministic and accessible: replace the placeholder image URL
with a seeded one (e.g. use a picsum seed URL like
https://picsum.photos/seed/reading/200), convert all completedDate values to ISO
8601 format (YYYY-MM-DD), and ensure the two PDF attachments are actually served
by Storybook by moving reading-note.pdf and book-list.pdf into the repo’s
public/ (or static/) directory so they are reachable at /reading-note.pdf and
/book-list.pdf (or update the attachment URLs to point to stable public URLs).
devgony
left a comment
devgony
left a comment
There was a problem hiding this comment.
뭔가 의도치 않게 todo input 이 남아있게 되는거같아요
저도 왜 발생하는지 더 찾아볼게요
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (5)
components/details/Modals/ModalAddingTodo/ModalAddingTodo.tsx (1)
78-90: Make form submission async, trim input, awaitonAddTodo, and reset statePrevents duplicate submits and ignores whitespace-only entries. Mirrors prior feedback.
- onSubmit={(e) => { - // 새로고침 방지 - e.preventDefault(); - if (!todo) { - setError(true); - return; - } - setError(false); - onAddTodo(todo, date); - }} + onSubmit={async (e) => { + e.preventDefault(); + const value = todo.trim(); + if (!value) { + setError(true); + return; + } + setError(false); + try { + await onAddTodo(value, date); + setTodo(""); + } catch (err) { + // TODO: surface error to user (toast or TextField isError) + console.error(err); + } + }}app/details/[goal_id]/_components/DetailsBody/DetailBody.tsx (4)
79-82: Guard against NaN D‑Day when due date is missingUse a null fallback and handle it in rendering to avoid “D+NaN”. This mirrors prior feedback.
- const leftDay = goalDetail?.dueDate?.dueDate - ? calcLeftDay(goalDetail?.dueDate.dueDate) - : NaN; + const leftDay = + goalDetail?.dueDate?.dueDate !== undefined + ? calcLeftDay(goalDetail.dueDate.dueDate) + : null;
204-209: Render D‑Day safely whenleftDayis nullAvoid rendering D+NaN by handling the null case explicitly.
- <p className="justify-center text-label-primary text-base font-bold font-['SUIT_Variable'] leading-tight">{`D${leftDay >= 0 ? "-" : "+"}${leftDay !== 0 ? Math.abs(leftDay) : "Day"}`}</p> + <p className="justify-center text-label-primary text-base font-bold font-['SUIT_Variable'] leading-tight"> + {leftDay === null + ? "-" + : `D${leftDay === 0 ? "-Day" : `${leftDay > 0 ? "-" : "+"}${Math.abs(leftDay)}`}`} + </p>
134-193: Bottom sheet effect: remove unconditional cleanup close; include stable depsThe cleanup currently closes the sheet on any dependency change, causing flicker. This was previously flagged; apply the same fix here.
useEffect(() => { const isRendered = checkRendered(); if (!shouldBottomSheetOpened && isRendered) { closeBottomSheet(); return; } const bottomSheetInfo: Parameters<typeof openBottomSheet>[0] = { backdropProps: { onClick: () => { // 내용물을 초기화 해야 함. -> key값 바꿔도 애니메이션이나 바텀시트 높이 정상적일까? setIsActive(false); updateBottomSheet((prev) => { return { ...prev, hasBackdrop: false }; }); }, className: "fixed inset-0 bg-black/20 z-20", }, ContentComponent: TodoBottomSheet, contentProps: { isActivated: isActive, initTodoInfo: initContent, setIsActivated: setIsActive, subGoals: data.subGoals?.map((subGoalInfo) => ({ id: subGoalInfo.subGoalId ?? "", title: subGoalInfo.subGoal ?? "", })) ?? [], onSubmitTodo: async (newTodoInfo) => { const afterSubmit = () => { mutate(); // 바텀시트 리셋 setIsActive(false); }; const res = await handleTodoBottomSheetSubmit( newTodoInfo, afterSubmit, ); return res; }, }, hasBackdrop: isActive, bottomSheetFixerStyle: { bottom: "0px" }, }; if (shouldBottomSheetOpened && !isRendered) { openBottomSheet(bottomSheetInfo); return; } updateBottomSheet(bottomSheetInfo); - return () => { - closeBottomSheet(); - }; - }, [isActive, initContent, data.subGoals?.length, shouldBottomSheetOpened]); + }, [ + isActive, + initContent, + data.subGoals?.length, + shouldBottomSheetOpened, + checkRendered, + openBottomSheet, + updateBottomSheet, + closeBottomSheet, + ]);If you want to close on unmount only, add a separate
useEffect(() => closeBottomSheet, []).
165-173: Refetch the correct sub-goal after submit
mutate()here targets the list bound todata.subGoals[targetSubGoalIdx](idx defaults to 0). Submissions for other sub-goals won’t refresh their lists.Option 1 (React Query): invalidate by subGoalId.
// Pseudocode: inject queryClient and invalidate by key containing subGoalId const res = await handleTodoBottomSheetSubmit(newTodoInfo, async () => { await queryClient.invalidateQueries({ queryKey: ["subGoalTodosAllInfinite", { subGoalId: newTodoInfo.subGoalId }], }); setIsActive(false); });Option 2 (SWR): expose an invalidator in your queries module or use a scoped mutate keyed by subGoalId.
// e.g., invalidateSubGoalTodos(newTodoInfo.subGoalId);
🧹 Nitpick comments (11)
components/mypage/MyPage.tsx (2)
60-66: Logout flow: remove stale setIsGuest(false) comment and ensure guest state is handled in storeThe comment suggests toggling guest to false on logout, which is likely the opposite of what you'd want. Either rely on logout() to set guest mode appropriately or explicitly set guest=true if needed. Remove the misleading comment to avoid confusion.
Apply this small cleanup:
- // setIsGuest(false);And verify that logout() transitions the store to guest mode (or a well-defined logged-out state) so the MyPage gating works as expected.
89-117: Remove legacy comment and gate profile fetching in guest mode
Delete the stale conditional comment at
components/mypage/MyPage.tsx:89:- {/* {isLoggedIn ? ( */}Avoid calling
useMyProfile()whenisGuestis true to prevent unnecessary API requests. You can choose one of the following approaches:
Extend the hook to accept an
enabledflag
Inapi/hooks.ts, updateuseMyProfileto:export const useMyProfile = ( config?: SWRConfiguration, enabled = true, ) => useApiQuery( "사용자Api", "getMyProfile", enabled ? [] : null, // pass `null` to skip fetch undefined, config, );Then in your component:
const { data: user, isLoading, error } = useMyProfile(undefined, !isGuest);Conditionally invoke the hook in-line
const guestProfile = { data: undefined, isLoading: false, error: undefined }; const { data: user, isLoading, error } = !isGuest ? useMyProfile() : guestProfile;components/shared/Modal/ModalCompletingGoal/ModalCompletingGoal.tsx (1)
36-41: Await completion to prevent double submit/race on “목표 달성!”If a user taps quickly, multiple requests can fire. Await the handler to serialize submissions.
Apply:
- onClick={() => { - onCompleteGoal(); - }} + onClick={async () => { + await onCompleteGoal(); + }}Optionally introduce a local loading flag to disable the button while pending if
Modal.Buttonsupportsdisabledorloading.components/main/GoalData/GoalData.tsx (2)
33-33: Clamp progress bar width to [0, 100]%Prevent visual overflow for out-of-bound values.
- style={{ width: `${progress.toFixed(2)}%` }} + style={{ width: `${Math.min(100, Math.max(0, progress))}%` }}
37-37: Conditional decimal display is fine; consider using the clamped valueSmall consistency nit: display the same clamped value you render in the bar (optionally with two decimals for 0<progress<100).
Example:
- {`${progress > 0 && progress < 100 ? progress.toFixed(2) : progress}%`} + {(() => { + const p = Math.min(100, Math.max(0, progress)); + return `${p > 0 && p < 100 ? p.toFixed(2) : p}%`; + })()}components/details/Modals/ModalAddingTodo/ModalAddingTodo.tsx (2)
72-75: Fix function name typo: handleDateCLick → handleDateClickMinor readability/consistency improvement.
- const handleDateCLick = () => { + const handleDateClick = () => { if (!ref.current) return; ref.current.showPicker(); };- onClick={handleDateCLick} + onClick={handleDateClick}Also applies to: 103-108
46-55: Optional: disable “추가하기” while pendingIf
Modal.Buttonsupportsdisabledorloading, wire a local pending flag in ModalBody via React Context/prop lift to block duplicate clicks.components/details/ListCard/ListCard.tsx (3)
101-151: Avoid bottom-sheet flicker: remove cleanup close and include stable depsThe effect’s cleanup closes on every dependency change, which can cause close/reopen jank. Also include the hook functions in the deps to align with React rules (assuming they’re stable, this is safe).
- useEffect(() => { + useEffect(() => { const isRendered = checkRendered(); if (!openBottomSheet && isRendered) { closeBottomSheet(); return; } const bottomSheetInfo: Parameters<typeof openTodoRootBottomSheet>[0] = { bottomSheetFixerStyle: { bottom: "0px" }, backdropProps: { onClick: () => { // 내용물을 초기화 해야 함. -> key값 바꿔도 애니메이션이나 바텀시트 높이 정상적일까? closeBottomSheet(); }, className: "fixed inset-0 bg-black/10 z-20", }, hasBackdrop: false, ContentComponent: TodoResultBottomSheet, contentProps: { onSubmit: async (todoResult) => { if (todoIdForResult && todoResult.emotion !== null) { const res = await postTodoResult( todoIdForResult, todoResult.emotion as TodoResultRqEmotionEnum, todoResult.memo, todoResult.file || undefined, ); if (!res) return; setOpenBottomSheet(false); mutate(); } }, openBottomSheet: openBottomSheet, setOpenBottomSheet: setOpenBottomSheet, }, }; const isOpened = checkOpened(); if (openBottomSheet && !isOpened) { openTodoRootBottomSheet(bottomSheetInfo); return; } if (isRendered) updateTodoResultBottomSheet(bottomSheetInfo); - - return () => { - if (isOpened && !openBottomSheet) { - closeBottomSheet(); - } - }; - }, [openBottomSheet, todoIdForResult]); + }, [ + openBottomSheet, + todoIdForResult, + checkRendered, + checkOpened, + openTodoRootBottomSheet, + updateTodoResultBottomSheet, + closeBottomSheet, + ]);
296-301: Await theonTodoCheckpromise to serialize togglesPrevents overlapping toggles when tapped rapidly.
onChecked={async () => { - onTodoCheck && onTodoCheck(todoInfo.id); + if (onTodoCheck) { + await onTodoCheck(todoInfo.id); + } }}
248-269: Add error handling to Todo creation flowSurface failures and avoid silent no-ops; optionally guard against rapid double tap.
onClick={() => { openModal( <ModalAddingTodo onAddTodo={async (newTodo, date) => { if (!subGoalInfo.id) return; - const res = await subGoalApi.createTodo(subGoalInfo.id, { - title: newTodo, - date: date - ? date2StringWithSpliter(date, "-") - : undefined, - }); - - if (res) { - closeModal(); - mutate(); - } + try { + const res = await subGoalApi.createTodo(subGoalInfo.id, { + title: newTodo, + date: date ? date2StringWithSpliter(date, "-") : undefined, + }); + if (res) { + closeModal(); + mutate(); + } else { + // TODO: toast error + console.error("Failed to create todo"); + } + } catch (e) { + // TODO: toast error + console.error(e); + } }} onClose={closeModal} />, ); }}app/details/[goal_id]/_components/DetailsBody/DetailBody.tsx (1)
214-219: Clamp and format progress values for consistencyPrevent overflow and match main/GoalData formatting.
- style={{ width: `${goalDetail?.progress ?? 0}%` }} + style={{ + width: `${Math.min(100, Math.max(0, goalDetail?.progress ?? 0))}%`, + }}- {`${goalDetail?.progress ?? 0}%`} + {(() => { + const p = Math.min(100, Math.max(0, goalDetail?.progress ?? 0)); + return `${p > 0 && p < 100 ? p.toFixed(2) : p}%`; + })()}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (7)
app/details/[goal_id]/_components/DetailsBody/DetailBody.tsx(8 hunks)components/details/ListCard/ListCard.tsx(7 hunks)components/details/Modals/ModalAddingTodo/ModalAddingTodo.tsx(1 hunks)components/main/GoalData/GoalData.tsx(2 hunks)components/main/GoalDataContainer/GoalDataContainer.tsx(1 hunks)components/mypage/MyPage.tsx(3 hunks)components/shared/Modal/ModalCompletingGoal/ModalCompletingGoal.tsx(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- components/main/GoalDataContainer/GoalDataContainer.tsx
🧰 Additional context used
📓 Path-based instructions (3)
components/**/*.{js,jsx,ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/global-rule.mdc)
For every styling, use tailwind css.
Files:
components/shared/Modal/ModalCompletingGoal/ModalCompletingGoal.tsxcomponents/main/GoalData/GoalData.tsxcomponents/mypage/MyPage.tsxcomponents/details/Modals/ModalAddingTodo/ModalAddingTodo.tsxcomponents/details/ListCard/ListCard.tsx
{components/**/*.{js,jsx,ts,tsx},**/*.css}
📄 CodeRabbit Inference Engine (.cursor/rules/global-rule.mdc)
For tokens, use tokens.css (mdc:app/tokens.css).
Files:
components/shared/Modal/ModalCompletingGoal/ModalCompletingGoal.tsxcomponents/main/GoalData/GoalData.tsxcomponents/mypage/MyPage.tsxcomponents/details/Modals/ModalAddingTodo/ModalAddingTodo.tsxcomponents/details/ListCard/ListCard.tsx
components/**/*
📄 CodeRabbit Inference Engine (.cursor/rules/global-rule.mdc)
Component should be created in components/
Files:
components/shared/Modal/ModalCompletingGoal/ModalCompletingGoal.tsxcomponents/main/GoalData/GoalData.tsxcomponents/mypage/MyPage.tsxcomponents/details/Modals/ModalAddingTodo/ModalAddingTodo.tsxcomponents/details/ListCard/ListCard.tsx
🧬 Code Graph Analysis (3)
app/details/[goal_id]/_components/DetailsBody/DetailBody.tsx (4)
utils/calcLeftDay.ts (1)
calcLeftDay(1-12)api/service.ts (2)
subGoalApi(90-90)todoApi(88-88)components/shared/BottomSheets/TodoBottomSheet/TodoBottomSheet.tsx (2)
TodoBottomSheetProps(255-255)TodoInfoForSubmission(255-255)utils/date2String.ts (1)
date2StringWithSpliter(7-7)
components/details/Modals/ModalAddingTodo/ModalAddingTodo.tsx (1)
utils/date2String.ts (1)
date2StringWithSpliter(7-7)
components/details/ListCard/ListCard.tsx (4)
types/todoList.ts (1)
TodoItemsInfo(7-7)lib/fetching/postTodoResult.ts (1)
postTodoResult(26-26)api/service.ts (1)
subGoalApi(90-90)utils/date2String.ts (1)
date2StringWithSpliter(7-7)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: chromatic-test
🔇 Additional comments (2)
components/shared/Modal/ModalCompletingGoal/ModalCompletingGoal.tsx (1)
59-65: Copy updates LGTMThe revised heading/subtitle read well and match the intent.
components/main/GoalData/GoalData.tsx (1)
16-16: D‑Day formatting LGTMThe sign-aware D-Day string avoids D--N and D-0 edge cases.
| import TodoBottomSheet, { | ||
| TodoBottomSheetProps, | ||
| } from "@/components/shared/BottomSheets/TodoBottomSheet/TodoBottomSheet"; | ||
| import useActiveTodoBottomSheet from "@/stores/useActiveTodoBottomSheet"; | ||
| import { date2StringWithSpliter } from "@/utils/date2String"; | ||
| import useToast from "@/hooks/useToast"; | ||
| import useBottomSheet from "@/hooks/useBottomSheet"; | ||
| import { TodoInfoForSubmission } from "@/components/shared/BottomSheets/TodoBottomSheet/TodoBottomSheet"; | ||
|
|
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Confirm date2StringWithSpliter accepts Date|string
In this file you pass newTodoInfo.date which may be Date | string by type. Ensure the helper handles both.
🏁 Script executed:
#!/bin/bash
# Verify the function signature and usage across the repo
rg -nC2 'export\s+const\s+date2StringWithSpliter' utils/date2String.ts
rg -nC2 'date2StringWithSpliter\(' --type=ts,tsxLength of output: 154
🏁 Script executed:
#!/bin/bash
# Locate the file(s) defining or referencing date2StringWithSpliter in TS/TSX
rg -n 'date2StringWithSpliter' -g '*.ts' -g '*.tsx' -C2
# Show the helper’s export/definition with context
rg -n 'export\s\+const\s\+date2StringWithSpliter' -g '*.ts' -g '*.tsx' -C3
# Show all call sites to verify argument types
rg -n 'date2StringWithSpliter\(' -g '*.ts' -g '*.tsx' -C2Length of output: 18313
Update date2StringWithSpliter to accept string inputs
The helper in utils/date2String.ts is currently typed as
const date2StringWithSpliter = (dateValue: Date, spliter: string) => { … }but in DetailBody.tsx (and other components) you pass newTodoInfo.date which is typed Date | string. This mismatch will throw at runtime if a string is passed.
Please address by either:
- Expanding the helper signature to
(dateValue: Date | string, spliter: string)and normalizing withnew Date(dateValue)when it’s a string. - Or ensuring all callers convert string dates to
Datebefore invoking the helper.
Key locations:
- utils/date2String.ts:1 – function signature
- app/details/[goal_id]/_components/DetailsBody/DetailBody.tsx:352 –
date2StringWithSpliter(newTodoInfo?.date, "-")
🤖 Prompt for AI Agents
In app/details/[goal_id]/_components/DetailsBody/DetailBody.tsx around line 352
and utils/date2String.ts at line 1, the helper date2StringWithSpliter is
currently typed to accept Date but callers (e.g. newTodoInfo.date) can be Date |
string; update the helper signature to (dateValue: Date | string, spliter:
string) and inside normalize the input by creating a Date from string inputs
(e.g. const d = typeof dateValue === "string" ? new Date(dateValue) :
dateValue), then use d for formatting; also update exported types/annotations so
callers no longer need to pre-convert, and add a small guard for invalid dates
(NaN) to return an empty string or a sensible default.
| const subGoalCompletingFeftches = data.subGoals?.map( | ||
| (subGoalInfo) => { | ||
| return subGoalApi.subGoalCompleteToggle( | ||
| subGoalInfo?.subGoalId ?? "", | ||
| ); | ||
| }, | ||
| ); | ||
|
|
||
| const subGoalCompletion = await Promise.all( | ||
| subGoalCompletingFeftches ?? [], | ||
| ).then( | ||
| (resList) => resList.filter((res) => res).length === resList.length, | ||
| ); | ||
| if (!subGoalCompletion) { | ||
| setToast("세부 목표 달성에 실패했습니다!"); | ||
| return; | ||
| } |
There was a problem hiding this comment.
Don’t toggle already-completed sub-goals; only complete the incomplete ones
Calling a “toggle” API for every sub-goal will flip already-completed items back to incomplete. Filter to incomplete sub-goals only.
- const subGoalCompletingFeftches = data.subGoals?.map(
- (subGoalInfo) => {
- return subGoalApi.subGoalCompleteToggle(
- subGoalInfo?.subGoalId ?? "",
- );
- },
- );
+ const subGoalCompletingFeftches = data.subGoals
+ ?.filter((sg) => !sg.isCompleted)
+ .map((sg) =>
+ subGoalApi.subGoalCompleteToggle(sg?.subGoalId ?? ""),
+ );📝 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.
| const subGoalCompletingFeftches = data.subGoals?.map( | |
| (subGoalInfo) => { | |
| return subGoalApi.subGoalCompleteToggle( | |
| subGoalInfo?.subGoalId ?? "", | |
| ); | |
| }, | |
| ); | |
| const subGoalCompletion = await Promise.all( | |
| subGoalCompletingFeftches ?? [], | |
| ).then( | |
| (resList) => resList.filter((res) => res).length === resList.length, | |
| ); | |
| if (!subGoalCompletion) { | |
| setToast("세부 목표 달성에 실패했습니다!"); | |
| return; | |
| } | |
| const subGoalCompletingFeftches = data.subGoals | |
| ?.filter((sg) => !sg.isCompleted) | |
| .map((sg) => | |
| subGoalApi.subGoalCompleteToggle(sg?.subGoalId ?? ""), | |
| ); | |
| const subGoalCompletion = await Promise.all( | |
| subGoalCompletingFeftches ?? [], | |
| ).then( | |
| (resList) => resList.filter((res) => res).length === resList.length, | |
| ); | |
| if (!subGoalCompletion) { | |
| setToast("세부 목표 달성에 실패했습니다!"); | |
| return; | |
| } |
🤖 Prompt for AI Agents
In app/details/[goal_id]/_components/DetailsBody/DetailBody.tsx around lines 89
to 105, the code currently calls a toggle API for every sub-goal which will flip
already-completed items back to incomplete; change it to first filter subGoals
to only those that are not completed (e.g., subGoalInfo.completed === false or
equivalent flag), then map that filtered list to the sub-goal completion API
(use the explicit complete endpoint if available rather than a toggle), await
Promise.all on that filtered list (handle the case when there are no incomplete
sub-goals by skipping the request and treating as success), and determine
overall success by checking each response; if any request fails show the
existing toast and return.
| export function MyPage({ className = "" }: MyPageProps) { | ||
| const router = useSafeRouter(); | ||
| const { isLoggedIn, login, logout } = useAuthStore(); | ||
| const { isLoggedIn, login, logout, isGuest } = useAuthStore(); |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Unify auth gating: use a single source of truth (isGuest) across UI and actions
You gate rendering with !isGuest but compute navigationItems from isLoggedIn. This can drift if the two flags ever get out of sync. Prefer consistently deriving UI/logic from isGuest (or from a single derived flag like isAuthed = !isGuest) for clarity and future-proofing.
[suggest_optional_refactor]
To verify store invariants and ensure logout/login keep these flags in sync, run:
If isGuest is the canonical flag, update navigationItems to use !isGuest instead of isLoggedIn for consistency.
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Locate useAuthStore implementation(s)"
fd -t f -i 'useAuthStore.*' | tee /tmp/auth_store_files
echo
echo "Inspect isGuest/isLoggedIn and login/logout behaviors"
while read -r f; do
echo ">>> $f"
rg -n -C3 -P '\bisGuest\b|\bisLoggedIn\b|logout\s*\(|login\s*\(' "$f" || true
done </tmp/auth_store_files
echo
echo "Find other components gating on isGuest vs isLoggedIn"
rg -n -C2 -P '\bisGuest\b' --type=ts --type=tsx || true
rg -n -C2 -P '\bisLoggedIn\b' --type=ts --type=tsx || trueLength of output: 256
Let’s simplify the search and directly locate the store definition and all uses of these flags:
This will verify:
- How
useAuthStoreis implemented. - Whether
isGuestandisLoggedInare strictly derived from one another. - Where else in
.ts/.tsxwe gate UI or logic on these flags.
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "1. Files that reference useAuthStore:"
rg -l --type=ts --type=tsx 'useAuthStore' || true
echo
echo "2. Inspecting useAuthStore definitions and related methods:"
rg -n 'useAuthStore' -C3 --type=ts --type=tsx || true
echo
echo "3. In each candidate file, show isGuest, isLoggedIn, login, logout:"
for file in $(rg -l --type=ts --type=tsx 'useAuthStore'); do
echo ">>> $file"
rg -n -C3 '\b(isGuest|isLoggedIn|login|logout)\b' --type=ts --type=tsx "$file" || true
done
echo
echo "4. Components gating on isGuest:"
rg -n -C2 '\bisGuest\b' --type=ts --type=tsx || true
echo
echo "5. Components gating on isLoggedIn:"
rg -n -C2 '\bisLoggedIn\b' --type=ts --type=tsx || trueLength of output: 962
Let's try without relying on built-in file-type filters:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "1. Files referencing useAuthStore:"
rg -l 'useAuthStore' -g '*.ts' -g '*.tsx' || true
echo
echo "2. Inspecting useAuthStore definitions and related methods:"
rg -n 'useAuthStore' -C3 -g '*.ts' -g '*.tsx' || true
echo
echo "3. In each candidate file, show isGuest, isLoggedIn, login, logout:"
for file in $(rg -l 'useAuthStore' -g '*.ts' -g '*.tsx'); do
echo ">>> $file"
rg -n -C3 '\b(isGuest|isLoggedIn|login|logout)\b' "$file" || true
done
echo
echo "4. Components gating on isGuest:"
rg -n '\bisGuest\b' -g '*.ts' -g '*.tsx' || true
echo
echo "5. Components gating on isLoggedIn:"
rg -n '\bisLoggedIn\b' -g '*.ts' -g '*.tsx' || trueLength of output: 27599
Unify auth gating: pick a single flag for UI and logic
Currently, login()/logout() only toggle isLoggedIn — isGuest never changes — so gating on !isGuest (render) but deriving nav items from isLoggedIn can drift and break the UI.
• components/mypage/MyPage.tsx
– Line 30: const { isLoggedIn, login, logout, isGuest } = useAuthStore()
– Line 37: const navigationItems = isLoggedIn ? […] : […]
– Line 90: {!isGuest ? (…) : (…)}
Suggested fix (choose one):
- Standardize on
isLoggedIn:
– Replace{!isGuest}with{isLoggedIn}for rendering
– Compute nav items and all gating fromisLoggedInonly - Standardize on
isGuest:
– Updatelogin()to alsoset({ isGuest: false })
– Updatelogout()to alsoset({ isGuest: true })
– Compute nav items from!isGuesteverywhere
🤖 Prompt for AI Agents
In components/mypage/MyPage.tsx around lines 30, 37 and 90, the component mixes
isLoggedIn and isGuest causing UI drift; standardize on isLoggedIn: keep using
const { isLoggedIn, login, logout } = useAuthStore() (remove reliance on
isGuest), change the navigationItems computation to use isLoggedIn (already
does), replace the render conditional at line 90 from {!isGuest ? … : …} to
{isLoggedIn ? … : …}, and remove or stop reading isGuest anywhere in this file
(or add a TODO to remove from the store if unused).
| // <Modal.Button | ||
| // onClick={() => { | ||
| // onWait(); | ||
| // }} | ||
| // key={"wait"} | ||
| // text="잠깐만요!" | ||
| // color="alternative" | ||
| // />, |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Remove dead onWait prop or re-enable the “잠깐만요!” action
You’ve commented out the button that used to call onWait, but the prop is still part of the component API and is passed by callers. This is dead API surface and can confuse future maintainers.
- Option A: Remove
onWaitfrom the props and all call sites. - Option B: Re-enable the button intentionally.
[suggest_optional_refactor]
Run this to find all onWait usages before removal:
If you choose removal, here’s a minimal diff for this file (you’ll need to update call sites too):
-interface ModalCompletingGoalProps extends ModalCommon {
- onWait: () => void;
- onCompleteGoal: () => Promise<void>;
-}
+interface ModalCompletingGoalProps extends ModalCommon {
+ onCompleteGoal: () => Promise<void>;
+}
const ModalCompletingGoal = ({
onClose,
- onWait,
onCompleteGoal,
}: ModalCompletingGoalProps) => {🏁 Script executed:
#!/bin/bash
rg -nP --type=ts,tsx -C2 '\bonWait\b'Length of output: 70
🏁 Script executed:
#!/bin/bash
# Search for all usages of `onWait` in .ts and .tsx files
rg -nP -C2 '\bonWait\b' -g '*.ts' -g '*.tsx'Length of output: 2965
Remove unused onWait API or restore the “잠깐만요!” button
You’ve commented out the “잠깐만요!” action but left the onWait prop in the component signature and callers—this creates dead API surface and unused locals. Address it in one of two ways:
Option A: Remove onWait everywhere
- In ModalCompletingGoal.tsx
• DeleteonWaitfromModalCompletingGoalProps
• RemoveonWaitfrom the destructuring
• Delete the commented-out<Modal.Button>block - In call sites
• app/details/[goal_id]/_components/DetailsBody/DetailBody.tsx – removeonWait={closeModal}
• components/shared/Modal/ModalCompletingGoal/ModalCompletingGoal.stories.tsx – drop theonWaitargType and stub
Option B: Re-enable the button
- Uncomment lines 27–34 in ModalCompletingGoal.tsx so that
onWait()is actually called
Choose one approach to keep the API consistent.
🤖 Prompt for AI Agents
In components/shared/Modal/ModalCompletingGoal/ModalCompletingGoal.tsx around
lines 27 to 34, the component still exposes an unused onWait prop while the
“잠깐만요!” button is commented out; remove the dead API by deleting onWait from
ModalCompletingGoalProps and from the props destructuring, and remove the
commented-out Modal.Button block; then update call sites: remove
onWait={closeModal} from
app/details/[goal_id]/_components/DetailsBody/DetailBody.tsx and drop the onWait
argType/stub from
components/shared/Modal/ModalCompletingGoal/ModalCompletingGoal.stories.tsx so
the prop is fully removed everywhere.
| <h3 className="justify-center text-label-alternative text-base font-bold font-['SUIT_Variable'] leading-tight"> | ||
| 남은 기간 | ||
| </h3> | ||
| <p className="justify-center text-label-primary text-base font-bold font-['SUIT_Variable'] leading-tight">{`D${leftDay >= 0 ? "-" : "+"}${leftDay !== 0 ? Math.abs(leftDay) : "Day"}`}</p> |
There was a problem hiding this comment.
게스트 모드에서 보면 NaN 이 출력되는거 같아요
There was a problem hiding this comment.
로딩되는 동안에는 "" 나 0 로 로 처리하거나 하는게 낫지않을까 싶습니당
| const bottomSheet = document.querySelector( | ||
| // "#fixer", | ||
| "#bottom-sheet", | ||
| ) as HTMLDivElement; |
There was a problem hiding this comment.
useRef 로 변경할 수 있을 것 같아요
const bottomSheetRef = useRef<HTMLDivElement>(null);
..
const handleMobileKeyboardResize = () => {
const bottomSheet = bottomSheetRef.current;
..
<Drawer.Content id="bottom-sheet" ref={bottomSheetRef}>There was a problem hiding this comment.
이거 처음엔 useRef로 했는데, 이러니까 동작 처리가 느리더라구요...
일단 id로 처리하고 추후에 아예 통으로 변경할까 합니답...
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
app/details/[goal_id]/_components/DetailsBody/DetailBody.tsx (1)
45-47: Fix incorrect todo refetch: bindmutateper sub-goalCurrently, you call
const { mutate } = useSubGoalTodosAllInfinite( data.subGoals?.[targetSubGoalIdx]?.subGoalId ?? "", );once for the initial sub-goal index, but then render all sub-goals in a map. As a result, every
onTodoCheckinvokesmutate()only for that first sub-goal, leaving other lists stale.To resolve this critical bug:
• Remove the
targetSubGoalIdxstate and the top-leveluseSubGoalTodosAllInfinitecall (lines 42–47).
• Inside thedata.subGoals?.mapblock, instantiate the hook for eachsubGoalInfo.subGoalId.
• Call that per-sub-goalmutateinonTodoCheck.Example diff in DetailBody.tsx:
- const [targetSubGoalIdx, setTargetSubGoalIdx] = useState(0); - const { mutate } = useSubGoalTodosAllInfinite( - data.subGoals?.[targetSubGoalIdx]?.subGoalId ?? "", - ); + // Removed: one-off hook. Instantiate per sub-goal below. … {data.subGoals?.map((subGoalInfo) => { + const { mutate: mutateTodos } = useSubGoalTodosAllInfinite(subGoalInfo.subGoalId); return ( <ListCard onTodoCheck={async (todoId) => { const res = await todoApi.toggleTodoCompletion(todoId); if (res) { - mutate(); + mutateTodos(); mutateForSubgoalCompleted(); } }} key={subGoalInfo.subGoalId} … /> ); })}This ensures each sub-goal’s todo list is re-fetched correctly when its check state changes.
♻️ Duplicate comments (6)
app/details/[goal_id]/_components/DetailsBody/DetailBody.tsx (6)
122-193: Avoid closing the bottom sheet on any dependency change (if you re-enable this effect)The cleanup that calls closeBottomSheet will cause close+reopen flicker on every dependency change. Close explicitly when desired or on unmount only; also include stable function deps.
Suggested diff when re-enabling:
- }, [isActive, initContent, data.subGoals?.length, shouldBottomSheetOpened]); + }, [ + isActive, + initContent, + data.subGoals?.length, + shouldBottomSheetOpened, + checkRendered, + openBottomSheet, + updateBottomSheet, + closeBottomSheet, + ]);- return () => { - closeBottomSheet(); - }; + // If you want to close on unmount only: + // useEffect(() => closeBottomSheet, []);
378-406: Let afterSubmit know which sub-goal was affected; adjust signature and callPass the submitted info so the caller can invalidate the correct sub-goal’s list. Also ensures future wiring from the bottom sheet invalidates by subGoalId.
Apply this diff:
-const handleTodoBottomSheetSubmit: ( - newTodoInfo: TodoInfoForSubmission, - afterSubmit: () => void, -) => Promise<boolean> = async (newTodoInfo, afterSubmit: () => void) => { +const handleTodoBottomSheetSubmit: ( + newTodoInfo: TodoInfoForSubmission, + afterSubmit: (submitted: TodoInfoForSubmission) => void, +) => Promise<boolean> = async ( + newTodoInfo, + afterSubmit: (submitted: TodoInfoForSubmission) => void, +) => { @@ - if (isFetchOk) { - afterSubmit(); + if (isFetchOk) { + afterSubmit(newTodoInfo); }Then, at the (currently commented) call site, update afterSubmit to accept the payload and invalidate using newTodoInfo.subGoalId before closing the sheet.
387-395: Ensure date2StringWithSpliter can accept Date | string inputsnewTodoInfo.date is likely typed as Date | string. Confirm the helper’s signature and normalize string inputs (e.g., new Date(value)) inside the helper to avoid runtime errors.
#!/bin/bash # Locate helper signature and call sites fd date2String.ts rg -nC2 'export\s+const\s+date2StringWithSpliter' $(fd date2String.ts) rg -nC2 'date2StringWithSpliter\(' -g '*.ts' -g '*.tsx'If currently typed as (date: Date, ...), update it to (date: Date | string, ...) and guard invalid dates.
79-82: Prevent D+NaN and show “D‑Day” for zero; render “-” when due date missingWhen dueDate is missing you render NaN; also handle D‑Day explicitly for leftDay === 0. This also addresses the guest-mode NaN report.
Apply this diff:
- const leftDay = goalDetail?.dueDate?.dueDate - ? calcLeftDay(goalDetail?.dueDate.dueDate) - : NaN; + const leftDay = goalDetail?.dueDate?.dueDate + ? calcLeftDay(goalDetail?.dueDate.dueDate) + : null;- <p className="justify-center text-label-primary text-base font-bold font-['SUIT_Variable'] leading-tight">{`D${leftDay >= 0 ? "-" : "+"}${leftDay !== 0 ? Math.abs(leftDay) : "Day"}`}</p> + <p className="justify-center text-label-primary text-base font-bold font-['SUIT_Variable'] leading-tight"> + {leftDay === null + ? "-" + : `D${leftDay === 0 ? "-Day" : `${leftDay > 0 ? "-" : "+"}${Math.abs(leftDay)}`}`} + </p>Also applies to: 205-209
89-95: Don’t toggle already-completed sub-goals; only complete the incomplete onesToggling every sub-goal will flip already-completed items back to incomplete.
Apply this diff:
- const subGoalCompletingFeftches = data.subGoals?.map( - (subGoalInfo) => { - return subGoalApi.subGoalCompleteToggle( - subGoalInfo?.subGoalId ?? "", - ); - }, - ); + const subGoalCompletingFeftches = data.subGoals + ?.filter((sg) => !sg.isCompleted) + .map((sg) => + subGoalApi.subGoalCompleteToggle(sg?.subGoalId ?? ""), + );
53-62: Fix false-positive “all todos completed” when there are zero todosCurrent reduce-equality yields true for empty lists (0 === 0), which will trigger the completion modal even when there are no todos.
Apply this diff:
- const allTodoCompleted = - data.subGoals && - data.subGoals.reduce((acc, cur) => { - const count = - cur.initTodoItemsInfo?.filter((info) => info.checked).length ?? 0; - return count + acc; - }, 0) === - data.subGoals.reduce((acc, cur) => { - return acc + cur.initTodoTotalLen; - }, 0); + const totalTodos = + data.subGoals?.reduce((acc, cur) => acc + (cur.initTodoTotalLen ?? 0), 0) ?? 0; + const completedTodos = + data.subGoals?.reduce( + (acc, cur) => acc + (cur.initTodoItemsInfo?.filter((i) => i.checked).length ?? 0), + 0, + ) ?? 0; + const allTodoCompleted = totalTodos > 0 && completedTodos === totalTodos;
🧹 Nitpick comments (1)
app/details/[goal_id]/_components/DetailsBody/DetailBody.tsx (1)
214-219: Use the rounded fixedProgress consistently in UIYou compute fixedProgress above but render goalDetail.progress directly. Use fixedProgress for consistent display.
- style={{ width: `${goalDetail?.progress ?? 0}%` }} + style={{ width: `${fixedProgress}%` }} ... - {`${goalDetail?.progress ?? 0}%`} + {`${fixedProgress}%`}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
app/details/[goal_id]/_components/DetailsBody/DetailBody.tsx(8 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
app/details/[goal_id]/_components/DetailsBody/DetailBody.tsx (4)
utils/calcLeftDay.ts (1)
calcLeftDay(1-12)api/service.ts (2)
subGoalApi(90-90)todoApi(88-88)components/shared/BottomSheets/TodoBottomSheet/TodoBottomSheet.tsx (1)
TodoInfoForSubmission(255-255)utils/date2String.ts (1)
date2StringWithSpliter(7-7)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: chromatic-test
| {data.subGoals?.map((subGoalInfo) => ( | ||
| <ListCard | ||
| onTodoCheck={async (todoId) => { | ||
| const res = await todoApi.toggleTodoCompletion(todoId); | ||
| if (res) { | ||
| mutate(); | ||
| mutateForSubgoalCompleted(); | ||
| } | ||
| }} | ||
| key={subGoalInfo.subGoalId} | ||
| initTodoInfoList={subGoalInfo?.initTodoItemsInfo} | ||
| subGoalInfo={{ | ||
| id: subGoalInfo?.subGoalId, | ||
| // idx: targetSubGoalIdx, | ||
| name: subGoalInfo?.subGoal, | ||
| // totalSubGoalsLen: data.subGoals?.length ?? 0, | ||
| isCompleted: subGoalInfo?.isCompleted, | ||
| }} | ||
| /> | ||
| ))} | ||
| {/* <ListCard |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Invalidate todos for the correct sub-goal and add stable key fallback
- Refetch the toggled sub-goal’s todos via query invalidation by subGoalId (not the static mutate bound to index 0).
- Add a fallback to the list key to avoid undefined keys at runtime.
Apply this diff:
- {data.subGoals?.map((subGoalInfo) => (
+ {data.subGoals?.map((subGoalInfo, idx) => (
<ListCard
onTodoCheck={async (todoId) => {
const res = await todoApi.toggleTodoCompletion(todoId);
if (res) {
- mutate();
+ // Refetch the toggled sub-goal's todos only
+ await queryClient.invalidateQueries({
+ queryKey: [
+ "subGoalTodosAllInfinite",
+ { subGoalId: subGoalInfo?.subGoalId ?? "" },
+ ],
+ });
mutateForSubgoalCompleted();
}
}}
- key={subGoalInfo.subGoalId}
+ key={subGoalInfo?.subGoalId ?? `subgoal-${idx}`}Additional changes required outside this hunk:
- Import and initialize the query client near the top of the component.
TypeScript (outside selected lines):
// import
import { useQueryClient } from "@tanstack/react-query";
// inside DetailBody component, near other hooks
const queryClient = useQueryClient();새로고침 막고, mutate처리 및 뒤로가기 정상화
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
app/adding-goal/page.tsx (1)
40-85: Fix useEffect dependencies and lifecycle; always close on cleanupCurrently missing deps will trigger ESLint and may cause stale closures. Also, you build
bottomSheetInfoeven when closed and only conditionally close on cleanup. Early-return when closed, unify open/update, and include all external deps. Also fix theisOpendedtypo.Apply this diff:
- useEffect(() => { - if (!openBottomSheet) { - closeBottomSheet(); - } - - const bottomSheetInfo: Parameters<typeof openGoalDurationBottomSheet>[0] = { + useEffect(() => { + if (!openBottomSheet) { + closeBottomSheet(); + return; + } + + const bottomSheetInfo: Parameters<typeof openGoalDurationBottomSheet>[0] = { backdropProps: { className: "fixed inset-0 z-20 bg-neutral-700/50 ", onClick: () => { setOpenBottomSheet(false); }, }, ContentComponent: GoalDurationBottomSheet, hasBackdrop: true, bottomSheetFixerStyle: { bottom: 0, }, contentProps: { setopenBottomSheet: setOpenBottomSheet, onEdit: async ({ type, value }) => { setGoalAddInfo((prev) => ({ ...prev, durationType: type, durationValue: value, })); // 닫기 setOpenBottomSheet(false); }, }, }; - const isOpended = checkOpened(); - if (openBottomSheet && !isOpended) { - openGoalDurationBottomSheet(bottomSheetInfo); - return; - } - - if (openBottomSheet) { - updateGoalDurationBottomSheet(bottomSheetInfo); - } - - return () => { - if (!openBottomSheet) { - closeBottomSheet(); - } - }; - }, [openBottomSheet]); + const isOpened = checkOpened(); + if (!isOpened) { + openGoalDurationBottomSheet(bottomSheetInfo); + } else { + updateGoalDurationBottomSheet(bottomSheetInfo); + } + + return () => { + // Always close to prevent a lingering sheet on unmount/navigation + closeBottomSheet(); + }; + }, [ + openBottomSheet, + checkOpened, + closeBottomSheet, + openGoalDurationBottomSheet, + updateGoalDurationBottomSheet, + ]);
🧹 Nitpick comments (4)
app/adding-goal/page.tsx (4)
33-38: Remove unusedcheckRenderedto satisfy no-unused-vars
checkRenderedis destructured but never used, likely tripping ESLint.Apply this diff:
const { checkOpened, - checkRendered, closeBottomSheet, openBottomSheet: openGoalDurationBottomSheet, updateBottomSheet: updateGoalDurationBottomSheet, } = useBottomSheet<Parameters<typeof GoalDurationBottomSheet>[0]>();
99-123: Add pending state and error handling to prevent duplicate submissionsRapid taps can fire multiple creates; also no error handling/UI feedback. Guard with an
isSubmittingflag and wrap with try/catch/finally.Apply this diff within onSubmit:
- onSubmit={async (e) => { - e.preventDefault(); - const res = await goalApi.createGoal({ + onSubmit={async (e) => { + e.preventDefault(); + if (isSubmitting) return; + setIsSubmitting(true); + try { + const res = await goalApi.createGoal({ isPeriodByMonth: goalAddInfo.durationType === "month", title: goalAddInfo.goal, dueDate: goalAddInfo.durationType === "month" ? undefined : date2StringWithSpliter( goalAddInfo.durationValue as Date, "-", ), month: goalAddInfo.durationType === "month" ? (goalAddInfo.durationValue as number) : undefined, subGoals: goalAddInfo.subGoals.map((subGoalName) => ({ title: subGoalName, })), }); - if (res) { - mutate(); - router.back(); - } + if (res) { + // If using React Query: await refetch(); otherwise mutate(); + mutate?.(); + router.back(); + } + } catch (err) { + // TODO: surface error (toast/snackbar) + console.error(err); + } finally { + setIsSubmitting(false); + } }}And add this state near your other
useStatecalls:const [isSubmitting, setIsSubmitting] = useState(false);
229-236: Disable the submit button while submitting for better UXAlso reflect disabled styling when
isSubmittingis true.Apply this diff:
- disabled={!goalAddInfo.goal} - className={`w-82 h-14 px-6 py-4 ${!goalAddInfo.goal ? "bg-background-disabled" : "bg-background-strong"} rounded-[999px] inline-flex justify-center items-center gap-2 overflow-hidden`} + disabled={!goalAddInfo.goal || isSubmitting} + className={`w-82 h-14 px-6 py-4 ${!goalAddInfo.goal || isSubmitting ? "bg-background-disabled" : "bg-background-strong"} rounded-[999px] inline-flex justify-center items-center gap-2 overflow-hidden`}And the label color:
- className={`flex-1 text-center justify-center ${!goalAddInfo.goal ? "text-label-disabled" : "text-label-inverse"} text-xl font-bold font-['SUIT'] leading-normal`} + className={`flex-1 text-center justify-center ${!goalAddInfo.goal || isSubmitting ? "text-label-disabled" : "text-label-inverse"} text-xl font-bold font-['SUIT'] leading-normal`}
240-252: Remove commented-out legacy JSXThe old inline
<GoalDurationBottomSheet />block is commented out. Dead code adds noise and can drift.Apply this diff to delete it:
- {/* <GoalDurationBottomSheet - openBottomSheet={openBottomSheet} - setopenBottomSheet={setOpenBottomSheet} - onEdit={async ({ type, value }) => { - setGoalAddInfo((prev) => ({ - ...prev, - durationType: type, - durationValue: value, - })); - // 닫기 - setOpenBottomSheet(false); - }} - /> */}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
app/adding-goal/page.tsx(6 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
app/adding-goal/page.tsx (1)
api/hooks.ts (1)
useGoals(174-174)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: chromatic-test
🔇 Additional comments (2)
app/adding-goal/page.tsx (2)
31-39: Nice adoption of the centralized bottom-sheet API with strong typingGood use of
useBottomSheet<Parameters<typeof GoalDurationBottomSheet>[0]>()and aliasingopenBottomSheettoopenGoalDurationBottomSheetto avoid name collision with local state. This improves clarity and type-safety.
29-30: No change needed—useGoalsis SWR-based and returnsmutateThe
useGoalshook is an alias foruseQuery.goals, which callsuseApiQuerywith anSWRConfigurationand under the hood uses SWR’suseSWR. SWR hooks exposemutate(notrefetch), so keepingconst { mutate } = useGoals()is correct.
todo 개수 0이 아닌 경우에 동작하도록 수정
2 participants
버그 수정
기능 추가
Summary by CodeRabbit
New Features
Improvements
Documentation