From cdceeb40d949e8738db222a485d7be592c136a9a Mon Sep 17 00:00:00 2001 From: Michael Matloka Date: Tue, 26 Apr 2022 16:22:45 +0200 Subject: [PATCH 1/5] fix(funnels): Highlight significant deviations in new funnel viz --- .../lib/components/LemonTable/LemonTable.tsx | 2 +- .../lib/components/LemonTable/TableRow.tsx | 2 +- frontend/src/lib/components/icons.tsx | 4 +- frontend/src/scenes/funnels/funnelLogic.ts | 13 ++--- frontend/src/scenes/insights/Insight.scss | 9 ++++ .../FunnelTab/FunnelStepsTable.tsx | 52 ++++++++++++++++--- 6 files changed, 65 insertions(+), 17 deletions(-) diff --git a/frontend/src/lib/components/LemonTable/LemonTable.tsx b/frontend/src/lib/components/LemonTable/LemonTable.tsx index 5111cd754066c..8fb19d57eaa85 100644 --- a/frontend/src/lib/components/LemonTable/LemonTable.tsx +++ b/frontend/src/lib/components/LemonTable/LemonTable.tsx @@ -33,7 +33,7 @@ export interface LemonTableProps> { /** Which column to use for the row key, as an alternative to the default row index mechanism. */ rowKey?: keyof T | ((record: T) => string | number) /** Class to append to each row. */ - rowClassName?: string | ((record: T) => string) + rowClassName?: string | ((record: T) => string | null) /** Color to mark each row with. */ rowRibbonColor?: string | ((record: T) => string | null) /** Status of each row. Defaults no status. */ diff --git a/frontend/src/lib/components/LemonTable/TableRow.tsx b/frontend/src/lib/components/LemonTable/TableRow.tsx index 41086c9f38fde..e5fe0d3b12cba 100644 --- a/frontend/src/lib/components/LemonTable/TableRow.tsx +++ b/frontend/src/lib/components/LemonTable/TableRow.tsx @@ -8,7 +8,7 @@ export interface TableRowProps> { record: T recordIndex: number rowKeyDetermined: string | number - rowClassNameDetermined: string | undefined + rowClassNameDetermined: string | null | undefined rowRibbonColorDetermined: string | null | undefined rowStatusDetermined: 'success' | 'warning' | 'danger' | 'highlighted' | undefined columnGroups: LemonTableColumnGroup[] diff --git a/frontend/src/lib/components/icons.tsx b/frontend/src/lib/components/icons.tsx index 6e7cd46a68b81..a712d1ae002b5 100644 --- a/frontend/src/lib/components/icons.tsx +++ b/frontend/src/lib/components/icons.tsx @@ -437,9 +437,9 @@ export function IconGroupedEvents(props: React.SVGProps): JSX.Ele } /** Material Design Assistant Photo icon. */ -export function IconFlag(): JSX.Element { +export function IconFlag(props: React.SVGProps): JSX.Element { return ( - + >({ (stepsInBreakdown[stepsInBreakdown.length - 1]?.count ?? 0) / (stepsInBreakdown[0]?.count ?? 1), }, - significant: stepsInBreakdown.some((step) => - step.significant ? Object.values(step.significant).some((val) => val) : false + significant: stepsInBreakdown.some( + (step) => step.significant?.total || step.significant?.fromBasisStep ), }) }) diff --git a/frontend/src/scenes/insights/Insight.scss b/frontend/src/scenes/insights/Insight.scss index 2c9e8c96f795f..5a9908ede2bc2 100644 --- a/frontend/src/scenes/insights/Insight.scss +++ b/frontend/src/scenes/insights/Insight.scss @@ -286,3 +286,12 @@ $funnel_canvas_background: #fff; } } } + +.significance-highlight { + display: inline-flex; + background: var(--primary); + color: var(--bg-light); + .LemonRow__icon { + color: var(--bg-light); + } +} diff --git a/frontend/src/scenes/insights/InsightTabs/FunnelTab/FunnelStepsTable.tsx b/frontend/src/scenes/insights/InsightTabs/FunnelTab/FunnelStepsTable.tsx index 674550f191497..83f596d1ffadd 100644 --- a/frontend/src/scenes/insights/InsightTabs/FunnelTab/FunnelStepsTable.tsx +++ b/frontend/src/scenes/insights/InsightTabs/FunnelTab/FunnelStepsTable.tsx @@ -6,7 +6,7 @@ import { LemonTable, LemonTableColumn, LemonTableColumnGroup } from 'lib/compone import { BreakdownKeyType, FlattenedFunnelStepByBreakdown } from '~/types' import { EntityFilterInfo } from 'lib/components/EntityFilterInfo' import { formatDisplayPercentage, getSeriesColor, getVisibilityIndex } from 'scenes/funnels/funnelUtils' -import { getActionFilterFromFunnelStep } from './funnelStepTableUtils' +import { getActionFilterFromFunnelStep, getSignificanceFromBreakdownStep } from './funnelStepTableUtils' import { formatBreakdownLabel } from 'scenes/insights/InsightsTable/InsightsTable' import { cohortsModel } from '~/models/cohortsModel' import { LemonCheckbox } from 'lib/components/LemonCheckbox' @@ -14,6 +14,7 @@ import { Lettermark, LettermarkColor } from 'lib/components/Lettermark/Lettermar import { LemonRow } from 'lib/components/LemonRow' import { humanFriendlyDuration } from 'lib/utils' import { ValueInspectorButton } from 'scenes/funnels/FunnelBarGraph' +import { IconFlag } from 'lib/components/icons' export function FunnelStepsTable(): JSX.Element | null { const { insightProps } = useValues(insightLogic) @@ -92,8 +93,23 @@ export function FunnelStepsTable(): JSX.Element | null { }, { title: 'Total conversion', - render: (_: void, breakdown: FlattenedFunnelStepByBreakdown) => - formatDisplayPercentage(breakdown?.conversionRates?.total ?? 0, true), + render: function RenderTotalConversion( + _: void, + breakdown: FlattenedFunnelStepByBreakdown + ): JSX.Element | string { + return getSignificanceFromBreakdownStep(breakdown, 0)?.total ? ( + } + compact + > + {formatDisplayPercentage(breakdown?.conversionRates?.total ?? 0, true)} + + ) : ( + formatDisplayPercentage(breakdown?.conversionRates?.total ?? 0, true) + ) + }, align: 'right', }, ], @@ -132,8 +148,29 @@ export function FunnelStepsTable(): JSX.Element | null { }, { title: 'Rate', - render: (_: void, breakdown: FlattenedFunnelStepByBreakdown) => - formatDisplayPercentage(breakdown.steps?.[stepIndex]?.conversionRates.fromPrevious ?? 0, true), + render: function RenderRate( + _: void, + breakdown: FlattenedFunnelStepByBreakdown + ): JSX.Element | string { + return getSignificanceFromBreakdownStep(breakdown, step.order)?.fromBasisStep ? ( + } + compact + > + {formatDisplayPercentage( + breakdown.steps?.[step.order]?.conversionRates.fromBasisStep ?? 0, + true + )} + + ) : ( + formatDisplayPercentage( + breakdown.steps?.[step.order]?.conversionRates.fromBasisStep ?? 0, + true + ) + ) + }, align: 'right', }, ...(stepIndex === 0 @@ -165,7 +202,7 @@ export function FunnelStepsTable(): JSX.Element | null { title: 'Rate', render: (_: void, breakdown: FlattenedFunnelStepByBreakdown) => formatDisplayPercentage( - 1 - (breakdown.steps?.[stepIndex]?.conversionRates.fromPrevious ?? 0), + 1 - (breakdown.steps?.[stepIndex]?.conversionRates.fromBasisStep ?? 0), true ), align: 'right', @@ -189,6 +226,7 @@ export function FunnelStepsTable(): JSX.Element | null { dataSource={flattenedBreakdowns} columns={columnsGrouped} loading={insightLoading} + rowKey="breakdownIndex" rowRibbonColor={(series) => getSeriesColor( series?.breakdownIndex, @@ -197,7 +235,7 @@ export function FunnelStepsTable(): JSX.Element | null { flattenedBreakdowns.length ) } - rowKey="breakdownIndex" + rowStatus={(record) => (record.significant ? 'highlighted' : undefined)} /> ) } From 257354a1cfcfa2f62c45033d373a1d93ded42445 Mon Sep 17 00:00:00 2001 From: Michael Matloka Date: Tue, 26 Apr 2022 16:25:10 +0200 Subject: [PATCH 2/5] Restore `DEVIATION_SIGNIFICANCE_MULTIPLIER` --- frontend/src/scenes/funnels/funnelLogic.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frontend/src/scenes/funnels/funnelLogic.ts b/frontend/src/scenes/funnels/funnelLogic.ts index c45d66fffb596..1fcc8677d138b 100644 --- a/frontend/src/scenes/funnels/funnelLogic.ts +++ b/frontend/src/scenes/funnels/funnelLogic.ts @@ -69,7 +69,7 @@ import { lemonToast } from 'lib/components/lemonToast' * Assuming a normal distribution, then 90% of values are within 1.5 standard deviations of the mean * which gives a ballpark of 1 highlighting every 10 breakdown values */ -const DEVIATION_SIGNIFICANCE_MULTIPLIER = 1.2 +const DEVIATION_SIGNIFICANCE_MULTIPLIER = 1.5 // List of events that should be excluded, if we don't have an explicit list of // excluded properties. Copied from From 98f37024a7b3fc62fb72abb651d950bcf2934609 Mon Sep 17 00:00:00 2001 From: Michael Matloka Date: Tue, 26 Apr 2022 16:25:34 +0200 Subject: [PATCH 3/5] Reorder props --- .../scenes/insights/InsightTabs/FunnelTab/FunnelStepsTable.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frontend/src/scenes/insights/InsightTabs/FunnelTab/FunnelStepsTable.tsx b/frontend/src/scenes/insights/InsightTabs/FunnelTab/FunnelStepsTable.tsx index 83f596d1ffadd..ae581017ad493 100644 --- a/frontend/src/scenes/insights/InsightTabs/FunnelTab/FunnelStepsTable.tsx +++ b/frontend/src/scenes/insights/InsightTabs/FunnelTab/FunnelStepsTable.tsx @@ -227,6 +227,7 @@ export function FunnelStepsTable(): JSX.Element | null { columns={columnsGrouped} loading={insightLoading} rowKey="breakdownIndex" + rowStatus={(record) => (record.significant ? 'highlighted' : undefined)} rowRibbonColor={(series) => getSeriesColor( series?.breakdownIndex, @@ -235,7 +236,6 @@ export function FunnelStepsTable(): JSX.Element | null { flattenedBreakdowns.length ) } - rowStatus={(record) => (record.significant ? 'highlighted' : undefined)} /> ) } From dcc10cd4df679ebcd0f5af10495030c22799d677 Mon Sep 17 00:00:00 2001 From: Michael Matloka Date: Tue, 26 Apr 2022 17:36:50 +0200 Subject: [PATCH 4/5] Fix `total` vs `fromBasisStep` confusion --- .../lib/components/LemonTable/LemonTable.scss | 1 - frontend/src/scenes/funnels/funnelLogic.ts | 2 +- .../FunnelTab/FunnelStepsTable.tsx | 104 +++++++++--------- frontend/src/styles/global.scss | 4 - 4 files changed, 55 insertions(+), 56 deletions(-) diff --git a/frontend/src/lib/components/LemonTable/LemonTable.scss b/frontend/src/lib/components/LemonTable/LemonTable.scss index e3b0ee9e228d6..bcdcfe97ff051 100644 --- a/frontend/src/lib/components/LemonTable/LemonTable.scss +++ b/frontend/src/lib/components/LemonTable/LemonTable.scss @@ -50,7 +50,6 @@ > th { font-weight: 700; text-align: left; - white-space: pre-wrap; } &.LemonTable__th--column-group { --row-base-height: 2.5rem; // Make group headers smaller for better hierarchy diff --git a/frontend/src/scenes/funnels/funnelLogic.ts b/frontend/src/scenes/funnels/funnelLogic.ts index 1fcc8677d138b..b84643e5f2636 100644 --- a/frontend/src/scenes/funnels/funnelLogic.ts +++ b/frontend/src/scenes/funnels/funnelLogic.ts @@ -827,7 +827,7 @@ export const funnelLogic = kea>({ (stepsInBreakdown[0]?.count ?? 1), }, significant: stepsInBreakdown.some( - (step) => step.significant?.total || step.significant?.fromBasisStep + (step) => step.significant?.total || step.significant?.fromPrevious ), }) }) diff --git a/frontend/src/scenes/insights/InsightTabs/FunnelTab/FunnelStepsTable.tsx b/frontend/src/scenes/insights/InsightTabs/FunnelTab/FunnelStepsTable.tsx index ae581017ad493..763deb1fe4bb7 100644 --- a/frontend/src/scenes/insights/InsightTabs/FunnelTab/FunnelStepsTable.tsx +++ b/frontend/src/scenes/insights/InsightTabs/FunnelTab/FunnelStepsTable.tsx @@ -93,23 +93,8 @@ export function FunnelStepsTable(): JSX.Element | null { }, { title: 'Total conversion', - render: function RenderTotalConversion( - _: void, - breakdown: FlattenedFunnelStepByBreakdown - ): JSX.Element | string { - return getSignificanceFromBreakdownStep(breakdown, 0)?.total ? ( - } - compact - > - {formatDisplayPercentage(breakdown?.conversionRates?.total ?? 0, true)} - - ) : ( - formatDisplayPercentage(breakdown?.conversionRates?.total ?? 0, true) - ) - }, + render: (_: void, breakdown: FlattenedFunnelStepByBreakdown) => + formatDisplayPercentage(breakdown?.conversionRates?.total ?? 0, true), align: 'right', }, ], @@ -126,7 +111,7 @@ export function FunnelStepsTable(): JSX.Element | null { ), children: [ { - title: 'Completed', + title: stepIndex === 0 ? 'Entered' : 'Converted', render: function RenderCompleted( _: void, breakdown: FlattenedFunnelStepByBreakdown @@ -146,13 +131,39 @@ export function FunnelStepsTable(): JSX.Element | null { align: 'right', }, + ...(stepIndex === 0 + ? [] + : [ + { + title: 'Dropped off', + render: function RenderDropped( + _: void, + breakdown: FlattenedFunnelStepByBreakdown + ): JSX.Element | undefined { + const stepSeries = breakdown.steps?.[stepIndex] + return ( + stepSeries && ( + + openPersonsModalForStep({ step: stepSeries, converted: false }) + } + style={{ padding: 0 }} + > + {stepSeries.droppedOffFromPrevious ?? 0} + + ) + ) + }, + align: 'right', + }, + ]), { - title: 'Rate', + title: 'Conversion so far', render: function RenderRate( _: void, breakdown: FlattenedFunnelStepByBreakdown ): JSX.Element | string { - return getSignificanceFromBreakdownStep(breakdown, step.order)?.fromBasisStep ? ( + return getSignificanceFromBreakdownStep(breakdown, step.order)?.total ? ( {formatDisplayPercentage( - breakdown.steps?.[step.order]?.conversionRates.fromBasisStep ?? 0, + breakdown.steps?.[step.order]?.conversionRates.total ?? 0, true )} ) : ( - formatDisplayPercentage( - breakdown.steps?.[step.order]?.conversionRates.fromBasisStep ?? 0, - true - ) + formatDisplayPercentage(breakdown.steps?.[step.order]?.conversionRates.total ?? 0, true) ) }, align: 'right', @@ -177,44 +185,40 @@ export function FunnelStepsTable(): JSX.Element | null { ? [] : [ { - title: 'Dropped', - render: function RenderDropped( + title: 'Conversion from previous', + render: function RenderRate( _: void, breakdown: FlattenedFunnelStepByBreakdown - ): JSX.Element | undefined { - const stepSeries = breakdown.steps?.[stepIndex] - return ( - stepSeries && ( - - openPersonsModalForStep({ step: stepSeries, converted: false }) - } - style={{ padding: 0 }} - > - {stepSeries.droppedOffFromPrevious ?? 0} - + ): JSX.Element | string { + return getSignificanceFromBreakdownStep(breakdown, step.order)?.fromPrevious ? ( + } + compact + > + {formatDisplayPercentage( + breakdown.steps?.[step.order]?.conversionRates.fromPrevious ?? 0, + true + )} + + ) : ( + formatDisplayPercentage( + breakdown.steps?.[step.order]?.conversionRates.fromPrevious ?? 0, + true ) ) }, align: 'right', }, { - title: 'Rate', - render: (_: void, breakdown: FlattenedFunnelStepByBreakdown) => - formatDisplayPercentage( - 1 - (breakdown.steps?.[stepIndex]?.conversionRates.fromBasisStep ?? 0), - true - ), - align: 'right', - }, - { - title: 'Avg. time', + title: 'Avg. time', render: (_: void, breakdown: FlattenedFunnelStepByBreakdown) => breakdown.steps?.[step.order]?.average_conversion_time != undefined ? humanFriendlyDuration(breakdown.steps[step.order].average_conversion_time, 3) : '–', align: 'right', - className: 'nowrap', + width: 0, }, ]), ] as LemonTableColumn[], diff --git a/frontend/src/styles/global.scss b/frontend/src/styles/global.scss index bc18713d65a49..dea8f4b1dd814 100644 --- a/frontend/src/styles/global.scss +++ b/frontend/src/styles/global.scss @@ -131,10 +131,6 @@ body strong { font-size: 8px; } -.nowrap { - white-space: nowrap !important; -} - .page-title-row { display: flex; flex-wrap: wrap; From b384332a539b4513117cd7ba4c33af67bd861197 Mon Sep 17 00:00:00 2001 From: Michael Matloka Date: Tue, 26 Apr 2022 18:43:27 +0200 Subject: [PATCH 5/5] Don't flag twice --- .../insights/InsightTabs/FunnelTab/FunnelStepsTable.tsx | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/frontend/src/scenes/insights/InsightTabs/FunnelTab/FunnelStepsTable.tsx b/frontend/src/scenes/insights/InsightTabs/FunnelTab/FunnelStepsTable.tsx index 763deb1fe4bb7..49e31165b99a1 100644 --- a/frontend/src/scenes/insights/InsightTabs/FunnelTab/FunnelStepsTable.tsx +++ b/frontend/src/scenes/insights/InsightTabs/FunnelTab/FunnelStepsTable.tsx @@ -163,7 +163,8 @@ export function FunnelStepsTable(): JSX.Element | null { _: void, breakdown: FlattenedFunnelStepByBreakdown ): JSX.Element | string { - return getSignificanceFromBreakdownStep(breakdown, step.order)?.total ? ( + const significance = getSignificanceFromBreakdownStep(breakdown, step.order) + return significance?.total ? (