refactor(insights): remove filter usage from insight legend and break…

…down labels (#22847)
PostHog · Jun 12, 2024 · 5ad9b93 · 5ad9b93
1 parent f6f25f0
commit 5ad9b93
Show file tree

Hide file tree

Showing 19 changed files with 178 additions and 184 deletions.
diff --git a/frontend/src/lib/components/InsightLegend/InsightLegend.tsx b/frontend/src/lib/components/InsightLegend/InsightLegend.tsx
@@ -1,12 +1,11 @@
 import './InsightLegend.scss'
 
 import clsx from 'clsx'
-import { useActions, useValues } from 'kea'
+import { useValues } from 'kea'
 import { insightLogic } from 'scenes/insights/insightLogic'
 import { trendsDataLogic } from 'scenes/trends/trendsDataLogic'
 
 import { InsightLegendRow } from './InsightLegendRow'
-import { shouldHighlightThisRow } from './utils'
 
 export interface InsightLegendProps {
     readOnly?: boolean
@@ -15,11 +14,8 @@ export interface InsightLegendProps {
 }
 
 export function InsightLegend({ horizontal, inCardView, readOnly = false }: InsightLegendProps): JSX.Element | null {
-    const { insightProps, highlightedSeries, hiddenLegendKeys } = useValues(insightLogic)
-    const { toggleVisibility } = useActions(insightLogic)
-    const { indexedResults, compare, display, trendsFilter, hasLegend, isSingleSeries } = useValues(
-        trendsDataLogic(insightProps)
-    )
+    const { insightProps } = useValues(insightLogic)
+    const { indexedResults, hasLegend } = useValues(trendsDataLogic(insightProps))
 
     return hasLegend ? (
         <div
@@ -31,20 +27,7 @@ export function InsightLegend({ horizontal, inCardView, readOnly = false }: Insi
         >
             <div className="grid grid-cols-1">
                 {indexedResults &&
-                    indexedResults.map((item, index) => (
-                        <InsightLegendRow
-                            key={index}
-                            hiddenLegendKeys={hiddenLegendKeys}
-                            item={item}
-                            rowIndex={index}
-                            hasMultipleSeries={!isSingleSeries}
-                            highlighted={shouldHighlightThisRow(hiddenLegendKeys, index, highlightedSeries)}
-                            toggleVisibility={toggleVisibility}
-                            compare={compare}
-                            display={display}
-                            trendsFilter={trendsFilter}
-                        />
-                    ))}
+                    indexedResults.map((item, index) => <InsightLegendRow key={index} item={item} rowIndex={index} />)}
             </div>
         </div>
     ) : null

diff --git a/frontend/src/lib/components/InsightLegend/InsightLegendRow.tsx b/frontend/src/lib/components/InsightLegend/InsightLegendRow.tsx
@@ -1,45 +1,35 @@
-import { useValues } from 'kea'
+import { useActions, useValues } from 'kea'
 import { getSeriesColor } from 'lib/colors'
 import { InsightLabel } from 'lib/components/InsightLabel'
 import { LemonCheckbox } from 'lib/lemon-ui/LemonCheckbox'
 import { useEffect, useRef } from 'react'
 import { formatAggregationAxisValue } from 'scenes/insights/aggregationAxisFormat'
-import { isTrendsFilter } from 'scenes/insights/sharedUtils'
+import { insightLogic } from 'scenes/insights/insightLogic'
 import { formatBreakdownLabel } from 'scenes/insights/utils'
 import { formatCompareLabel } from 'scenes/insights/views/InsightsTable/columns/SeriesColumn'
+import { trendsDataLogic } from 'scenes/trends/trendsDataLogic'
 import { IndexedTrendResult } from 'scenes/trends/types'
 
 import { cohortsModel } from '~/models/cohortsModel'
 import { propertyDefinitionsModel } from '~/models/propertyDefinitionsModel'
-import { TrendsFilter } from '~/queries/schema'
 import { ChartDisplayType } from '~/types'
 
+import { shouldHighlightThisRow } from './utils'
+
 type InsightLegendRowProps = {
-    hiddenLegendKeys: Record<string, boolean | undefined>
     rowIndex: number
     item: IndexedTrendResult
-    hasMultipleSeries: boolean
-    toggleVisibility: (index: number) => void
-    compare?: boolean | null
-    display?: ChartDisplayType | null
-    trendsFilter?: TrendsFilter | null
-    highlighted: boolean
 }
 
-export function InsightLegendRow({
-    hiddenLegendKeys,
-    rowIndex,
-    item,
-    hasMultipleSeries,
-    toggleVisibility,
-    compare,
-    display,
-    trendsFilter,
-    highlighted,
-}: InsightLegendRowProps): JSX.Element {
+export function InsightLegendRow({ rowIndex, item }: InsightLegendRowProps): JSX.Element {
     const { cohorts } = useValues(cohortsModel)
     const { formatPropertyValueForDisplay } = useValues(propertyDefinitionsModel)
 
+    const { insightProps, hiddenLegendKeys, highlightedSeries } = useValues(insightLogic)
+    const { toggleVisibility } = useActions(insightLogic)
+    const { compare, display, trendsFilter, breakdownFilter, isSingleSeries } = useValues(trendsDataLogic(insightProps))
+
+    const highlighted = shouldHighlightThisRow(hiddenLegendKeys, rowIndex, highlightedSeries)
     const highlightStyle: Record<string, any> = highlighted
         ? {
               style: { backgroundColor: getSeriesColor(item.seriesIndex, false, true) },
@@ -54,12 +44,10 @@ export function InsightLegendRow({
     }, [highlighted])
 
     const formattedBreakdownValue = formatBreakdownLabel(
-        cohorts,
-        formatPropertyValueForDisplay,
         item.breakdown_value,
-        item.filter?.breakdown,
-        item.filter?.breakdown_type,
-        item.filter && isTrendsFilter(item.filter) && item.filter?.breakdown_histogram_bin_count !== undefined
+        breakdownFilter,
+        cohorts,
+        formatPropertyValueForDisplay
     )
 
     return (
@@ -77,10 +65,10 @@ export function InsightLegendRow({
                             seriesColor={getSeriesColor(item.seriesIndex, compare)}
                             action={item.action}
                             fallbackName={item.breakdown_value === '' ? 'None' : item.label}
-                            hasMultipleSeries={hasMultipleSeries}
+                            hasMultipleSeries={!isSingleSeries}
                             breakdownValue={formattedBreakdownValue}
                             compareValue={compare ? formatCompareLabel(item) : undefined}
-                            pillMidEllipsis={item?.filter?.breakdown === '$current_url'} // TODO: define set of breakdown values that would benefit from mid ellipsis truncation
+                            pillMidEllipsis={breakdownFilter?.breakdown === '$current_url'} // TODO: define set of breakdown values that would benefit from mid ellipsis truncation
                             hideIcon
                         />
                     }

diff --git a/frontend/src/scenes/funnels/FunnelTooltip.tsx b/frontend/src/scenes/funnels/FunnelTooltip.tsx
@@ -56,11 +56,10 @@ export function FunnelTooltip({
                     <EntityFilterInfo filter={getActionFilterFromFunnelStep(series)} allowWrap />
                     <span className="mx-1">•</span>
                     {formatBreakdownLabel(
-                        cohorts,
-                        formatPropertyValueForDisplay,
                         series.breakdown_value,
-                        series.breakdown,
-                        breakdownFilter?.breakdown_type
+                        breakdownFilter,
+                        cohorts,
+                        formatPropertyValueForDisplay
                     )}
                 </strong>
             </LemonRow>

diff --git a/frontend/src/scenes/insights/InsightTooltip/InsightTooltip.stories.tsx b/frontend/src/scenes/insights/InsightTooltip/InsightTooltip.stories.tsx
@@ -31,38 +31,6 @@ const data = {
             label: '$pageview',
             color: '#1d4aff',
             count: 1,
-            filter: {
-                breakdown_attribution_type: 'first_touch',
-                date_from: '-7d',
-                display: 'ActionsLineGraph',
-                events: [
-                    {
-                        id: '$pageview',
-                        type: 'events',
-                        order: 0,
-                        name: '$pageview',
-                        custom_name: null,
-                        math: 'dau',
-                        math_property: null,
-                        math_group_type_index: null,
-                        properties: {},
-                    },
-                    {
-                        id: 'filter added',
-                        type: 'events',
-                        order: 1,
-                        name: 'filter added',
-                        custom_name: null,
-                        math: null,
-                        math_property: null,
-                        math_group_type_index: null,
-                        properties: {},
-                    },
-                ],
-                insight: 'TRENDS',
-                interval: 'day',
-                smoothing_intervals: 1,
-            },
         },
         {
             id: 1,
@@ -83,38 +51,6 @@ const data = {
             label: 'filter added',
             color: '#621da6',
             count: 1,
-            filter: {
-                breakdown_attribution_type: 'first_touch',
-                date_from: '-7d',
-                display: 'ActionsLineGraph',
-                events: [
-                    {
-                        id: '$pageview',
-                        type: 'events',
-                        order: 0,
-                        name: '$pageview',
-                        custom_name: null,
-                        math: 'dau',
-                        math_property: null,
-                        math_group_type_index: null,
-                        properties: {},
-                    },
-                    {
-                        id: 'filter added',
-                        type: 'events',
-                        order: 1,
-                        name: 'filter added',
-                        custom_name: null,
-                        math: null,
-                        math_property: null,
-                        math_group_type_index: null,
-                        properties: {},
-                    },
-                ],
-                insight: 'TRENDS',
-                interval: 'day',
-                smoothing_intervals: 1,
-            },
         },
     ],
 }

diff --git a/frontend/src/scenes/insights/InsightTooltip/InsightTooltip.tsx b/frontend/src/scenes/insights/InsightTooltip/InsightTooltip.tsx
@@ -87,6 +87,7 @@ export function InsightTooltip({
     colCutoff = COL_CUTOFF,
     showHeader = true,
     groupTypeLabel = 'people',
+    breakdownFilter,
 }: InsightTooltipProps): JSX.Element {
     // If multiple entities exist (i.e., pageview + autocapture) and there is a breakdown/compare/multi-group happening, itemize entities as columns to save vertical space..
     // If only a single entity exists, itemize entity counts as rows.
@@ -109,7 +110,7 @@ export function InsightTooltip({
 
     if (itemizeEntitiesAsColumns) {
         hideColorCol = true
-        const dataSource = invertDataSource(seriesData)
+        const dataSource = invertDataSource(seriesData, breakdownFilter)
         const columns: LemonTableColumns<InvertedSeriesDatum> = [
             {
                 key: 'datum',

diff --git a/frontend/src/scenes/insights/InsightTooltip/insightTooltipUtils.tsx b/frontend/src/scenes/insights/InsightTooltip/insightTooltipUtils.tsx
@@ -1,9 +1,9 @@
 import { dayjs } from 'lib/dayjs'
 import { capitalizeFirstLetter, midEllipsis, pluralize } from 'lib/utils'
-import { isTrendsFilter } from 'scenes/insights/sharedUtils'
 
 import { cohortsModel } from '~/models/cohortsModel'
 import { propertyDefinitionsModel } from '~/models/propertyDefinitionsModel'
+import { BreakdownFilter } from '~/queries/schema'
 import { ActionFilter, CompareLabelType, FilterType, IntervalType } from '~/types'
 
 import { formatBreakdownLabel } from '../utils'
@@ -56,6 +56,7 @@ export interface InsightTooltipProps extends Omit<TooltipConfig, 'renderSeries'
     hideInspectActorsSection?: boolean
     seriesData: SeriesDatum[]
     formula?: boolean
+    breakdownFilter?: BreakdownFilter | undefined | null
     groupTypeLabel?: string
     timezone?: string | null
 }
@@ -99,7 +100,10 @@ export function getFormattedDate(input?: string | number, interval: IntervalType
     return String(input)
 }
 
-export function invertDataSource(seriesData: SeriesDatum[]): InvertedSeriesDatum[] {
+export function invertDataSource(
+    seriesData: SeriesDatum[],
+    breakdownFilter: BreakdownFilter | null | undefined
+): InvertedSeriesDatum[] {
     // NOTE: Assuming these logics are mounted elsewhere, and we're not interested in tracking changes.
     const cohorts = cohortsModel.findMounted()?.values?.cohorts
     const formatPropertyValueForDisplay = propertyDefinitionsModel.findMounted()?.values?.formatPropertyValueForDisplay
@@ -109,14 +113,7 @@ export function invertDataSource(seriesData: SeriesDatum[]): InvertedSeriesDatum
         const pillValues = []
         if (s.breakdown_value !== undefined) {
             pillValues.push(
-                formatBreakdownLabel(
-                    cohorts,
-                    formatPropertyValueForDisplay,
-                    s.breakdown_value,
-                    s.filter?.breakdown,
-                    s.filter?.breakdown_type,
-                    s.filter && isTrendsFilter(s.filter) && s.filter?.breakdown_histogram_bin_count !== undefined
-                )
+                formatBreakdownLabel(s.breakdown_value, breakdownFilter, cohorts, formatPropertyValueForDisplay)
             )
         }
         if (s.compare_label) {

diff --git a/frontend/src/scenes/insights/filters/ActionFilter/ActionFilterRow/ActionFilterRow.tsx b/frontend/src/scenes/insights/filters/ActionFilter/ActionFilterRow/ActionFilterRow.tsx
@@ -53,7 +53,7 @@ import { LocalFilter } from '../entityFilterLogic'
 import { entityFilterLogicType } from '../entityFilterLogicType'
 
 const DragHandle = (props: DraggableSyntheticListeners | undefined): JSX.Element => (
-    <span className="ActionFilterRowDragHandle" {...props}>
+    <span className="ActionFilterRowDragHandle" key="drag-handle" {...props}>
         <SortableDragIcon />
     </span>
 )