feat(funnels): funnel stepper #4345
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Skipped Deployment
|
| <QueryClientProvider client={queryClient}> | ||
| <ProgressiveEnhancementContextProvider> | ||
| <AppSubscriptionContextProvider> | ||
| {children} | ||
| </AppSubscriptionContextProvider> | ||
| </ProgressiveEnhancementContextProvider> | ||
| </QueryClientProvider> |
There was a problem hiding this comment.
We need the LogContext
There was a problem hiding this comment.
Where? because it's already inside one of these no?
Afraid this might give some edge-case issue where context wrong order now.
packages/shared/src/features/onboarding/shared/FunnelStepper.tsx
Outdated
Show resolved
Hide resolved
packages/shared/src/features/onboarding/shared/FunnelStepper.tsx
Outdated
Show resolved
Hide resolved
…into MI-851-funnel-stepper
…into MI-851-funnel-stepper
| @@ -72,8 +72,9 @@ const FormInputCheckbox = ({ | |||
| aria-checked={isSelected} | |||
| aria-label={item.label} | |||
| className={classNames(isVertical ? 'typo-subhead' : 'typo-body')} | |||
| pressed={isSelected} | |||
| data-track-click | |||
There was a problem hiding this comment.
What is this actually? not familiar with it.
There was a problem hiding this comment.
I've added a onClickCapture on stepper level, that send events on each element that has this data-track-click
| const handleScroll = () => { | ||
| setScrollPosition(window.scrollY); | ||
| onScroll?.(); | ||
| }; |
There was a problem hiding this comment.
Maybe if this is useCallback you don't need the exhaustive deps below?
| const searchParams = useSearchParams(); | ||
| const pathname = usePathname(); | ||
| const [stepTimerStart, setStepTimerStart] = useState<number>(0); | ||
| const [position, setPosition] = useAtom(funnelPositionAtom); |
There was a problem hiding this comment.
I guess we need atoms to reshare between things here?
Don't we already have exposes context for funnels specifically?
| cx="98" | ||
| cy="98" | ||
| r="90" | ||
| className="stroke-overlay-primary-pepper" |
There was a problem hiding this comment.
This is the right color it's solid non themed one?
There was a problem hiding this comment.
Don't know, this comes from @AmarTrebinjac component, just removed the bg wrapper
| <QueryClientProvider client={queryClient}> | ||
| <ProgressiveEnhancementContextProvider> | ||
| <AppSubscriptionContextProvider> | ||
| {children} | ||
| </AppSubscriptionContextProvider> | ||
| </ProgressiveEnhancementContextProvider> | ||
| </QueryClientProvider> |
There was a problem hiding this comment.
Where? because it's already inside one of these no?
Afraid this might give some edge-case issue where context wrong order now.
| y: number; | ||
| } | ||
|
|
||
| export const useWindowScroll = ( |
There was a problem hiding this comment.
I'd do this hook differently. I'd create a ref for the callback and this callback would call the onScroll with the window.scrollY value. We don't really need state
| const [position, setPosition] = useAtom(funnelPositionAtom); | ||
| const [history, dispatchHistory] = useAtom(funnelPositionHistoryAtom); | ||
|
|
||
| const chapters: Chapters = useMemo( |
There was a problem hiding this comment.
Not sure that this is how we do the chapters on the backend. we'll need to check
There was a problem hiding this comment.
This is how the Header component's props is shaped, just passing to it
| globalThis?.removeEventListener?.('scroll', handleScroll); | ||
| }; | ||
| }, | ||
| // eslint-disable-next-line react-hooks/exhaustive-deps |
| type: FunnelStepTransitionType; | ||
| }) => void; | ||
|
|
||
| interface USeFunnelTrackingProps { |
There was a problem hiding this comment.
Double capitalization
| return map; | ||
| }, [funnel]); | ||
|
|
||
| const step: NonChapterStep = useMemo( |
There was a problem hiding this comment.
Each step in funnel can be chapter or actual step. This is for every non chapter one
| const from = step.id; | ||
| const timeDuration = Date.now() - stepTimerStart; | ||
|
|
||
| if (!stepMap[to]?.position) { |
There was a problem hiding this comment.
Does it mean we finished the funnel?
There was a problem hiding this comment.
Or that the editor have put a non existing step as target 👀
| @@ -0,0 +1,177 @@ | |||
| import { useMemo, useState, useEffect, useCallback } from 'react'; | |||
There was a problem hiding this comment.
We need some more comments here to make it maintainable or breakdown to smaller pieces even within the same file
| details: { | ||
| target_type: | ||
| trackedElement.tagName.toLowerCase() as keyof HTMLElementTagNameMap, | ||
| ...(trackedElement.id ? { target_id: trackedElement.id } : {}), |
There was a problem hiding this comment.
Since we gonna preload all steps, we should consider maybe we need to send something else other than id. Because then we'll have duplicate ids
There was a problem hiding this comment.
Should we track if a step has been interacted with twice? Maybe just add a check for already completed steps?
| }); | ||
|
|
||
| const trackOnHoverCapture: TrackOnMouseCapture = trackOnMouseCapture({ | ||
| selector: '[data-track-hover]', |
There was a problem hiding this comment.
i think every element that should be tracked for hover will be tracked for click. should we track the same selector for both?
| details: { | ||
| target_type: event.type, | ||
| target_id: event.to, | ||
| event_duration: event.timeDuration, |
There was a problem hiding this comment.
This was supposed to be outside of extra, but i'll update the excel
| event_duration: event.timeDuration, | |
| duration: event.timeDuration, |
| // todo: implement resume funnel event | ||
|
|
||
| return () => { | ||
| trackFunnelEvent({ name: FunnelEventName.LeaveFunnel }); |
There was a problem hiding this comment.
This should be triggered only when the user didn't finish the funnel
| @@ -32,12 +31,39 @@ const variantToClassName: Record<FunnelBackgroundVariant, string> = { | |||
| [FunnelBackgroundVariant.Hourglass]: 'bg-gradient-funnel-hourglass', | |||
| }; | |||
|
|
|||
| const getVariantFromStep = (step: NonChapterStep): FunnelBackgroundVariant => { | |||
| if (!step) { | |||
There was a problem hiding this comment.
When are we not on a step ?
| {funnel.steps.map((chapter: FunnelStepChapter) => ( | ||
| <Fragment key={chapter?.id}> | ||
| {chapter?.steps?.map((funnelStep: NonChapterStep) => ( | ||
| <div |
Changes
Events
Did you introduce any new tracking events?
Experiment
Did you introduce any new experiments?
Manual Testing
Caution
Please make sure existing components are not breaking/affected by this PR
Jira ticket
MI-851
Preview domain
https://mi-851-funnel-stepper.preview.app.daily.dev