Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FE] 행사 생성 페이지에 퍼널 방식 적용 #591

Merged
merged 23 commits into from
Sep 24, 2024
Merged

Conversation

pakxe
Copy link
Contributor

@pakxe pakxe commented Sep 23, 2024

issue

구현 이유

AddBillFunnel 이라는 퍼널 구현체(?)가 도입되었어요. 그래서 비슷하게 퍼널이 적용될 수 있는 곳인 행사 생성 기능도 퍼널을 적용했습니다.




구현 사항

처음에는 퍼널을 아래처럼 구현했어요.

// 추상화 + 불필요한 것들은 트리셰이킹된 코드입니다.

const Component = () => {
  const [step, setStep] = useState('1번 스텝');

  return (
    <>
      {step === '1번 스텝' && <Step1 onNext={() => setStep('2번 스텝')}/>}
      {step === '2번 스텝' && <Step2 onNext={() => setStep('2번 스텝')}/>}
      {step === '3번 스텝' && <Step3/>}
    </>
  )
}


하지만 하나의 스텝에서 다음 스텝으로 넘겨지는 함수(이후부터는 스텝넘기기 함수라고 부르겠습니다.)가 이 컴포넌트 안에 작성되는게 보기 좋지 않아서 useFunnel을 구현하게 되었습니다.

그런데 토스의 퍼널 영상을 보니까 훅 안에서 Funnel, Step이라는 컴포넌트도 return하고 있었습니다.

Funnel 컴포넌트는 현재 스텝에 해당하는 Step 컴포넌트를 찾아 렌더링하는 책임이 있습니다. Step 컴포넌트는 스텝의 이름과 children에 이 스텝에서 렌더링할 컴포넌트를 넘겨받기라는 책임이 있었어요.

그래서 스텝 넘기기 함수와 Funnel, Step을 담아 useFunnel을 구성하게 되었습니다.

그리고 최종적으로 아래와 같은 형태로 행사 생성 퍼널을 구현했어요.

// 추상화 + 불필요한 것들은 트리셰이킹된 코드입니다.

const Component = () => {
  const {moveToNextStep, Step, Funnel} = useFunnel();

  return (
    <Funnel>
      <Step name="1번 스텝">
        <Step1 moveToNextStep={moveToNextStep} />
      </Step>
      
      <Step name="2번 스텝">
        <Step2 moveToNextStep={moveToNextStep} />
      </Step>
      
      <Step name="3번 스텝">
        <Step3 />
      </Step>
    </Funnel>
  );
};


하나의 스텝에서만 사용되는 것이 아닌 여러 스텝에 걸쳐 사용되는 상태는 하나의 훅(useCreateEventData)으로 묶고 행사 생성 퍼널 컴포넌트에서 호출해 하위 스텝으로 내려보내는 방식을 선택했습니다.

이 퍼널에서만 사용되는 전역 상태를 만들어서 사용하는 방법도 있었는데요. 만약 스텝에 하위 컴포넌트가 많고, 거기까지 프롭이 내려가야 했다면 전역 상태를 사용했을 것 같은데 그렇게 깊진 않아서 그냥 드릴링으로 넘겨주게 되었어요.

image


논의하고 싶은 부분(선택)

행사 생성 페이지에서 여러 스텝에 걸쳐 사용되는 상태를 선언해 내려주는 용도의 훅인 useCreateEventData의 이름이 직관적이지 않은 것 같은데 이름 추천 받습니다..

테스트를 최대한 유지하고 싶어서 일단 바뀐 path를 사이프레스에 적용했는데요. cypress에서 행사 이름을 입력하고 "다음" 버튼을 누르면 퍼널이라서 비밀 번호 입력 페이지로 넘어가도 url이 바뀌면 안됩니다. 그런데 /create/event/?경로로 바뀌네요.왜 이러는지 모르겠어요. 혹시 아시는 분 있나요?

@pakxe pakxe added 🖥️ FE Frontend ⚙️ feat feature labels Sep 23, 2024
@pakxe pakxe added this to the lev4 milestone Sep 23, 2024
@pakxe pakxe self-assigned this Sep 23, 2024
@pakxe pakxe changed the title Feature/#575 [FE] 행사 생성 페이지에 퍼널 방식 적용 Sep 23, 2024
Copy link

Copy link

Copy link

Copy link

Copy link

Copy link
Contributor

@jinhokim98 jinhokim98 left a comment

Choose a reason for hiding this comment

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

웨디 고생했어요~ 퍼널 구조로 바꿔준 덕분에 행사를 생성할 때 필드가 더 추가되어도 유연하게 추가할 수 있을 것 같아요!!

그리고 웨디가 cypress에서 url이 event/create/? 질문 떠오르는 것이 있긴한데 아마 form 태그의 submit 기본 동작때문에 그런 것이 아닐까 생각되는데 확인해보세요!

const navigate = useNavigate();

return (
<TextButton onClick={() => navigate(-1)} textSize="bodyBold" textColor="gray">
<TextButton onClick={() => (onClick ? onClick() : navigate(-1))} textSize="bodyBold" textColor="gray">
Copy link
Contributor

Choose a reason for hiding this comment

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

back onClick의 기본값을 navigate(-1)기능을 넣어준 것은 컴포넌트의 이름이 back이기 때문에 ㄷ로가기 기능을 default로 넣어준걸까요?

// 행사 생성 페이지에서 여러 스텝에 걸쳐 사용되는 상태를 선언해 내려주는 용도의 훅입니다.
const useCreateEventData = () => {
const eventNameProps = useSetEventNameStep();
const [eventToken, setEventToken] = useState('');
Copy link
Contributor

Choose a reason for hiding this comment

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

이벤트 토큰은 행사가 생성된 후 백엔드로부터 값을 받아오는 값이니깐 마지막 퍼널에서만 필요할 것 같아요.
즉 여러 상태에 걸쳐 사용되는 데이터는 아닌 것 같아서 다른 곳으로 이동해도 되지 않을까하는 생각입니다.

Comment on lines +3 to +6
type UseFunnel = {
defaultStep: string;
stepList: string[];
};
Copy link
Contributor

Choose a reason for hiding this comment

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

우리가 정의한 step과 여기의 step이 다른 의미라서 조금 헷갈릴 수 있을 것 같아요.

};

type FunnelProps = {
children: React.ReactElement<StepProps>[];
Copy link
Contributor

Choose a reason for hiding this comment

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

Funnel안에 Step밖에 못 오게 한 것 넘 좋아잉

Comment on lines +23 to +25
if (curStepIndex === stepList.length - 1) return;

setStep(stepList[curStepIndex + 1]);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if (curStepIndex === stepList.length - 1) return;
setStep(stepList[curStepIndex + 1]);
setStep(stepList[Math.min(stepList.length - 1, curStepIndex + 1)]));

Comment on lines +31 to +33
if (curStepIndex === 0) return;

setStep(stepList[curStepIndex - 1]);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if (curStepIndex === 0) return;
setStep(stepList[curStepIndex - 1]);
setStep(stepList[Math.max(0, curStepIndex - 1)]));

const targetStep = children.find(curStep => curStep.props.name === step);

if (!targetStep)
throw new Error(`현재 ${step} 단계에 보여줄 컴포넌트가 존재하지 않습니다. Step 컴포넌트를 호출해 사용해주세요.`);
Copy link
Contributor

Choose a reason for hiding this comment

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

역시 웨디 에러까지 꼼꼼히 👍👍

Comment on lines +53 to +58
if (validation.isValid) {
setPassword(newValue);
setErrorMessage('');
} else {
event.target.value = password;
setErrorMessage(validation.errorMessage ?? '');
Copy link
Contributor

Choose a reason for hiding this comment

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

ErrorMessage type이 string | null 이었던 걸로 기억합니다. 그래서 빈 문자열 대신에 null을 넣는 것이 더 좋을 것 같아요! 하지만 중요한 것은 아니라 패쑤해도 괜찮아유

Comment on lines +24 to +30
const handleBack = () => {
if (step === STEP_SEQUENCE[0]) {
navigate('/');
} else {
moveToPrevStep();
}
};
Copy link
Contributor

Choose a reason for hiding this comment

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

뒤로가기 눌렀을 때 바로 다 꺼지지 않고 전 스텝으로 돌리는 디테일 너무좋아요~


return (
<MainLayout backgroundColor="white">
<TopNav>{step !== STEP_SEQUENCE[STEP_SEQUENCE.length - 1] && <Back onClick={handleBack} />}</TopNav>
Copy link
Contributor

Choose a reason for hiding this comment

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

handleBack에서 이미 뒤로가기 조건처리를 해주고 있는데 여기서 한 번 더 해준 이유가 있을까요?

Copy link

Copy link

@pakxe pakxe merged commit 1bfc10e into fe-dev Sep 24, 2024
2 checks passed
@pakxe pakxe deleted the feature/#575 branch September 24, 2024 07:47
@Todari Todari added this to the v2.0.0 milestone Sep 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

3 participants