-
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(web-analytics): Sync web analytics state with url #19400
Conversation
Size Change: 0 B Total Size: 1.99 MB ℹ️ 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.
👍
actionToUrl(({ values }) => ({ | ||
setWebAnalyticsFilters: () => stateToUrl(values), | ||
togglePropertyFilter: () => stateToUrl(values), | ||
setDates: () => stateToUrl(values), | ||
setInterval: () => stateToUrl(values), | ||
setDeviceTab: () => stateToUrl(values), | ||
setSourceTab: () => stateToUrl(values), | ||
setGraphsTab: () => stateToUrl(values), | ||
setPathTab: () => stateToUrl(values), | ||
setGeographyTab: () => stateToUrl(values), | ||
})), |
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.
Here's a patter you can use if you want to clean this up a bit, yet keep the url logic contained:
actionToUrl(({ values }) => ({ | |
setWebAnalyticsFilters: () => stateToUrl(values), | |
togglePropertyFilter: () => stateToUrl(values), | |
setDates: () => stateToUrl(values), | |
setInterval: () => stateToUrl(values), | |
setDeviceTab: () => stateToUrl(values), | |
setSourceTab: () => stateToUrl(values), | |
setGraphsTab: () => stateToUrl(values), | |
setPathTab: () => stateToUrl(values), | |
setGeographyTab: () => stateToUrl(values), | |
})), | |
actionToUrl(({ values }) => { | |
function stateToUrl() { | |
const urlParams = new URLSearchParams() | |
// ... | |
} | |
return { | |
setWebAnalyticsFilters: stateToUrl, | |
togglePropertyFilter: stateToUrl, | |
setDates: stateToUrl, | |
setInterval: stateToUrl, | |
setDeviceTab: stateToUrl, | |
setSourceTab: stateToUrl, | |
setGraphsTab: stateToUrl, | |
setPathTab: stateToUrl, | |
setGeographyTab: stateToUrl, | |
} | |
})), |
This is just a nit at this point, definitely not blocking.
Problem
Some feedback asking a few related things:
An example zendesk ticket: https://posthoghelp.zendesk.com/agent/tickets/8277
Changes
Store relevant parts of the state in the url.
How did you test this code?
Manually, spending a lot of time making sure that the back button behaviour is correct. A few edge cases: