Skip to content
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions apps/client/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@
"preview": "vite preview"
},
"dependencies": {
"class-variance-authority": "^0.7.1",
"framer-motion": "^12.23.12",
"react": "^19.1.1",
"react-dom": "^19.1.1",
"react-router-dom": "^7.8.2"
Expand Down
10 changes: 10 additions & 0 deletions apps/client/src/assets/onBoarding/background/onBoardingBg.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
9 changes: 9 additions & 0 deletions apps/client/src/assets/onBoarding/icons/chippi_bell.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
9 changes: 9 additions & 0 deletions apps/client/src/assets/onBoarding/icons/chippi_morning.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
21 changes: 21 additions & 0 deletions apps/client/src/assets/onBoarding/icons/chippi_night.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
9 changes: 9 additions & 0 deletions apps/client/src/assets/onBoarding/icons/dotori.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
3 changes: 3 additions & 0 deletions apps/client/src/assets/onBoarding/icons/header_logo.svg
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

로고는 사이드바에서도 사용하고 익스텐션에서등 다양한 위치에서 사용된다고 생각하는데 디자인 시스템에 추가하는 것은 어떤가요?
괜찮으시다면 추후 제 사이드바 pr에서 로고 디자인 시스템 위치에 아이콘 적용해뒀는데 가져다 사용해도 좋을것같아요-!

Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
9 changes: 9 additions & 0 deletions apps/client/src/assets/onBoarding/story/macNotice.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
9 changes: 9 additions & 0 deletions apps/client/src/assets/onBoarding/story/story1.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
9 changes: 9 additions & 0 deletions apps/client/src/assets/onBoarding/story/story2.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
9 changes: 9 additions & 0 deletions apps/client/src/assets/onBoarding/story/story3.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
6 changes: 0 additions & 6 deletions apps/client/src/layout/Layout.tsx
Original file line number Diff line number Diff line change
@@ -1,16 +1,10 @@
import { Outlet } from 'react-router-dom';
import OptionsMenuButton from '@shared/components/optionsMenuButton/OptionsMenuButton';

const Layout = () => {
return (
<>
{/* TODO: 필요시 레이아웃 추가 */}
{/* TODO: 사이드바 추가 */}
<OptionsMenuButton
onEdit={function (): void {}}
onDelete={function (): void {}}
></OptionsMenuButton>

<Outlet />
</>
);
Expand Down
13 changes: 12 additions & 1 deletion apps/client/src/pages/onBoarding/OnBoarding.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,16 @@
import onBoardingBg from '../../assets/onBoarding/background/onBoardingBg.svg';
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

필요하면 assets도 절대 경로에 추가해도 될 것 같은데 어떻게 생각하시나요!

import Header from './components/header/Header';
import MainCard from './components/funnel/MainCard';
const OnBoarding = () => {
return <div>OnBoarding</div>;
return (
<div
className={`relative flex h-screen w-screen items-center justify-center bg-cover bg-center bg-no-repeat`}
style={{ backgroundImage: `url(${onBoardingBg})` }}
>
<Header />
<MainCard />
</div>
);
};

export default OnBoarding;
137 changes: 137 additions & 0 deletions apps/client/src/pages/onBoarding/components/funnel/MainCard.tsx
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

전체적으로 step을 컴포넌트로 잘 분리해주셨는데 해당 step에 관련된 코드가 각 step안이 아니라 해당 MainCard에 있는 것 같아요. 관련 코드를 step안으로 넣어주고, 책임을 확실히 해주는 것이 어떨까요?

현재는 step에 해당하는 코드가 step 컴포넌트가 아니라 해당 MainCard안에 있어서 조금 어색한 느낌이 드는 것 같아요!

Original file line number Diff line number Diff line change
@@ -0,0 +1,137 @@
import { Progress, Button } from '@pinback/design-system/ui';
import { useState, useEffect } from 'react';
import { motion, AnimatePresence } from 'framer-motion';
import StoryStep from './step/StoryStep';
import AlarmStep from './step/AlarmStep';
import MacStep from './step/MacStep';
import FinalStep from './step/FinalStep';
import { cva } from 'class-variance-authority';
const stepProgress = [{ progress: 30 }, { progress: 60 }, { progress: 100 }];

const variants = {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

variants 네이밍을 조금 더 구체화해주세요!

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

slideIn~Out 느낌으로 네이밍 구체화 했습니당

enter: (direction: number) => ({
x: direction > 0 ? 200 : -200,
opacity: 0,
}),
center: { x: 0, opacity: 1 },
exit: (direction: number) => ({
x: direction > 0 ? -200 : 200,
opacity: 0,
}),
};
const CardStyle = cva(
'bg-white-bg flex h-[54.8rem] w-[63.2rem] flex-col items-center justify-between rounded-[2.4rem] pt-[3.2rem]',
{
variants: {
overflow: {
true: 'overflow-visible',
false: 'overflow-hidden',
},
},
defaultVariants: { overflow: false },
}
);
const MainCard = () => {
const [step, setStep] = useState(0);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

제 개인적인 생각으로는 stepnumber로 관리하게 되면 해당 step이 무엇인지 각 컴포넌트에서 파악하기가 쉽지 않다고 생각해요.

예를 들어

 <Button
    variant="primary"
    size="medium"
    isDisabled={step === 6}
    className="ml-auto w-[4.8rem]"
    onClick={nextStep}
>

이런 코드도 step이 6이라는 것을 해당 코드만 보고는 쉽게 알지 못한다고 생각해요.
따라서 stepstring 형식으로, 더 타입을 안정화하려면 union으로 'mac', 'story' 등으로 가드해두면 자동 완성/안정성도 좋아지고 가독성 측면에서 훨씬 좋은 코드가 될 것 같다고 생각해요.

재림님은 이 부분에 대해서 어떻게 생각하시나요!

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

안그래도 구현할때, step 몇이 어떤단계인지 저도 헷갈렸었는데,, 너무 좋은 방향인 것 같아요!!
union으로 명시화해두겠습니당

const [direction, setDirection] = useState(0);
const [alarmSelected, setAlarmSelected] = useState<1 | 2 | 3>(1);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

1,2,3 보다 더 의미 있는 string 값으로 가독성을 높이는 것은 어떨까요?

const [isMac, setIsMac] = useState(false);

useEffect(() => {
const ua = navigator.userAgent.toLowerCase();
// Mac OS (Intel/ARM), iPhone, iPad 까지 포함하고 싶으면 아래처럼 체크 가능
if (ua.includes('mac os') || ua.includes('iphone') || ua.includes('ipad')) {
setIsMac(true);
}
}, []);
const renderStep = () => {
switch (step) {
case 0:
case 1:
case 2:
return <StoryStep step={step as 0 | 1 | 2} />;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

StoryStep props interface에서

interface StoryStepProps {
  step: 0 | 1 | 2;
}

union으로 구체화 해주셔서 잘 타입이 추론 되는 것 같아요! as로 타입 단언을 사용하신 특별한 이유가 있을까요?


image

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

혹시 스텝꼬일까봐
step이 0,1,2 중 하나인 걸 보장할때만 하도록 타입 단언해두긴 했습니다.! 너무 불필요한 단언이라면 뺴두겠습니다 :)

case 3:
return (
<AlarmStep selected={alarmSelected} setSelected={setAlarmSelected} />
);
case 4:
return isMac ? <MacStep /> : <FinalStep />;
case 5:
return <FinalStep />;
default:
Comment on lines +56 to +60
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue

비-Apple(Windows 등) 환경에서 step 5가 공백으로 렌더되고, ‘다음’을 두 번 눌러야 홈으로 이동하는 버그

step 5가 !isMac일 때 null을 반환하고, 리다이렉트는 step 5에서만 발생합니다. step 4에서 step 5로 이동(첫 클릭) 후 빈 화면이 표시되고, 한 번 더 눌러야 이동됩니다. 아래처럼 비Mac은 step 4에서 바로 리다이렉트하고, 안전망으로 step 5는 항상 FinalStep을 렌더하도록 수정하세요.

@@
-      case 5:
-        if (isMac) return <FinalStep />;
-        return null;
+      case 5:
+        return <FinalStep />;
@@
-  const nextStep = () => {
+  const nextStep = () => {
     if (step === 3) {
       // 이거 이후에 api 붙일 자리 표시임! console.log('선택된 알람:', AlarmsType[alarmSelected - 1].time);
     }
-    if (step < 5) {
-      setDirection(1);
-      setStep((prev) => prev + 1);
-    } else if (step === 5) {
-      window.location.href = '/';
-    }
+    const shouldRedirect = (!isMac && step === 4) || (isMac && step === 5);
+    if (shouldRedirect) {
+      window.location.href = '/';
+      return;
+    }
+    setDirection(1);
+    setStep((prev) => prev + 1);
   };

Also applies to: 65-75

🤖 Prompt for AI Agents
In apps/client/src/pages/onBoarding/components/funnel/MainCard.tsx around lines
56-60 (and similarly 65-75), the switch currently returns null for case 5 on
non-Mac causing a blank render and requiring a second click to trigger the
redirect; change the flow so non-Mac users are redirected earlier and case 5
never returns null. Specifically: update case 4 to perform the non-Mac redirect
immediately (same redirect logic used for Mac) so clicking Next from step 4
moves straight to home, and change case 5 to always render FinalStep as a safety
net (do not return null). Apply the same fixes in the repeated block at lines
65-75.

return <FinalStep />;
}
Comment on lines +57 to +62
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

이렇게 될 때 window 사용자는 final step을 2번 볼 가능성은 없을까요?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

헉 이부분 클날뻔했네요!! 분기를 꼬이게 잘못 설계한 것 같아서 수정했습니다! 감사합니다~~!

};
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

stepstring으로 하면 어떨지 관련 코멘트 확인해주세요!

추가로 storyStep에서 캐러셀로 구현되는 부분도 step으로 분리를 해주셨는데, 의미상으로는 storyStep은 동일하고 안에 이미지가 progress bar를 통해 바뀌기만 하는 로직같아요.

이 부분도 step에 포함시키신 이유가 있으실까요? step이라는 의미 자체로는 storyStep이 공통으로 가지고 있는 것 같아서 질문 드립니다!

const nextStep = () => {
if (step === 3) {
// 이거 이후에 api 붙일 자리 표시임! console.log('선택된 알람:', AlarmsType[alarmSelected - 1].time);
}
if (step < 5) {
setDirection(1);
setStep((prev) => prev + 1);
} else if (step === 5) {
window.location.href = '/';
}
};

const prevStep = () => {
if (step > 0) {
setDirection(-1);
setStep((prev) => prev - 1);
}
};
Comment on lines +65 to +82
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

step string 관련 의논을 해보고 필요하다면 로직 관련은 커스텀 훅으로 분리하는 것도 좋을 것 같아요!


return (
<div className={CardStyle({ overflow: step === 3 && alarmSelected === 3 })}>
{step < 3 && (
<Progress
value={stepProgress[step].progress}
variant="profile"
className="w-[30%]"
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

30%가 figma상으로 정확한 width 값인가요?!?!?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

px/rem로 박으면 너무 쪼그라들거나 작아보이는 거 같아서 %로 하긴했는데, 이부분은 QA때 디바이스 별로 확인하는 걸로 하고 실제 width로 다시 지정해두겠습니당 :)

/>
)}
Comment on lines +86 to +92
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

해당 progress bar는 story step에만 들어가는 컴포넌트 아닌가요?!
step 컴포넌트 잘 분리 해주셨는데, story step안으로 넣어주는 것은 어떨까요? MainCard는 말 그대로 step들을 합친 역할을 할 뿐이니!

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

아 해당 mainCard 는 패딩 레이아웃 다 통일해둔 흰박스레이아웃 안에 안에 내용물만 슬라이딩 하면서 바뀌게 되는데요!
Progress를 storyStep에 넣게 되면,

  1. 프로그레스 바도 같이 슬라이딩이 되어버리는 ux흐름이 되는 문제와
  2. 프로그레스 바 위치에는 storyStep이후의 스텝에서는 도토리 위치가 들어오는 구조로, mainCard 큰 틀 패딩 레이아웃을 다 통일했어서,! ui 위쪽 패딩이 깨지는 문제가 생겨서

다음처럼 설계했었습니다! 좋은 의견 있다면 말씀주세요!


<div className="relative flex h-full w-full items-center justify-center">
<AnimatePresence custom={direction} mode="wait">
<motion.div
key={step}
custom={direction}
variants={variants}
initial="enter"
animate="center"
exit="exit"
transition={{ duration: 0.4 }}
className="flex h-full flex-col items-center"
>
{renderStep()}
</motion.div>
</AnimatePresence>
</div>

<div className="mb-[4.8rem] mt-[1.2rem] flex w-full justify-between px-[3.2rem]">
{step < 4 && (
<Button
variant="primary"
size="medium"
isDisabled={step === 0}
className="w-[4.8rem]"
onClick={prevStep}
>
이전
</Button>
)}
<Button
variant="primary"
size="medium"
isDisabled={step === 6}
className="ml-auto w-[4.8rem]"
onClick={nextStep}
>
다음
</Button>
</div>
Comment on lines +123 to +132
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Sep 6, 2025

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

다음 버튼 비활성화 조건 버그(step === 6) 수정

step은 0–5 범위로 운용되므로 현재 조건은 영원히 false입니다. 의미 없는 prop 전달이므로 제거하거나 전환 중(isAnimating) 비활성화로 교체하세요.

전환 중 비활성화로 교체 예:

-          isDisabled={step === 6}
+          isDisabled={isAnimating}

단순 제거 예:

-          isDisabled={step === 6}
+          // isDisabled 제거
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
<Button
variant="primary"
size="medium"
isDisabled={step === 6}
className="ml-auto w-[4.8rem]"
onClick={nextStep}
>
다음
</Button>
</div>
<Button
variant="primary"
size="medium"
isDisabled={isAnimating}
className="ml-auto w-[4.8rem]"
onClick={nextStep}
>
다음
</Button>
</div>
🤖 Prompt for AI Agents
In apps/client/src/pages/onBoarding/components/funnel/MainCard.tsx around lines
123 to 132 the Button has isDisabled={step === 6} which is always false because
step ranges 0–5; remove this meaningless prop or replace it with a real
animation/transition flag (e.g., isDisabled={isAnimating}) so the Next button is
disabled during transitions; update the component to either drop isDisabled
entirely or wire it to the existing isAnimating state (ensure isAnimating is
defined and passed down or managed in this component).

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

이부분 한번 확인해주세요-!

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

@jjangminii 네, 확인했습니다! CodeRabbit이 지적한 버그가 맞습니다.

문제점:

  • isDisabled={step === 6} 조건이 의미없습니다
  • step 상태는 0-5 범위에서만 동작하므로 step === 6은 항상 false입니다
  • 따라서 "다음" 버튼이 실제로는 비활성화되지 않습니다

수정 방안:

  1. 단순 제거 (가장 간단):
<Button
  variant="primary"
  size="medium"
  className="ml-auto w-[4.8rem]"
  onClick={nextStep}
>
  다음
</Button>
  1. 애니메이션 중 비활성화 (더 나은 UX):
// 상태 추가
const [isAnimating, setIsAnimating] = useState(false);

// 애니메이션 처리
const nextStep = () => {
  setIsAnimating(true);
  // ... 기존 로직
  setTimeout(() => setIsAnimating(false), 400); // 애니메이션 duration과 맞춤
};

// 버튼에서 사용
<Button
  variant="primary"
  size="medium"
  isDisabled={isAnimating}
  className="ml-auto w-[4.8rem]"
  onClick={nextStep}
>
  다음
</Button>

어떤 방향으로 수정하실 건가요?

</div>
);
};

export default MainCard;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

이 파일은 step에 들어가는 것이 아닌 것 같아요!

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

잘 구현해주셨네요! 다만 안에서 모든 분기처리를 하는 방법도 있지만, 단순 텍스트와 이미지를 외부에서 주입하게 하는 방법도 있을 것 같아요.

인터페이스를 생각해보면

title
subTitle (optional) // 마지막 3번째 box는 없으니!
imageUrl
onClick
isActive

이런 느낌!

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

아하! 그러네요 추후 리팩토링 때, 이 방법이 더 관리가 쉽다면 수정해보겠습니다! 아이디어 감사합니다

Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
import { cva } from 'class-variance-authority';
import TimePicker from '../../timePicker/TimePicker';
import { AlarmsType } from '../type/alarms';
import { useState } from 'react';
interface AlarmBoxProps {
select: 1 | 2 | 3;
isDisabled: boolean;
onClick?: () => void;
}

const boxStyle = cva(
'flex h-[22.4rem] w-[18rem] flex-col items-center rounded-[1.2rem] px-[3.9rem] pb-[2.6rem] pt-[3.6rem] cursor-pointer transition',
{
variants: {
disabled: {
true: 'border-main400 bg-main100 border',
false: 'bg-white border border-transparent hover:border-main300',
},
},
defaultVariants: { disabled: false },
}
);

const AlarmBox = ({ select, isDisabled, onClick }: AlarmBoxProps) => {
const [showPicker, setShowPicker] = useState(false);

return (
<div
className={boxStyle({ disabled: isDisabled })}
onClick={() => {
if (select === 3 && isDisabled) {
setShowPicker(true);
}
onClick?.();
}}
>
<img src={AlarmsType[select - 1].img} alt="chippi" />
<p
className={`sub3-sb ${
isDisabled ? 'text-main500' : 'text-font-black-1'
}`}
>
{AlarmsType[select - 1].title}
</p>

{select <= 2 && (
<p className="caption2-m text-font-gray-3">
{AlarmsType[select - 1].time}
</p>
)}

{select === 3 && isDisabled && (
<>
{AlarmsType[2].time && (
<p className="caption2-m text-font-gray-3">{AlarmsType[2].time}</p>
)}

{showPicker && (
<TimePicker
onSave={({ hour, minute, meridiem }) => {
const formatted = `${meridiem} ${hour}:${minute}`;
AlarmsType[2].time = formatted;
setShowPicker(false);
// 이거 나중에 api 연결때 쓸려고 표시한거.. 그떄 지우겠듬여 console.log('저장된 사용자 알람:', AlarmsType[2].time);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

함수로 따로 분리해도 좋을 것 같아요!

}}
onCancel={() => {
AlarmsType[2].time = '';
}}
onClick={(e) => e.stopPropagation()}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

전역 상수 변경과 취소 동작 누락

  1. AlarmsType[2].time을 직접 변경하면 전역 가변 상태가 됩니다. 상위에서 customTime을 state로 내려받아 갱신하세요.
  2. 취소 시 피커가 닫히지 않습니다.
-              onSave={({ hour, minute, meridiem }) => {
-                const formatted = `${meridiem} ${hour}:${minute}`;
-                AlarmsType[2].time = formatted;
-                setShowPicker(false);
-                // 이거 나중에 api 연결때 쓸려고 표시한거.. 그떄 지우겠듬여 console.log('저장된 사용자 알람:', AlarmsType[2].time);
-              }}
-              onCancel={() => {
-                AlarmsType[2].time = '';
-              }}
+              onSave={({ hour, minute, meridiem }) => {
+                const mm = `${minute}`.padStart(2, '0');
+                const formatted = `${meridiem} ${hour}:${mm}`;
+                // TODO: 상위에서 내려준 setCustomTime(formatted) 호출
+                setShowPicker(false);
+              }}
+              onCancel={() => {
+                // TODO: 상위에서 내려준 setCustomTime('') 호출
+                setShowPicker(false);
+              }}

또한 파일 상단 props에 customTime?: string; setCustomTime?: (v: string) => void;를 추가하고, 표시부에서도 AlarmsType[2].time 대신 customTime을 사용하도록 변경해 주세요.

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In apps/client/src/pages/onBoarding/components/funnel/step/AlarmBox.tsx around
lines 52-61, the code mutates the global AlarmsType[2].time and doesn't close
the picker on cancel; change to accept props customTime?: string and
setCustomTime?: (v: string) => void (add to component props at top), use
customTime for display instead of AlarmsType[2].time, call
setCustomTime(formatted) inside onSave and setShowPicker(false), and in onCancel
call setCustomTime('') and setShowPicker(false) (guard calls with optional
chaining if setCustomTime may be undefined).

/>
)}
</>
)}
</div>
);
};

export default AlarmBox;
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
import dotori from '../../../../../assets/onBoarding/icons/dotori.svg';
import AlarmBox from './AlarmBox';
interface AlarmStepProps {
selected: 1 | 2 | 3;
setSelected: (n: 1 | 2 | 3) => void;
}
const AlarmStep = ({ selected, setSelected }: AlarmStepProps) => {
return (
<div className="flex flex-col items-center justify-between">
<img src={dotori} className="mb-[1.2rem]" alt="dotori" />
<p className="head2 text-font-black-1">
도토리 찾으러 갈 시간을 정해볼까요?
</p>
<p className="body2-m text-font-gray-3 mb-[4.3rem] mt-[0.8rem] text-center">
매일 지정한 시간에 저장한 북마크를 리마인드해드려요
</p>

<div className="mb-[2rem] flex w-full items-center justify-center gap-[1.4rem]">
{[1, 2, 3].map((n) => (
<AlarmBox
key={n}
select={n as 1 | 2 | 3}
isDisabled={selected === n}
onClick={() => setSelected(n as 1 | 2 | 3)}
/>
))}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

as를 써도 되고 해당 배열을 따로 분리해서 as const 키워드를 붙여서 값 (리터럴) 자체로 type이 되도록 설정하면 as 안 써도 에러가 안 뜰 거에요!

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

아하! 단언문을 불필요하게 반복하고 있었군요..! as const로 리터럴 타입 그대로 고정하게 할 수 있군녀 꿀정보 감삼다

</div>
Comment on lines +18 to +27
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

라디오 그룹 역할 명시 및 키보드 접근성 개선

세 가지 중 하나를 선택하는 UI는 라디오 그룹 패턴에 적합합니다. role="radiogroup"를 선언하고 각 항목에 role="radio"/aria-checked가 노출되도록 하세요(AlarmBox 수정 포함).

-      <div className="mb-[2rem] flex w-full items-center justify-center gap-[1.4rem]">
+      <div
+        className="mb-[2rem] flex w-full items-center justify-center gap-[1.4rem]"
+        role="radiogroup"
+        aria-label="알람 옵션 선택"
+      >
         {[1, 2, 3].map((n) => (
           <AlarmBox
             key={n}
             select={n as 1 | 2 | 3}
             isDisabled={selected === n}
             onClick={() => setSelected(n as 1 | 2 | 3)}
           />
         ))}
       </div>

추가로 AlarmBox에서 role="radio"aria-checked={isDisabled}를 부여해 주세요.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
<div className="mb-[2rem] flex w-full items-center justify-center gap-[1.4rem]">
{[1, 2, 3].map((n) => (
<AlarmBox
key={n}
select={n as 1 | 2 | 3}
isDisabled={selected === n}
onClick={() => setSelected(n as 1 | 2 | 3)}
/>
))}
</div>
<div
className="mb-[2rem] flex w-full items-center justify-center gap-[1.4rem]"
role="radiogroup"
aria-label="알람 옵션 선택"
>
{[1, 2, 3].map((n) => (
<AlarmBox
key={n}
select={n as 1 | 2 | 3}
isDisabled={selected === n}
onClick={() => setSelected(n as 1 | 2 | 3)}
/>
))}
</div>
🤖 Prompt for AI Agents
In apps/client/src/pages/onBoarding/components/funnel/step/AlarmStep.tsx around
lines 18-27, the selection UI must expose a radiogroup for screen readers and
keyboard users: add role="radiogroup" to the wrapper div and pass role="radio"
plus aria-checked={selected === n} (or aria-checked={isDisabled} if you keep
that prop naming) into each AlarmBox via props; update AlarmBox to accept and
apply role="radio" and aria-checked accordingly and ensure it remains
keyboard-focusable (e.g., tabIndex/Enter/Space handling) so assistive tech and
keyboard users can operate the radio items.

</div>
);
};

export default AlarmStep;
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import dotori from '../../../../../assets/onBoarding/icons/dotori.svg';
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

이 부분에 상대경로에 대해서 저도 코드를 작성하며 가독성이 좋지 못하다고 느꼈는데 절대경로를 더 추가하는 것에 대해 어떻게 생각하시나여

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

절대경로 선언하여 적용해두었습니당!

const FinalStep = () => {
return (
<div className="flex h-full flex-col items-center">
<img src={dotori} className="mb-[1.2rem]" alt="dotori" />
<p className="head2 text-font-black-1">
도토리 찾으러 갈 시간을 정해볼까요?
</p>
</div>
);
};
export default FinalStep;
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
import MacNotice from '../../../../../assets/onBoarding/story/macNotice.svg';
import dotori from '../../../../../assets/onBoarding/icons/dotori.svg';
const MacStep = () => {
return (
<div className="flex h-full flex-col items-center">
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

절대 위치 요소가 기준 요소 없이 배치됩니다 — 컨테이너에 relative 추가 권장

하단 이미지가 absolute인데 상위에 포지셔닝 컨텍스트가 없어 레이아웃이 흔들릴 수 있습니다.

다음으로 수정해 주세요:

-    <div className="flex h-full flex-col items-center">
+    <div className="relative flex h-full flex-col items-center">
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
<div className="flex h-full flex-col items-center">
<div className="relative flex h-full flex-col items-center">
🤖 Prompt for AI Agents
In apps/client/src/pages/onBoarding/components/funnel/step/MacStep.tsx around
line 5, the container <div className="flex h-full flex-col items-center"> is
missing a positioning context while it contains absolutely positioned children;
add the positioning context by adding the "relative" class to this container
(e.g., className="flex h-full flex-col items-center relative") so the
absolute-positioned image is anchored to this element.

<img src={dotori} className="mb-[1.2rem]" alt="dotori" />
<p className="head2 text-font-black-1">
도토리 찾으러 갈 시간을 정해볼까요?
</p>
<p className="body2-m text-font-gray-3 mb-[4.3rem] mt-[0.8rem] text-center">
Mac 사용자는 추가 알림 설정을 진행해 주세요.
</p>
<img src={MacNotice} className="absolute -bottom-[104px]" alt="mac" />
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

이부분 특별히 px로 하신 이유가 있을까요??

</div>
);
};
export default MacStep;
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
import story1 from '../../../../../assets/onBoarding/story/story1.svg';
import story2 from '../../../../../assets/onBoarding/story/story2.svg';
import story3 from '../../../../../assets/onBoarding/story/story3.svg';
interface StoryStepProps {
step: 0 | 1 | 2;
}
const steps = [
{
img: story1,
text: (
<>
깊고 신비한 숲에는 지식 나무가 있어요. <br />
지식 나무는 사람들의 잊힌 기록을 도토리 씨앗으로 바꾼답니다.
</>
),
progress: 30,
},
{
img: story2,
text: (
<>
당신이 정보를 읽고 활용하는 것을 양분삼아, <br />
지식 나무에는 맛있는 도토리 열매가 열려요.
</>
),
progress: 60,
},
{
img: story3,
text: (
<>
다람쥐 치삐는 정보를 활용하지 못해 아직 도토리 만개 숲에 도착하지 못하고
있어요.
<br />
도토리를 모아 치삐가 숲에 닿을 수 있도록 도와주세요!
</>
),
progress: 100,
},
];
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

이건 constants로 분리해도 좋을 것 같아요!

Copy link
Copy Markdown
Member Author

@jllee000 jllee000 Sep 10, 2025

Choose a reason for hiding this comment

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

분리해서 @constants로 끌고 올수있도록 해두었습니다!


const StoryStep = ({ step }: StoryStepProps) => {
return (
<div className="flex flex-col items-center">
<img
src={steps[step].img}
className="mb-[1.6rem] mt-[2.4rem] w-[31.2rem]"
alt="onboarding"
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

step에 따라 이미지가 달라질 수 있으니 alt 속성에도 onboardingstep 관련 명시도 추가해주면 좋을 것 같아요!

/>
<p className="sub4-sb text-center text-black">{steps[step].text}</p>
</div>
);
};

export default StoryStep;
Loading
Loading