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 empty states #2068

Merged
merged 9 commits into from
Oct 28, 2020
Merged

Dashboard empty states #2068

merged 9 commits into from
Oct 28, 2020

Conversation

paolodamico
Copy link
Contributor

@paolodamico paolodamico commented Oct 28, 2020

Changes

Closes #1694.
Closes #1695.

PR introduces the following changes:

  • Depending on a feature flag (1694-dashboards) the default web app dashboard introduced on Dashboard templates & first default dashboard #1942 will automatically be created when a new user signs up.
  • If the feature flag is active, we'll show a new empty state when there's no data to show the line graph (intended for the template dashboards). Dark & light mode supported.

- Introduces a `description` attribute on `DashboardItem` which is used to add more information to the default graphs created (and adds description to the default web dashboard). In the future this attribute can also be used so users can set up their custom description.

  • Introduces an is_sample attribute on DashboardItem to identify items that have been created by templates but that require further configuration. When the user hovers on a graph that fits that description, a CTA to configuration will appear. Whenever a dashboard is altered by a user, is_sample will be set to False to indicate this is no longer a template item.

image

  • Reintroduces Django functional tests on api/signup.

Checklist

  • All querysets/queries filter by Organization, Team, and User (if this PR affects ANY querysets/queries).
  • Django backend tests (if this PR affects the backend).
  • Cypress end-to-end tests (if this PR affects the frontend).

@timgl timgl temporarily deployed to posthog-dashboard-empty-019xut October 28, 2020 13:37 Inactive
@timgl timgl temporarily deployed to posthog-dashboard-empty-019xut October 28, 2020 14:47 Inactive
@paolodamico
Copy link
Contributor Author

FYI @Twixes re signup tests. @jamesefhawkins it'd be great if you can review the default app dashboard and particularly the description for each graph.

@paolodamico paolodamico marked this pull request as ready for review October 28, 2020 14:55
@jamesefhawkins
Copy link
Collaborator

jamesefhawkins commented Oct 28, 2020

  • I created a new account
  • Went through tutorial but skipped sending an event
  • Went to default dashboard, saw a blank screen:

Screenshot 2020-10-28 at 14 58 43

  • On refresh I get this:

Screenshot 2020-10-28 at 14 59 28

  • Clicking out of the above to other pages ie 'Dashboards', gives a blank page too

@timgl timgl temporarily deployed to posthog-dashboard-empty-019xut October 28, 2020 15:00 Inactive
@paolodamico
Copy link
Contributor Author

Re @jamesefhawkins, "Default" means the previously existing default dashboard was created, let me enable the feature flag so you can see the new behavior and you can try again.

Copy link
Collaborator

@Twixes Twixes left a comment

Choose a reason for hiding this comment

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

There are a few bits that I think could use some polish, but great feature!

posthog/models/team.py Outdated Show resolved Hide resolved
posthog/migrations/0094_auto_20201028_1208.py Outdated Show resolved Hide resolved
Comment on lines +317 to +320
{item.description && (
<div style={{ padding: '0 16px', marginBottom: 16, fontSize: 12 }}>{item.description}</div>
)}

Copy link
Collaborator

Choose a reason for hiding this comment

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

This really should allow for editing now IMO, as the descriptions will become irrelevant and confusing when the user updates the dashboard item with some other insight than the initial one.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree, I've opened #2075 to tackle this as an improvement to be able to get this in before the code freeze in the meantime.

frontend/src/scenes/dashboard/DashboardItem.js Outdated Show resolved Hide resolved
Comment on lines +137 to +141
{item.is_sample && (
<div className="sample-dasbhoard-overlay">
<Button onClick={() => router.actions.push(link)}>Configure</Button>
</div>
)}
Copy link
Collaborator

Choose a reason for hiding this comment

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

It's sort of annoying that this Configure overlay just pops in on hover with no warning. I think it should be clear that this needs configuration before attempting to interact + the overlay should be less surprising and intrusive (also, something like a 200ms transition would do a lot here).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yup agreed, just updated, wdyt? the delay would be great, but I prefer to keep this in CSS to simplify

@timgl timgl temporarily deployed to posthog-dashboard-empty-019xut October 28, 2020 15:46 Inactive
@paolodamico paolodamico merged commit e6217f6 into master Oct 28, 2020
@paolodamico paolodamico deleted the dashboard-empty-state branch October 28, 2020 16:28
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.

FR: Dashboard empty state FR: First default dashboard - App
4 participants