Conversation
|
MATEBALL-STORYBOOK |
Deploying mateball-client with
|
| Latest commit: |
19251bf
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://b453b8b9.mateball-client.pages.dev |
| Branch Preview URL: | https://feat--85-use-funnel.mateball-client.pages.dev |
heesunee
left a comment
There was a problem hiding this comment.
고생많으셨습니다
| children: ReactNode; | ||
| } | ||
|
|
||
| export const useFunnel = <T extends readonly string[]>(steps: T, completePath: string) => { |
There was a problem hiding this comment.
원래 퍼널을 사용했을때 step=1, 2, 3... 이런식으로 표기되는 퍼널을 사용했었는데 step 이름이 파라미터에 들어가니까 더 직관적이고 좋은 것 같아요 !
|
|
||
| const stepFromUrl = searchParams.get('step'); | ||
| const isValidStep = stepFromUrl && steps.includes(stepFromUrl as T[number]); | ||
| const currentIndex = isValidStep ? steps.indexOf(stepFromUrl as T[number]) : 0; |
There was a problem hiding this comment.
예를들어 nickname, birth 라는 두 스텝이 있을때 step=age 같이 steps에 포함되지 않는 단계가 들어오면 다시 퍼널의 첫단계인 nickname으로 이동하는건가요?
There was a problem hiding this comment.
네!! 다만 nickname 화면이 렌더링되어도 url은 여전히 ?step=age로 남아있어요...
그상태에서 다음 버튼을 누르면 내부적으로는 currentIndex가 0이기 때문에 ?step=birth로 넘어가긴 하는데,
- url과 맞지 않은 페이지가 렌더링되어 사용자가 혼란스러울 수 있고
- 뒤로가기 시
?step=age가 히스토리에 남아 이상한 페이지로 갈 수 있다는 문제가 있을 것 같네용..
강제로 setSearchParams({ step: steps[0] })로 이동시켜서 ui와 일치하는 url을 렌더링하도록 할까요??
There was a problem hiding this comment.
url과 맞지 않은 페이지가 렌더링되어 사용자가 혼란스러울 수 있고
뒤로가기 시 ?step=age가 히스토리에 남아 이상한 페이지로 갈 수 있다는 문제가 있을 것 같네용..
말씀하신 현상 모두 사용자 입장에서 혼란이 올 수 있겠네요 😅
이런 경우에.. 몇 가지 방법을 생각해보자면
- 잘못된 step일 경우 navigate 시켜서 첫 단계 step으로 강제 이동시키고, replace 시켜서 뒤로가기를 눌러도 잘못된 url로 가지 않도록 해 처리한다.
- 말씀하신 것처럼
setSearchParams({ step })URL만 보정하는 방식, 이 경우엔 히스토리에 남아서 다시 잘못된 step으로 가는 이슈는 남을 것 같지만, 현재 가장 깔끔한 방법인 것 같기도 해요 - 아니면 아예 처음 진입시 step에 대한 유효성검사를 useEffect안에서 해두고, 잘못된 경우엔 첫 단계로 리디렉션 처리되거나 error 페이지로 이동시켜보는 방식도 있을 것 같아요
There was a problem hiding this comment.
근데 생각해보니 useEffect로 stepFromurl이 바뀔때마다 검사해도 결국 navigate를 사용하는 거고, 차이점은 단지 "유효하지 않을 때만 이동"이라는 조건을 useEffect 안에서 처리한다는 것뿐이긴하네요.
if (!isValidStep) {
navigate(`${pathname}?step=${steps[0]}`, { replace: true });
return null; 렌더링 전에 바로 navigate로 이동시킬지, 아니면 히스토리는 남기되 setSearchParams로 상태를 유지할지 고민이 되네요.... 혹시 채은님은 다른 방식도 고려해보셨을까요?
There was a problem hiding this comment.
말씀해주신 세가지 방법 모두 충분히 고려해볼 수 있을 것 같고, 각각 장단점이 분명히 있는 것 같아요...
저도 처음엔 navigate로 바로 이동시키는 방식도 생각했었는데, 렌더링 중 side effect가 발생한다는 점이 조금 걸려서요..!
개인적으로는 useEffect 안에서 isValidStep 여부를 체크한 뒤, 잘못된 step일 경우엔 첫 step으로 setSearchParams를 통해 처리하는 방식을 우선 고려했어요
useEffect(() => {
if (!isValidStep) {
setSearchParams({ step: steps[0] }, { replace: true });
}
}, [isValidStep, setSearchParams, steps]);다만 이 방식은 초기 렌더 후 한 프레임 정도 blank 상태가 생길 수 있다는 게 아쉬운점이긴 해요ㅜ
아예 에러 페이지로 명확히 분기하는 것도 나쁘지 않을 것 같은데, 다른 분들 의견도 궁금합니다..! @Dubabbi @yeeeww
There was a problem hiding this comment.
현재 로직에서 step=age처럼 유효하지 않은 step 값이 들어오면 첫 단계 화면(nickname)은 렌더링되지만, URL은 여전히 ?step=age 상태로 남아 있어서 사용자 입장에서 혼란이 있을 수 있을 것 같아요. 예를 들어보자면 화면과 주소가 일치하지 않아 '내가 지금 어떤 단계에 있는지' 직관적으로 파악하기 어렵고, 뒤로가기 시 잘못된 URL이 히스토리에 남아 이상한 화면으로 되돌아가는 문제도 발생할 수 있을 것 같습니다...
이런 경우에는 채은 님이 마지막에 언급하신 방안과 유사하게 아래 방식으로 처리하는 것이 UX 측면에서 더 안전하지 않을까 제안해 봅니다!!
useEffect(() => {
if (!isValidStep) {
navigate(`${pathname}?step=${steps[0]}`, { replace: true });
}
}, [isValidStep, navigate, pathname, steps]);
if (!isValidStep) return null;navigate에 replace: true를 사용하면 뒤로가기 시 잘못된 step으로 되돌아가지 않게 브라우저 히스토리까지 정리할 수 있고 마지막에 return null로 일시적으로 렌더링을 막아줘서 잘못된 화면이 보이지 않도록 할 수 있기 때문에 사용자 혼란을 최소화할 수 있을 것 같아요. 물론 이것도 완전히 장점만 있는 방식은 아니지만, 전체적인 사용자 흐름을 생각했을 때 가장 자연스럽고 안전한 처리 방식이라고 생각합니다~!
There was a problem hiding this comment.
저도 히스토리가 남긴 하지만 setSearchParams로 url을 보정하는 방식이 좋을 것 같다고 생각합니다! 에러페이지로 처리하면 퍼널 흐름이 끊길 수 있다고 생각하고, navigate로 이동한다면 말씀해주신 것처럼 side effect 발생 가능성이 있을 것 같아서요..!
There was a problem hiding this comment.
에러 페이지로 명확히 분기하는 것도 나쁘지 않을 것 같은데, 다른 분들 의견도 궁금합니다..!
개인적으로는 상황을 명확히 구분해 에러페이지로 분기하는 방식이 더 적절하다고 판단됩니다! 저희 서비스가 모바일 웹 환경이라는 점을 고려할 때, 정상적인 클릭 경로를 통해 접근했을때도 url에 오류가 일어났다면 이는 명백한 에러 상황입니다.
반면 주소를 임의 조작하여 비정상적인 접근을 유도한 경우라면, 그 또한 오류에 해당하므로 별도로 퍼널 첫 단계로 유도할 필요는 없다고 생각합니다.
초기 단계로 되돌리는 대신 명확한 에러 페이지로 사용자에게 현재 상황을 인지시키는 편이 사용자 경험 측면, 서비스 안정성과 신뢰도 측면에서도 바람직하다고 생각합니다.
There was a problem hiding this comment.
희선님에게 설득당했습니다... 잘못된 step일 경우 error 페이지로 리디렉션되도록 처리해두었어요!
다양한 관점에서 의견 나눠주셔서 감사해요 👍
src/shared/hooks/use-funnel.tsx
Outdated
| const Funnel = useMemo(() => { | ||
| return ({ children }: FunnelProps) => { | ||
| const childrenArray = Children.toArray(children) as ReactElement<{ name: string }>[]; | ||
| const matched = childrenArray.find((child) => child.props.name === currentStep); | ||
| return <>{matched}</>; | ||
| }; | ||
| }, [currentStep]); |
There was a problem hiding this comment.
여기 Funnel을 useMemo로 감싼 이유가 궁금합니다! currentStep이 바뀔때만 새 컴포넌트를 생성하게 되는데, 그냥 매 렌더링마다 함수가 생성되어도 큰 부담이 없을 거라 생각했었습니다
There was a problem hiding this comment.
맞네요..ㅎ currentStep이 변경될 때만 컴포넌트 함수가 재정의되도록 하려고 useMemo로 감쌌는데, jsx에서 <Funnel>로 사용되는 구조상 컴포넌트 자체가 렌더링되기 때문에 메모이제이션의 실질적인 이점은 거의 없는 것 같습니다..😅
일반 함수형 컴포넌트로 수정했습니다 !!
| const currentIndex = isValidStep ? steps.indexOf(stepFromUrl as T[number]) : 0; | ||
| const currentStep = steps[currentIndex]; | ||
|
|
||
| const goTo = useCallback( |
There was a problem hiding this comment.
useCallback으로 감싸서 참조 고정하는 거 굿이네용 ㅎㅎ
Dubabbi
left a comment
There was a problem hiding this comment.
use-funnel 훅 구현해 주신 부분 확인했습니다!! 고생하셨습니다~ 최고 🎸
|
|
||
| const stepFromUrl = searchParams.get('step'); | ||
| const isValidStep = stepFromUrl && steps.includes(stepFromUrl as T[number]); | ||
| const currentIndex = isValidStep ? steps.indexOf(stepFromUrl as T[number]) : 0; |
There was a problem hiding this comment.
저도 히스토리가 남긴 하지만 setSearchParams로 url을 보정하는 방식이 좋을 것 같다고 생각합니다! 에러페이지로 처리하면 퍼널 흐름이 끊길 수 있다고 생각하고, navigate로 이동한다면 말씀해주신 것처럼 side effect 발생 가능성이 있을 것 같아서요..!
|
@bongtta 채은님, 코드 다시 확인해보니 최초 퍼널 진입 시 step 쿼리 파라미터가 없으면 자동으로 첫 단계(steps[0])로 이동시키는 로직이 없지 않나요? |
그러네요..! 말씀해주신 내용 반영해서 새 브랜치에서 수정했습니다. |
#️⃣ Related Issue
Closes #85
☀️ New-insight
커스텀 훅 내부에 렌더링 책임이 있는
Funnel,Step컴포넌트를 함께 선언했다.useMemo를 활용하여Funnel컴포넌트를 메모이제이션함으로써 불필요한 재생성을 방지할 수 있었다.url 쿼리스트링 기반으로 퍼널 흐름을 제어했다.
💎 PR Point
❶
useFunnel(steps, completePath)steps: 퍼널 단계 문자열 배열 (ex.['intro', 'info', 'confirm'])completePath: 마지막 스텝에서goNext호출 시 이동할 경로❷ 내부 상태 관리
stepFromUrl: 쿼리 파라미터에서 현재 스텝 파악currentIndex,currentStep계산goTo,goNext,goPrev등 단계 이동 함수 제공❸ 내부 컴포넌트 포함
Funnel: 현재 step에 해당하는Step만 렌더링Step: 스텝 정의용 컴포넌트 래퍼❹ 방어 코드 포함
stepFromUrl이 유효하지 않은 경우 첫 스텝으로 fallbackgoTo에서 유효하지 않은 스텝 입력 무시❺ 사용방법:
Summary by CodeRabbit