PostHog / posthog

🦔 PostHog provides open-source web & product analytics, session recording, feature flagging and A/B testing that you can self-host. Get started - free.
https://posthog.com
Other
22.3k stars 1.35k forks source link

fix: removing session replay button from feature flag create page #26407

Closed surbhi-posthog closed 3 days ago

surbhi-posthog commented 3 days ago

Removing the "View recordings" button from the Create Feature Flag page and making sure it still exists in the edit/view page

I created a branch that I deprecated and started a new one since I needed to revert too many files. Here is the original PR for reference: https://github.com/PostHog/posthog/pull/26364

Changes

Before After After (multi-variant create view)
Before Image After Image image

Verified that the View recordings button is still visible in the view/edit pages:

Unimpacted views

View Page Edit Page
View Page Edit Page

👉 Stay up-to-date with PostHog coding conventions for a smoother review.

Does this work well for both Cloud and self-hosted?

Cloud

How did you test this code?

Verified that button is not seen in the view page, but can still be access in the edit/view pages. tested locally on: http://localhost:8000/project/2/feature_flags/new

posthog-bot commented 3 days ago

📸 UI snapshots have been updated

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

Triggered by this commit.

👉 Review this PR's diff of snapshots.

surbhi-posthog commented 3 days ago

@zlwaterfield @raquelmsmith I noticed that the View recording button is implemented in another place in the same file:

I'm assuming this was built for the multi variant view. I verified that the button does not show up while creating a flag with multi variants.

Should we also try to consolidate button logic into one component? Or does it make sense to keep in separate here?

github-actions[bot] commented 3 days ago

Size Change: 0 B

Total Size: 1.16 MB

ℹ️ View Unchanged | Filename | Size | | :--- | :---: | | `frontend/dist/toolbar.js` | 1.16 MB |

compressed-size-action

neilkakkar commented 3 days ago

100% optional flyby, I think it makes more sense to re-use the readonly parameter here - recordings button should show only when read only, vs checking the id is not new. It doesn't make too much sense for this button to exist on the edit page either - at the stage its not something editable and clutters the edit screen 😅

zlwaterfield commented 2 days ago

@neilkakkar you're 100% correct, and that was part of the task but I missed it when reviewing. @surbhi-posthog can you make an update to make sure it doesn't in create or edit?

surbhi-posthog commented 2 days ago

Sure thing! I initially left the button on the edit page, thinking it might be useful for users to check session relays while editing the feature flag. That said, I trust your judgment on use behaviors, so I’ll go ahead and make the change.

surbhi-posthog commented 2 days ago

updated logic to use the readOnly flag instead here: https://github.com/PostHog/posthog/pull/26451