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

Remove shownas filter and move stickiness/lifecycle into separate insight tabs #2899

Merged
merged 12 commits into from
Jan 12, 2021

Conversation

EDsCODE
Copy link
Member

@EDsCODE EDsCODE commented Jan 8, 2021

Changes

Adhoc changes to separate stickiness and lifecycle into separate insight tabs. Logic will be reorganized if data shows better usage and we commit to making this change

Please describe.

  • places this separation behind feature flag remove-shownas to test usage

If this affects the frontend, include screenshots.

Checklist

  • All querysets/queries filter by Organization, by Team, and by User
  • Django backend tests
  • Jest frontend tests
  • Cypress end-to-end tests

@timgl timgl temporarily deployed to posthog-remove-shownas-uhh60it January 8, 2021 15:45 Inactive
@EDsCODE EDsCODE temporarily deployed to posthog-remove-shownas-uhh60it January 8, 2021 18:18 Inactive
@EDsCODE EDsCODE temporarily deployed to posthog-remove-shownas-uhh60it January 8, 2021 18:19 Inactive
@EDsCODE EDsCODE temporarily deployed to posthog-remove-shownas-uhh60it January 8, 2021 18:58 Inactive
@EDsCODE EDsCODE mentioned this pull request Jan 8, 2021
21 tasks
@EDsCODE EDsCODE marked this pull request as ready for review January 8, 2021 20:09
Copy link
Contributor

@paolodamico paolodamico left a comment

Choose a reason for hiding this comment

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

Great idea here! Definitely an improvement on the discoverability of these features, and is definitely more intuitive.

Almost everything is 100%, a few points after the QA:

  • Found an edge case in which after switching between any trends tab and either the "Stickiness" or "Lifecycle" tab, it would no longer let you change the selected event (see recording below with the error and dropdown not changing).
  • On the lifecycle view, I can remove the first event and then I'm allowed to add multiple events/actions (see below for suggestion on UX improvement).

Some potential improvements:

  • I would use this opportunity to reorder the insights a bit, doing Trends & Funnels first (most used), and adding these two new spin-offs to the end (that way I believe it's even more clear that you now can do that stuff).
  • I would consider adding a "NEW" badge to the lifecycle insights to increase discoverability for this new feature. The badge should only appear until you use this for the first time.
  • As you hide the math selector for the lifecycle view, maybe we should keep the select box disabled showing "Unique users"? Thinking that could make it even clearer.
    • Related to the above I would remove the "Add graph series" button altogether in the lifecycle graph. I would also not allow the user to remove the one (only) event/action.

P.S. Great stuff A/B testing this! Will merge #2787 soon to be able to measure the results accurately.

@EDsCODE EDsCODE temporarily deployed to posthog-remove-shownas-uhh60it January 11, 2021 17:39 Inactive
@EDsCODE EDsCODE temporarily deployed to posthog-remove-shownas-uhh60it January 11, 2021 18:39 Inactive
@EDsCODE
Copy link
Member Author

EDsCODE commented Jan 12, 2021

@paolodamico ready for another look. I did everything except the new badge and locking the math selector on lifecycle. The new badge is cool but I think it's better in a follow up PR so if we want it, we can hold off an initiating feature flag for this until i finish the new badge. I don't think we should leave in a disabled math selector because I tried it and it just looks more confusing why you can't select other options. I'd suggest we work on making the description for lifecycle abundantly clear

@EDsCODE EDsCODE temporarily deployed to posthog-remove-shownas-uhh60it January 12, 2021 14:37 Inactive
@timgl timgl temporarily deployed to posthog-remove-shownas-uhh60it January 12, 2021 18:02 Inactive
Copy link
Contributor

@paolodamico paolodamico left a comment

Choose a reason for hiding this comment

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

lgtm! everything seems addressed!

@paolodamico paolodamico merged commit 650bc42 into master Jan 12, 2021
@paolodamico paolodamico deleted the remove-shownas branch January 12, 2021 18:12
@paolodamico paolodamico mentioned this pull request Jan 12, 2021
4 tasks
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.

3 participants