style(lemon-ui): Make Sparkline look great (#19813)

* style: Make `Sparkline` look great

* Fix formatting

* Fix formatting more

* Update UI snapshots for `chromium` (1)

* Incorporate feedback

* Skip "Events dropped" in destinations if none were

---------

Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>

frontend/src/lib/lemon-ui/Popover/Popover.scss

@@ -29,6 +29,7 @@
     flex-grow: 1;
     max-width: 100%;
     padding: 0.5rem;
+    overflow: hidden;
     background: var(--bg-light);
     border: 1px solid var(--border);
     border-radius: var(--radius);

frontend/src/lib/lemon-ui/Popover/Popover.tsx

@@ -43,6 +43,8 @@ export interface PopoverProps {
     sameWidth?: boolean
     maxContentWidth?: boolean
     className?: string
+    /** Whether default box padding should be applies. @default true */
+    padded?: boolean
     middleware?: Middleware[]
     /** Any other refs that needs to be taken into account for handling outside clicks e.g. other nested popovers.
      * Works also with strings, matching classnames or ids, for antd legacy components that don't support refs
@@ -84,6 +86,7 @@ export const Popover = React.forwardRef<HTMLDivElement, PopoverProps>(function P
         placement = 'bottom-start',
         fallbackPlacements = ['bottom-start', 'bottom-end', 'top-start', 'top-end'],
         className,
+        padded = true,
         actionable = false,
         middleware,
         sameWidth = false,
@@ -249,7 +252,7 @@ export const Popover = React.forwardRef<HTMLDivElement, PopoverProps>(function P
                             onMouseLeave={onMouseLeaveInside}
                             aria-level={currentPopoverLevel}
                         >
-                            <div className="Popover__box">
+                            <div className={clsx('Popover__box', !padded && 'p-0')}>
                                 {showArrow && isAttached && (
                                     // Arrow is outside of .Popover__content to avoid affecting :nth-child for content
                                     <div

frontend/src/lib/lemon-ui/Sparkline.tsx

@@ -1,30 +1,51 @@
-import { offset } from '@floating-ui/react'
-import { Chart, ChartItem, TooltipModel } from 'lib/Chart'
+import clsx from 'clsx'
+import { Chart, ChartItem } from 'lib/Chart'
 import { getColorVar } from 'lib/colors'
 import { Popover } from 'lib/lemon-ui/Popover/Popover'
-import React from 'react'
+import { humanFriendlyNumber } from 'lib/utils'
 import { useEffect, useRef, useState } from 'react'
+import { InsightTooltip } from 'scenes/insights/InsightTooltip/InsightTooltip'
 
-interface SparkLineTimeSeries {
-    name: string | null
-    /** Check vars.scss for available colors */
-    color: string
+import { LemonSkeleton } from './LemonSkeleton'
+
+export interface SparklineTimeSeries {
+    name: string
     values: number[]
+    /** Check vars.scss for available colors. @default 'muted' */
+    color?: string
 }
 
 interface SparklineProps {
     /** Optional labels for the X axis. */
     labels?: string[]
     /** Either a list of numbers for a muted graph or an array of time series */
-    data: number[] | SparkLineTimeSeries[]
+    data: number[] | SparklineTimeSeries[]
+    /** @default 'bar' */
+    type?: 'bar' | 'line'
+    /** Whether the Y-axis maximum should be indicated in the graph. @default true */
+    maximumIndicator?: boolean
+    /** A skeleton is shown during loading. */
+    loading?: boolean
+    className?: string
 }
 
-export function Sparkline({ labels, data }: SparklineProps): JSX.Element {
+export function Sparkline({
+    labels,
+    data,
+    type = 'bar',
+    maximumIndicator = true,
+    loading = false,
+    className,
+}: SparklineProps): JSX.Element {
     const canvasRef = useRef<HTMLCanvasElement | null>(null)
     const tooltipRef = useRef<HTMLDivElement | null>(null)
 
+    const [isTooltipShown, setIsTooltipShown] = useState(false)
     const [popoverContent, setPopoverContent] = useState<JSX.Element | null>(null)
-    const [popoverOffset, setPopoverOffset] = useState(0)
+
+    const adjustedData: SparklineTimeSeries[] = !isSparkLineTimeSeries(data)
+        ? [{ name: 'Data', color: 'muted', values: data }]
+        : data
 
     useEffect(() => {
         // data should always be provided but React can render this without it,
@@ -33,33 +54,75 @@ export function Sparkline({ labels, data }: SparklineProps): JSX.Element {
             return
         }
 
-        const adjustedData: SparkLineTimeSeries[] = !isSparkLineTimeSeries(data)
-            ? [{ name: null, color: 'muted', values: data }]
-            : data
-
         let chart: Chart
         if (canvasRef.current) {
-            chart = new Chart(canvasRef.current?.getContext('2d') as ChartItem, {
-                type: 'bar',
+            chart = new Chart(canvasRef.current.getContext('2d') as ChartItem, {
+                type,
                 data: {
                     labels: labels || adjustedData[0].values.map((_, i) => `Entry ${i}`),
-                    datasets: adjustedData.map((timeseries) => ({
-                        data: timeseries.values,
-                        minBarLength: 0,
-                        backgroundColor: getColorVar(timeseries.color),
-                        hoverBackgroundColor: getColorVar('primary'),
-                    })),
+                    datasets: adjustedData.map((timeseries) => {
+                        const color = getColorVar(timeseries.color || 'muted')
+                        return {
+                            label: timeseries.name,
+                            data: timeseries.values,
+                            minBarLength: 0,
+                            categoryPercentage: 0.9, // Slightly tighter bar spacing than the default 0.8
+                            backgroundColor: color,
+                            borderColor: color,
+                            borderWidth: type === 'line' ? 2 : 0,
+                            pointRadius: 0,
+                        }
+                    }),
                 },
                 options: {
                     scales: {
                         x: {
-                            display: false,
+                            // X axis not needed in line charts without indicators
+                            display: type === 'bar' || maximumIndicator,
+                            bounds: 'data',
                             stacked: true,
+                            ticks: {
+                                display: false,
+                            },
+                            grid: {
+                                drawTicks: false,
+                                display: false,
+                            },
+                            alignToPixels: true,
                         },
                         y: {
-                            beginAtZero: true,
-                            display: false,
+                            // We use the Y axis for the maximum indicator
+                            display: maximumIndicator,
+                            bounds: 'data',
+                            min: 0, // Always starting at 0
+                            suggestedMax: 1,
                             stacked: true,
+                            ticks: {
+                                includeBounds: true,
+                                autoSkip: true,
+                                maxTicksLimit: 1, // Only the max
+                                align: 'start',
+                                callback: (tickValue) =>
+                                    typeof tickValue === 'number' && tickValue > 0 // Hide the zero tick
+                                        ? humanFriendlyNumber(tickValue)
+                                        : null,
+                                font: {
+                                    size: 10,
+                                    lineHeight: 1,
+                                },
+                            },
+                            grid: {
+                                borderDash: [2],
+                                drawBorder: false,
+                                display: true,
+                                tickLength: 0,
+                            },
+                            alignToPixels: true,
+                            afterFit: (axis) => {
+                                // Remove unneccessary padding
+                                axis.paddingTop = 1 // 1px and not 0 to avoid clipping of the grid
+                                axis.paddingBottom = 1
+                            },
                         },
                     },
                     plugins: {
@@ -69,32 +132,27 @@ export function Sparkline({ labels, data }: SparklineProps): JSX.Element {
                             display: false,
                         },
                         tooltip: {
-                            // TODO: use InsightsTooltip instead
-                            enabled: false, // using external tooltip
-                            external({ tooltip }: { chart: Chart; tooltip: TooltipModel<'bar'> }) {
-                                if (tooltip.opacity === 0) {
-                                    setPopoverContent(null)
-                                    return
-                                }
-                                const datapoint = tooltip.dataPoints[0]
-                                const toolTipLabel = datapoint.label ? `${datapoint.label}: ` : ''
-                                if (tooltip.dataPoints.length === 1) {
-                                    const tooltipContent = toolTipLabel + datapoint.formattedValue
-                                    setPopoverContent(<>{tooltipContent}</>)
-                                } else {
-                                    const tooltipContent = [<React.Fragment key="-1">{toolTipLabel}</React.Fragment>]
-                                    for (let i = 0; i < tooltip.dataPoints.length; i++) {
-                                        const datapoint = tooltip.dataPoints[i]
-                                        tooltipContent.push(
-                                            <React.Fragment key={i}>
-                                                <br />
-                                                {adjustedData[i].name}: {datapoint.formattedValue}
-                                            </React.Fragment>
-                                        )
-                                    }
-                                    setPopoverContent(<>{tooltipContent}</>)
-                                }
-                                setPopoverOffset(tooltip.x)
+                            enabled: false, // Using external tooltip
+                            external({ tooltip }) {
+                                setIsTooltipShown(tooltip.opacity > 0)
+                                setPopoverContent(
+                                    <InsightTooltip
+                                        embedded
+                                        hideInspectActorsSection
+                                        showHeader={!!labels}
+                                        date={tooltip.dataPoints[0].label}
+                                        seriesData={tooltip.dataPoints.map((dp, i) => ({
+                                            id: i,
+                                            dataIndex: 0,
+                                            datasetIndex: 0,
+                                            label: dp.dataset.label,
+                                            color: dp.dataset.borderColor as string,
+                                            count: (dp.dataset.data?.[dp.dataIndex] as number) || 0,
+                                        }))}
+                                        renderSeries={(value) => value}
+                                        renderCount={(count) => humanFriendlyNumber(count)}
+                                    />
+                                )
                             },
                         },
                     },
@@ -112,21 +170,25 @@ export function Sparkline({ labels, data }: SparklineProps): JSX.Element {
         }
     }, [labels, data])
 
-    return (
-        <div className="w-full">
-            <canvas ref={canvasRef} className="h-9" />
-            <Popover
-                visible={!!popoverContent}
-                overlay={popoverContent}
-                placement="bottom-start"
-                middleware={[offset({ crossAxis: popoverOffset })]}
-            >
+    const dataPointCount = adjustedData[0].values.length
+    const finalClassName = clsx(
+        dataPointCount > 16 ? 'w-64' : dataPointCount > 8 ? 'w-48' : dataPointCount > 4 ? 'w-32' : 'w-24',
+        'h-8',
+        className
+    )
+
+    return !loading ? (
+        <div className={finalClassName}>
+            <canvas ref={canvasRef} />
+            <Popover visible={isTooltipShown} overlay={popoverContent} placement="bottom-start" padded={false}>
                 <div ref={tooltipRef} />
             </Popover>
         </div>
+    ) : (
+        <LemonSkeleton className={finalClassName} />
     )
 }
 
-function isSparkLineTimeSeries(data: number[] | SparkLineTimeSeries[]): data is SparkLineTimeSeries[] {
+function isSparkLineTimeSeries(data: number[] | SparklineTimeSeries[]): data is SparklineTimeSeries[] {
     return typeof data[0] !== 'number'
 }

frontend/src/scenes/insights/InsightTooltip/InsightTooltip.scss

@@ -15,6 +15,12 @@
     border-radius: var(--radius);
     box-shadow: var(--shadow-elevation);
 
+    &--embedded {
+        border: none;
+        border-radius: 0;
+        box-shadow: none;
+    }
+
     .LemonRow {
         font-size: 0.8125rem;
     }

frontend/src/scenes/insights/InsightTooltip/InsightTooltip.tsx

@@ -1,5 +1,6 @@
 import './InsightTooltip.scss'
 
+import clsx from 'clsx'
 import { useValues } from 'kea'
 import { InsightLabel } from 'lib/components/InsightLabel'
 import { IconHandClick } from 'lib/lemon-ui/icons'
@@ -78,6 +79,7 @@ export function InsightTooltip({
     altRightTitle,
     renderSeries,
     renderCount,
+    embedded = false,
     hideColorCol = false,
     hideInspectActorsSection = false,
     entitiesAsColumnsOverride,
@@ -171,7 +173,7 @@ export function InsightTooltip({
         }
 
         return (
-            <div className="InsightTooltip">
+            <div className={clsx('InsightTooltip', embedded && 'InsightTooltip--embedded')}>
                 <LemonTable
                     dataSource={dataSource.slice(0, rowCutoff)}
                     columns={columns}
@@ -232,7 +234,7 @@ export function InsightTooltip({
     })
 
     return (
-        <div className="InsightTooltip">
+        <div className={clsx('InsightTooltip', embedded && 'InsightTooltip--embedded')}>
             <LemonTable
                 dataSource={dataSource.slice(0, rowCutoff)}
                 columns={columns}

frontend/src/scenes/insights/InsightTooltip/insightTooltipUtils.tsx

@@ -46,6 +46,12 @@ export interface TooltipConfig {
 export interface InsightTooltipProps extends Omit<TooltipConfig, 'renderSeries' | 'renderCount'> {
     renderSeries: Required<TooltipConfig>['renderSeries']
     renderCount: Required<TooltipConfig>['renderCount']
+    /**
+     * Whether the tooltip should be rendered as a table embeddable into an existing popover
+     * (instead of as a popover of its own)
+     * @default false
+     */
+    embedded?: boolean
     date?: string
     hideInspectActorsSection?: boolean
     seriesData: SeriesDatum[]

frontend/src/scenes/pipeline/Destinations.tsx

@@ -1,20 +1,11 @@
-import {
-    LemonButton,
-    LemonDivider,
-    LemonSkeleton,
-    LemonTable,
-    LemonTableColumn,
-    LemonTag,
-    Link,
-    Tooltip,
-} from '@posthog/lemon-ui'
+import { LemonButton, LemonDivider, LemonTable, LemonTableColumn, LemonTag, Link, Tooltip } from '@posthog/lemon-ui'
 import { useActions, useValues } from 'kea'
 import { ProductIntroduction } from 'lib/components/ProductIntroduction/ProductIntroduction'
 import { FEATURE_FLAGS } from 'lib/constants'
 import { More } from 'lib/lemon-ui/LemonButton/More'
 import { LemonMarkdown } from 'lib/lemon-ui/LemonMarkdown/LemonMarkdown'
 import { updatedAtColumn } from 'lib/lemon-ui/LemonTable/columnUtils'
-import { Sparkline } from 'lib/lemon-ui/Sparkline'
+import { Sparkline, SparklineTimeSeries } from 'lib/lemon-ui/Sparkline'
 import { featureFlagLogic } from 'lib/logic/featureFlagLogic'
 import { deleteWithUndo } from 'lib/utils/deleteWithUndo'
 
@@ -218,17 +209,26 @@ function DestinationSparkLine({ destination }: { destination: DestinationType })
         const logic = pipelineAppMetricsLogic({ pluginConfigId: destination.id })
         const { appMetricsResponse } = useValues(logic)
 
-        if (appMetricsResponse === null) {
-            return <LemonSkeleton className="w-full h-9" />
+        const displayData: SparklineTimeSeries[] = [
+            {
+                color: 'success',
+                name: 'Events sent',
+                values: appMetricsResponse ? appMetricsResponse.metrics.successes : [],
+            },
+        ]
+        if (appMetricsResponse?.metrics.failures.some((failure) => failure > 0)) {
+            displayData.push({
+                color: 'danger',
+                name: 'Events dropped',
+                values: appMetricsResponse ? appMetricsResponse.metrics.failures : [],
+            })
         }
 
         return (
             <Sparkline
-                labels={appMetricsResponse.metrics.dates}
-                data={[
-                    { color: 'danger', name: 'failures', values: appMetricsResponse.metrics.failures },
-                    { color: 'success', name: 'sucesses', values: appMetricsResponse.metrics.successes },
-                ]}
+                loading={appMetricsResponse === null}
+                labels={appMetricsResponse ? appMetricsResponse.metrics.dates : []}
+                data={displayData}
             />
         )
     }