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

703 multiple dashboards #740

Merged
merged 62 commits into from
May 11, 2020
Merged

703 multiple dashboards #740

merged 62 commits into from
May 11, 2020

Conversation

mariusandra
Copy link
Collaborator

@mariusandra mariusandra commented May 11, 2020

Changes

Fixes #703 .

This turned into a rather big PR again. It adds support for multiple dashboards and improves a bunch of other stuff:

image

Other things added:

  • Improved the visual style of the dashboards
  • Pinning dashboards
  • Show all pinned dashboards in the sidebar, open the first one if there are any when clicking on the main "dashboards" entry
  • Darker sidebar and light gray backgrounds for dashboard, trends and funnels scenes
  • Use ellipsis ("...") when names in the dashboard are too long
  • Rename dashboard items
  • A new prompt logic that lets you ask window.prompt style questions easily from within a logic, including support for closing the prompt modal when the logic that asks the question is unmounted (e.g. you click "new dashboard" and then click back in the browser and go to another page)
  • Now in case any loader loads for more than 0.5 sec, we also display a line on top of the page with nprogress, to give better indication to the user that something is happening. This gives a bit of feedback during long running queries when there is no obvious "loading" spinner on the page.
  • All the API methods now return the HTTP status code as well in case there was an error
  • Hedgehog overlay for the 404 page

There's also one hidden feature in this PR

  • Support for dashboard items with different colors. Currently the menu is just disabled, as I haven't adapted the colors for the line graphs and pie charts. They work well with funnels though.

There's only one problem. mypy gives me some error and I'm not sure what to do about it. @timgl any thoughts?

I'm also not 100% sure of the django code, so please check and feedback :).

Checklist

  • All querysets/queries filter by Team (if applicable)
  • Backend tests (if applicable)

@timgl timgl temporarily deployed to posthog-703-multiple-da-birnly May 11, 2020 12:20 Inactive
@mariusandra mariusandra requested a review from timgl May 11, 2020 12:24
posthog/api/dashboard.py Outdated Show resolved Hide resolved
@timgl timgl temporarily deployed to posthog-703-multiple-da-birnly May 11, 2020 12:26 Inactive
Copy link
Collaborator

@timgl timgl left a comment

Choose a reason for hiding this comment

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

Quite a few comments but in general this looks great!

frontend/src/scenes/dashboard/DashboardItems.scss Outdated Show resolved Hide resolved
frontend/src/style/nprogress.css Outdated Show resolved Hide resolved
frontend/src/index.js Outdated Show resolved Hide resolved
frontend/src/layout/Sidebar.js Show resolved Hide resolved
frontend/src/scenes/dashboard/DashboardItems.js Outdated Show resolved Hide resolved
frontend/src/models/dashboardsModel.js Outdated Show resolved Hide resolved
frontend/src/layout/Sidebar.js Outdated Show resolved Hide resolved
frontend/src/models/dashboardsModel.js Show resolved Hide resolved
frontend/src/scenes/dashboard/DashboardHeader.js Outdated Show resolved Hide resolved
frontend/src/scenes/dashboard/Dashboards.js Outdated Show resolved Hide resolved
@timgl timgl temporarily deployed to posthog-703-multiple-da-birnly May 11, 2020 14:19 Inactive
@timgl timgl temporarily deployed to posthog-703-multiple-da-birnly May 11, 2020 14:31 Inactive
@mariusandra
Copy link
Collaborator Author

Hi, I fixed all the points, except for the sidebars, where I left a comment and propose to keep it like this for a bit and perhaps update within a week.

Copy link
Collaborator

@timgl timgl left a comment

Choose a reason for hiding this comment

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

One small comment, otherwise looks great 👍

frontend/src/scenes/dashboard/Dashboards.js Show resolved Hide resolved
@timgl timgl temporarily deployed to posthog-703-multiple-da-birnly May 11, 2020 16:06 Inactive
@mariusandra mariusandra merged commit c324582 into master May 11, 2020
@mariusandra mariusandra deleted the 703-multiple-dashboards branch May 11, 2020 16:09
fuziontech added a commit to fuziontech/posthog that referenced this pull request May 11, 2020
* upstream/master:
  Closes PostHog#169 break down by cohort (PostHog#690)
  703 multiple dashboards (PostHog#740)
  Use person_id instead of distinct_id for unique count (PostHog#734)
  new contributors (PostHog#739)
  Update Trends dotted line UX (PostHog#735)
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.

Multiple dashboards
2 participants