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

#4050 insights layouts & misc improvements #4306

Merged
merged 8 commits into from
May 12, 2021
Merged

Conversation

paolodamico
Copy link
Contributor

@paolodamico paolodamico commented May 11, 2021

Changes

This PR continues our saga to improve the query experience from #4050; particularly builds up on #4296.

  • Implements custom layouts for retention, sessions & paths (based on [Product & Design] - Revamp insights experience #4050 (comment)).
  • Hides the formula input by default behind an "Add formula" button to follow convention of global filters and breakdown. Further, the button is disabled if the trend graph has only one series (with a prompt to add another series to be able to use formulas).

  • Removes global filters from lifecycle graph. As only one series can be used, filters can be applied locally.
  • Adjusts UI & location of remove button for each graph series.
  • Miscellaneous UX improvements across insights.

Checklist

  • All querysets/queries filter by Organization, by Team, and by User
  • Django backend tests
  • Jest frontend tests
  • Cypress end-to-end tests
  • Migrations are safe to run at scale (e.g. PostHog Cloud) – present proof if not obvious

@paolodamico paolodamico changed the base branch from master to 4050-query-ui-optB-details May 11, 2021 19:36
@timgl timgl temporarily deployed to posthog-pr-4306 May 11, 2021 19:39 Inactive
@timgl timgl temporarily deployed to posthog-pr-4306 May 12, 2021 02:26 Inactive
Base automatically changed from 4050-query-ui-optB-details to master May 12, 2021 02:34
@timgl timgl temporarily deployed to posthog-pr-4306 May 12, 2021 02:44 Inactive
@paolodamico paolodamico marked this pull request as ready for review May 12, 2021 02:44
Copy link
Contributor

@samwinslow samwinslow left a comment

Choose a reason for hiding this comment

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

LGTM!! Really tiny bits of feedback, mostly copy suggestions. 🚢

@samwinslow
Copy link
Contributor

This is outside the scope of this PR but do you think it would be useful to have filters on retention? E.g. if I want to track retention of users on two Pageview events, one on Sign-In Page and the other on Activity Business Value Stuff Page.

Screen Shot 2021-05-12 at 12 03 20 AM

@timgl timgl temporarily deployed to posthog-pr-4306 May 12, 2021 10:24 Inactive
@paolodamico
Copy link
Contributor Author

Thanks for the quick review here @samwinslow! Re retention filters, if I remember correctly, there was a backend limitation for this that required use of actions? I believe it was due to computations on-the-fly timing out in PostHog. CC @EDsCODE

@paolodamico
Copy link
Contributor Author

Merging as failing checks are unrelated.

@paolodamico paolodamico merged commit ebddd25 into master May 12, 2021
@paolodamico paolodamico deleted the 4050-insights-layout branch May 12, 2021 10:38
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