-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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
Nv 3352 - Onboarding Analytics #5212
Nv 3352 - Onboarding Analytics #5212
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the work! Please see note about the icon before merging
@@ -92,6 +94,7 @@ export function SideNav({}: Props) { | |||
}, [environment, addItem, removeItems, setEnvironment]); | |||
|
|||
const handleHideOnboardingClick = async () => { | |||
segment.track('Click Hide Get Started Page - [Get Started]'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💭 thought: I've seen us primarily use [<area of application>] - <description of event>
in other places -- should we follow that ordering?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it's a good question, the main agenda for doing it like description - [area]
is if we add the area
first we will lose the description
while searching in our analytic tool. As a user, if I am searching for the template editor
events I would prefer to see more of the description
and not the area
that I am searching for.
wdyt?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm I'm a little torn on this since I think it's important that we stay consistent one way or another. Also, perhaps there is a better way to track these prefixes /tags
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It should be in the format xxx - [yyy]
, that was the decision we took some time ago. All other places were added before and we didn't want to "lose" some data because of that change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This smells of something that should have been refactored when when that decision was made 😅
Ticket here, please add your thoughts: https://linear.app/novu/issue/NV-3529/centralize-segment-tracking-standards
display: flex; | ||
align-items: center; | ||
justify-content: flex-start; | ||
margin-top: 24px; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
margin-top: 24px; |
Most components should leave margin to their parent / caller for layout
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you elaborate a bit not sure i understand correctly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since margin adds additional spacing between spacing elements, it's generally preferable to let the parent component add it:
...Rest of the layout...
<AdditionalInformationLink ...other props... mt={24}>...</AdditionalInformationLink>
...
<OtherComponents />
This way, components are more reusable
apps/web/src/pages/get-started/components/get-started-tabs/useGetStartedTabs.ts
Show resolved
Hide resolved
apps/web/src/pages/get-started/consts/TranslationUseCase.const.tsx
Outdated
Show resolved
Hide resolved
052c28e
to
6c95835
Compare
What change does this PR introduce?
*Who dismissed the onboarding page?
Those are the events that we added in this task
Click Hide Get Started Page - [Get Started]
Click Use-case Tab - [Get Started] props: { tab }
Link Click - [Get Started] props: { href } examples: integrations store, activity feed, docs…
The following events was added in 3510 #5207
Click Configure Use-Case Node [Get Started] props { templateIdentifier, node }
Click Create Workflow [Get Started] props { templateIdentifier }