Skip to content
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

chore: session replay project config for mobile - what is possible #26289

Merged
merged 11 commits into from
Nov 20, 2024

Conversation

marandaneto
Copy link
Member

@marandaneto marandaneto commented Nov 19, 2024

Problem

Sometimes customers get confused about whats possible or not yet via the project config, so let's add a hint about what is available or not for Mobile, so they don't waste time testing something that wouldn't work yet.
We already have open issues for those controls and we will eventually implement all of them.

Changes

👉 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?

@marandaneto marandaneto changed the title changes chore: session replay project config for mobile - what is possible Nov 19, 2024
@marandaneto marandaneto requested a review from a team November 19, 2024 16:58
@marandaneto marandaneto marked this pull request as ready for review November 19, 2024 16:58
Copy link
Contributor

github-actions bot commented Nov 19, 2024

Size Change: 0 B

Total Size: 1.16 MB

ℹ️ View Unchanged
Filename Size
frontend/dist/toolbar.js 1.16 MB

compressed-size-action

@posthog-bot
Copy link
Contributor

📸 UI snapshots have been updated

16 snapshot changes in total. 0 added, 16 modified, 0 deleted:

  • chromium: 0 added, 16 modified, 0 deleted (diff for shard 1)
  • webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

Copy link
Contributor

@ioannisj ioannisj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just left a few minor comments. I think this will help a lot!
I'm wondering if it's also worth on working on something similar to https://posthog.com/docs/libraries but with sub-features as well

<p>
Log capture is also available for{' '}
<Link to="https://posthog.com/docs/session-replay/mobile" target="_blank">
Mobile session replay
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think console logs are not available yet on iOS, so should we just list and link to the SDKs that current have support for this?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

true, I don't think we need to go too granular here, but we can if users still get confused with that.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PostHog/posthog-ios#258 just so we dont forget

@pauldambra
Copy link
Member

pauldambra commented Nov 19, 2024

so we have

Feature Web iOS Android Flutter React Native
Console log capture X X X (Android)
Canvas capture (obvs web only) X
Network timing capture X X X X (iOS)
Network header capture X
Network body capture X
Authorized domains X
Sampling X
Minimum duration X
Linked flag X X X X
URL trigger X
URL denylist X
Event trigger X

feels like we could have a nice row of tags alongside the headers showing what's supported giving somewhere for a tooltip or something to highlight minimum versions?

(great idea btw)

@marandaneto
Copy link
Member Author

So we have

Feature Web iOS Android Flutter React Native
Console log capture
Canvas capture (obvs web only) ✔
Network timing capture
Network header capture
Network body capture
Authorized domains
Sampling
Minimum duration
Linked flag
URL trigger
URL denylist
Event trigger
feels like we could have a nice row of tags alongside the headers showing what's supported giving somewhere for a tooltip or something to highlight minimum versions?

(great idea btw)

yep a good idea, but I think it'd bloat the UI even more, maybe this would make more sense in the documentation and the project settings just link to that section.
I'd like to get this PR merged though since its a quick improvement already on what's available and what's not on Mobile.
is that ok?

@pauldambra
Copy link
Member

I'd like to get this PR merged though since its a quick improvement already on what's available and what's not on Mobile.
is that ok?

yep... my guess is people don't actually read this text 🙈 but fine to merge this and come back to it

making the skeleton for the table made me realise I don't know the answer to which boxes should be ticked... might be worth us having this tick table somewhere so we can track the gaps??

@ioannisj
Copy link
Contributor

I'd like to get this PR merged though since its a quick improvement already on what's available and what's not on Mobile.
is that ok?

yep... my guess is people don't actually read this text 🙈 but fine to merge this and come back to it

making the skeleton for the table made me realise I don't know the answer to which boxes should be ticked... might be worth us having this tick table somewhere so we can track the gaps??

Yeah, was going to comment that this matrix will be helpful to us as well, so we can add it somewhere in the documentation maybe

@posthog-bot
Copy link
Contributor

📸 UI snapshots have been updated

16 snapshot changes in total. 0 added, 16 modified, 0 deleted:

  • chromium: 0 added, 16 modified, 0 deleted (diff for shard 1)
  • webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

@posthog-bot
Copy link
Contributor

📸 UI snapshots have been updated

16 snapshot changes in total. 0 added, 16 modified, 0 deleted:

  • chromium: 0 added, 16 modified, 0 deleted (diff for shard 1)
  • webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

@marandaneto
Copy link
Member Author

I filled the table with what I know by heart, should be correct but feel free to double check

@marandaneto marandaneto enabled auto-merge (squash) November 20, 2024 12:34
@marandaneto marandaneto merged commit bc98fda into master Nov 20, 2024
96 checks passed
@marandaneto marandaneto deleted the fix/no-mobile-support-yet branch November 20, 2024 13:31
fuziontech added a commit that referenced this pull request Nov 21, 2024
* master: (66 commits)
  feat: display line context in stack frames (#26254)
  fix(experiments HogQL): fix funnels filter in prepare_query (#26327)
  fix(hogql): Support parsing dw properties with nested values (#26321)
  fix(err): include raw js frame on frame content in pg (#26326)
  fix: mobile project settings network labels (#26318)
  fix: reduce perms from 3 to 2 for test perf (#26322)
  feat: add new alerts feature for product_analytics (#26320)
  feat(experiments): Enable holdouts for everyone (#26301)
  feat: reorg inspector list rows (#26243)
  feat: Split-up batch exports into sync and async (#26294)
  fix(data-warehouse): Fix custom series colors in chart tooltips (#26310)
  feat(cdp): adjust zapier destination description (#26313)
  fix(insights): prevent race condition (#26265)
  feat(product-assistant): evaluation pipeline (#26179)
  chore: session replay project config for mobile - what is possible (#26289)
  feat(cdp): make hash functions return null if the input is null (#26311)
  feat(err): cymbal for all teams (#26312)
  feat(hogql): Allow breakdowns for lazy-joined tables (#26302)
  feat: create events table to store last 7 days of data (#26239)
  chore: Bump batch exports start jitter (#26278)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants