-
Notifications
You must be signed in to change notification settings - Fork 129
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: Add pathname to prev page events #776
Conversation
Size Change: +629 B (0%) Total Size: 676 kB
ℹ️ View Unchanged
|
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.
Approved with a couple of optional comments
src/page-view.ts
Outdated
export class PageViewManager { | ||
_pageViewData: PageViewData | undefined | ||
_hasSeenPageView = false | ||
|
||
_createPageViewData(): PageViewData { | ||
return {} | ||
return { | ||
pathname: utils.window.location.pathname, |
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.
I assume we need this to query on? Do we perhaps want to just have a full url which allows any kind of grouping filtering later on?
|
@@ -24,15 +25,21 @@ interface ScrollProperties { | |||
$prev_pageview_max_content_percentage?: number | |||
} | |||
|
|||
interface PageViewEventProperties extends ScrollProperties { | |||
$prev_pageview_pathname?: string |
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.
We might want to update isURLNormalizeable()
in the main repo to include this property
b602a53
to
0d75918
Compare
Changes
Follow up from #773, looks like I did need this
Checklist