-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
feat(onboarding-templates): allow someone to select pageview as variable #24899
feat(onboarding-templates): allow someone to select pageview as variable #24899
Conversation
Oh, I need to rebase this on master once #24825 goes out. It's not actually 700 lines! |
Size Change: +475 B (+0.04%) Total Size: 1.1 MB ℹ️ View Unchanged
|
ef0bc19
to
206551a
Compare
📸 UI snapshots have been updated1 snapshot changes in total. 0 added, 1 modified, 0 deleted:
Triggered by this commit. |
📸 UI snapshots have been updated1 snapshot changes in total. 0 added, 1 modified, 0 deleted:
Triggered by this commit. |
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.
👏
@@ -255,6 +279,9 @@ export const iframedToolbarBrowserLogic = kea<iframedToolbarBrowserLogicType>([ | |||
actions.setNewActionName(null) | |||
actions.disableElementSelector() | |||
return | |||
case PostHogAppToolbarEvent.PH_TOOLBAR_NAVIGATED: | |||
// remove leading / from path | |||
return actions.setCurrentPath(payload.path.replace(/^\/+/, '')) |
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 function returns void so technically you should be doing something like this:
return actions.setCurrentPath(payload.path.replace(/^\/+/, '')) | |
actions.setCurrentPath(payload.path.replace(/^\/+/, '')) | |
return void |
Problem
For #23065
A common event in dashboards/insights is a pageview. Eg signup page viewed -> clicked signup button.
Additionally, sometimes these events happen on different domains - eg posthog.com and us.posthog.com.
Changes
Adds a url navigator, constrained to URLs that have seen ingested events. Shows current iframe URL in the URL input. Lets someone use a page URL as the template variable.
Screen.Recording.2024-09-10.at.4.35.29.PM.mov
👉 Stay up-to-date with PostHog coding conventions for a smoother review.
Does this work well for both Cloud and self-hosted?
How did you test this code?