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

Dashboard insight setup #4308

Merged
merged 5 commits into from
May 12, 2021
Merged

Dashboard insight setup #4308

merged 5 commits into from
May 12, 2021

Conversation

liyiy
Copy link
Contributor

@liyiy liyiy commented May 11, 2021

Changes

Round 2 of dashboard insight page setup 😄 for #3551

  1. Set up separate component and logic files for this page. The route is accessible only if you manually go to /dashboard_insight/${id}

  2. Moved over the dashboard item description editing stuff I had worked on

TODO for a separate, next PR

  • Address the actual filters handling !
  • Address the look and size of the graphs
  • Address more detailed description/title editing
  • Address somewhat duplicate code with the mapping like how the DashboardItem component renders its own charts
  • Include a e2e test or two 😓

Overall Page View
Screen Shot 2021-05-11 at 4 20 53 PM

Description editing
https://user-images.githubusercontent.com/25164963/117879267-c3735c80-b274-11eb-999f-c8fb2a42049a.mov

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

@timgl timgl temporarily deployed to posthog-pr-4308 May 11, 2021 20:21 Inactive
@liyiy liyiy requested a review from EDsCODE May 11, 2021 20:30
@timgl timgl temporarily deployed to posthog-pr-4308 May 11, 2021 20:56 Inactive
@timgl timgl temporarily deployed to posthog-pr-4308 May 11, 2021 21:29 Inactive
@paolodamico
Copy link
Contributor

Hey @liyiy! With this PR it'll be important to consider the changes from #4314, as the new querying experience will require a different approach to the UI. Let me know if you want to sync on this.

@liyiy
Copy link
Contributor Author

liyiy commented May 12, 2021

Hey @liyiy! With this PR it'll be important to consider the changes from #4314, as the new querying experience will require a different approach to the UI. Let me know if you want to sync on this.

Yes, let's sync on that.

@liyiy liyiy merged commit 91b6b5f into master May 12, 2021
@liyiy liyiy deleted the dashboard-insight-setup branch May 12, 2021 16:18
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