feat: Improve insights overall loading state

This isn't very modern and it occupies a lot of space. This was prompted by it not fitting the Web Vitals UI, let's tweak it all around the app and see if it looks nicer like this.
PostHog · Feb 3, 2025 · 531668a · 531668a
1 parent df2dc69
commit 531668a
Show file tree

Hide file tree

Showing 12 changed files with 327 additions and 375 deletions.
diff --git a/cypress/e2e/dashboard-shared.cy.ts b/cypress/e2e/dashboard-shared.cy.ts
@@ -61,7 +61,8 @@ describe('Shared dashboard', () => {
         })
 
         cy.get('.InsightCard').should('have.length', 6)
+
         // Make sure no element with text "There are no matching events for this query" exists
-        cy.get('.insight-empty-state').should('not.exist')
+        cy.get('[data-attr="insight-empty-state"]').should('not.exist')
     })
 })
diff --git a/cypress/e2e/insights-navigation-open-directly.cy.ts b/cypress/e2e/insights-navigation-open-directly.cy.ts
@@ -27,7 +27,7 @@ describe('Insights', () => {
 
             it('can open a new funnels insight', () => {
                 insight.newInsight('FUNNELS')
-                cy.get('.funnels-empty-state__title').should('exist')
+                cy.get('[data-attr="insight-empty-state"]').find('h2').should('exist')
             })
 
             it('can open a new paths insight', () => {

diff --git a/cypress/e2e/insights-navigation-open-sql-insight-first.cy.ts b/cypress/e2e/insights-navigation-open-sql-insight-first.cy.ts
@@ -40,7 +40,7 @@ describe('Insights', () => {
 
             it('can open a new funnels insight', () => {
                 insight.clickTab('FUNNELS')
-                cy.get('.funnels-empty-state__title').should('exist')
+                cy.get('[data-attr="insight-empty-state"]').find('h2').should('exist')
             })
 
             it('can open a new retention insight', () => {

diff --git a/cypress/e2e/insights-saved.cy.ts b/cypress/e2e/insights-saved.cy.ts
@@ -13,7 +13,8 @@ describe('Insights - saved', () => {
         })
         cy.task('resetInsightCache').then(() => {
             cy.visit(urls.insightView(newInsightId)) // Full refresh
-            cy.get('.insight-empty-state').should('exist') // There should be a loading state for a moment
+
+            cy.get('[data-attr="insight-empty-state"]').should('exist') // There should be a loading state for a moment
             cy.wait('@getInsightsRefreshAsync').then(() => {
                 cy.get('[data-attr=trend-line-graph]').should('exist')
             })

diff --git a/frontend/src/lib/components/Cards/InsightCard/InsightCard.scss b/frontend/src/lib/components/Cards/InsightCard/InsightCard.scss
@@ -37,13 +37,6 @@
         padding: 0.5rem;
     }
 
-    .insight-empty-state {
-        height: 100%; // Fix wonkiness when SpinnerOverlay is shown
-        padding-top: 0;
-        padding-bottom: 0;
-        font-size: 0.875rem; // Reduce font size
-    }
-
     .LemonTable {
         background: none;
         border: none;

diff --git a/frontend/src/queries/nodes/InsightViz/InsightVizDisplay.tsx b/frontend/src/queries/nodes/InsightViz/InsightVizDisplay.tsx
@@ -83,11 +83,7 @@ export function InsightVizDisplay({
     // Empty states that completely replace the graph
     const BlockingEmptyState = (() => {
         if (insightDataLoading) {
-            return (
-                <div className="flex flex-col flex-1 justify-center items-center p-2">
-                    <InsightLoadingState queryId={queryId} key={queryId} insightProps={insightProps} />
-                </div>
-            )
+            return <InsightLoadingState queryId={queryId} key={queryId} insightProps={insightProps} />
         }
 
         if (validationError) {

diff --git a/frontend/src/queries/nodes/WebVitals/WebVitalsContent.tsx b/frontend/src/queries/nodes/WebVitals/WebVitalsContent.tsx
@@ -43,11 +43,7 @@ export const WebVitalsContent = ({ webVitalsQueryResponse }: WebVitalsContentPro
     // NOTE: `band` will only return `none` if the value is undefined,
     // so this is basically the same check twice, but we need that to make TS happy
     if (value === undefined || band === 'none') {
-        return (
-            <div className="w-full border rounded p-4 md:w-[30%]">
-                <LemonSkeleton fade className="w-full h-40" />
-            </div>
-        )
+        return <LemonSkeleton fade className="w-full h-40 rounded md:w-[30%]" />
     }
 
     const grade = GRADE_PER_BAND[band]
@@ -60,7 +56,7 @@ export const WebVitalsContent = ({ webVitalsQueryResponse }: WebVitalsContentPro
     const experience = EXPERIENCE_PER_BAND[band]
 
     return (
-        <div className="w-full border rounded p-6 md:w-[30%] flex flex-col gap-2">
+        <div className="w-full border rounded p-6 md:w-[30%] flex flex-col gap-2 m-2">
             <span className="text-lg">
                 <strong>{LONG_METRIC_NAME[webVitalsTab]}</strong>
             </span>

diff --git a/frontend/src/scenes/data-warehouse/editor/OutputPane.tsx b/frontend/src/scenes/data-warehouse/editor/OutputPane.tsx
@@ -367,7 +367,9 @@ const Content = ({
         }
 
         return responseLoading ? (
-            <StatelessInsightLoadingState queryId={queryId} pollResponse={pollResponse} />
+            <div className="flex flex-1 p-2 w-full">
+                <StatelessInsightLoadingState queryId={queryId} pollResponse={pollResponse} />
+            </div>
         ) : !response ? (
             <div className="flex flex-1 justify-center items-center">
                 <span className="text-muted mt-3">Query results will appear here</span>

diff --git a/frontend/src/scenes/insights/EmptyStates/EmptyStates.scss b/frontend/src/scenes/insights/EmptyStates/EmptyStates.scss
@@ -1,50 +1,4 @@
-.insight-empty-state {
-    display: flex;
-    flex-direction: column;
-    flex-grow: 1;
-    align-items: center;
-    justify-content: center;
-    padding: 1rem;
-    font-size: 1.1em;
-    color: var(--muted);
-
-    &.error {
-        h2 {
-            color: var(--danger);
-        }
-    }
-
-    &.warning {
-        h2 {
-            color: var(--warning);
-        }
-    }
-
-    h2 {
-        font-size: 1.5rem;
-        font-weight: 600;
-        line-height: 1.6rem;
-        color: var(--primary-alt);
-    }
-
-    .empty-state-inner {
-        display: flex;
-        flex-direction: column;
-        align-items: center;
-        max-width: 600px;
-
-        .illustration-main {
-            font-size: 2.5rem;
-        }
-
-        h2 {
-            width: 100%;
-            text-align: center;
-            word-wrap: break-word;
-        }
-
-        ol {
-            margin: 0.5rem 0;
-        }
-    }
+/* TODO: Migrate this to a CSS variable (with a `bg-` Tailwind class) once @adamleithp finishes his Tailwind work */
+.insights-empty-state {
+    background-color: rgb(129 129 129 / 20%);
 }
diff --git a/frontend/src/scenes/insights/EmptyStates/EmptyStates.stories.tsx b/frontend/src/scenes/insights/EmptyStates/EmptyStates.stories.tsx
@@ -18,7 +18,7 @@ const meta: Meta = {
         layout: 'fullscreen',
         viewMode: 'story',
         testOptions: {
-            waitForSelector: '.empty-state-inner',
+            waitForSelector: '[data-attr="insight-empty-state"]',
         },
     },
 }