fix: remove geolocation from session replay onboarding #22341
Conversation
|
Size Change: 0 B Total Size: 1.03 MB ℹ️ View Unchanged
|
| } | ||
| type PluginContentMapping = Record<string, PluginContent> | ||
| const pluginContentMapping: PluginContentMapping = { | ||
| GeoIP: { | ||
| title: 'Capture location information', | ||
| description: | ||
| 'Enrich PostHog events and persons with IP location data. This is useful for understanding where your users are coming from. This setting can be found under the data pipelines apps.', | ||
| productOnboardingDenyList: [ProductKey.SESSION_REPLAY], |
There was a problem hiding this comment.
This seems backwards to me, I feel like it should maybe be an allowlist.
There was a problem hiding this comment.
Do we not want the setting on all pages except those listed?
This felt clearer because we're saying "don't show only this one" over "do show on these other ones" and then you have to figure out how many it won't be shown on
No strong feeling here, since you all know this area better than me :)
There was a problem hiding this comment.
will merge it since it's green, @raquelmsmith happy to change to an allowlist if you have strong opinions.
but I agree with @pauldambra here, it's easier to add to a deny list rather than to an allowlist for all the new features (looking at less maintenance)
There was a problem hiding this comment.
Sorry I forgot about this!
I think for this specific instance the denylist makes sense. But as we move forward with other products that aren't so tied to product analytics (eg data warehouse) we'll have to remember to add those to the denylist. An allowlist is a better construct imo. But, we can change it in the future if / when it starts to get annoying. Thanks for getting this in!
📸 UI snapshots have been updated2 snapshot changes in total. 0 added, 2 modified, 0 deleted:
Triggered by this commit. |
I ran through onboarding and mentioning geo-location during session replay onboarding was confusing
There's not geo-location for session replay, it only applies to event ingestion. So this is setting incorrect expectations for folk who are only going to use session replay
## other onboarding pages still list it
## session replay onboarding page does not