From 33ffaee5d793a9a5522b19550be126d22c1feb75 Mon Sep 17 00:00:00 2001 From: Sandy Spicer Date: Tue, 30 Apr 2024 01:49:08 +0200 Subject: [PATCH] feat: Improved Legend for Lifecycle charts (#21915) --- .../InsightQuery/utils/filtersToQueryNode.ts | 1 + .../InsightQuery/utils/queryNodeToFilter.ts | 2 ++ .../nodes/InsightViz/InsightDisplayConfig.tsx | 4 +-- .../src/queries/nodes/InsightViz/utils.ts | 4 ++- frontend/src/queries/schema.json | 6 ++++ frontend/src/queries/schema.ts | 1 + .../EditorFilters/ValueOnSeriesFilter.tsx | 8 ++--- .../insights/InsightNav/insightNavLogic.tsx | 18 +++++------- .../scenes/insights/insightVizDataLogic.ts | 12 ++++---- .../insights/views/LineGraph/LineGraph.tsx | 21 +++++++------- .../insights/views/LineGraph/PieChart.tsx | 4 +-- frontend/src/scenes/surveys/surveyViewViz.tsx | 7 ++--- frontend/src/scenes/trends/trendsDataLogic.ts | 3 +- .../trends/viz/ActionsHorizontalBar.tsx | 4 +-- .../scenes/trends/viz/ActionsLineGraph.tsx | 29 +++++++++++++++++-- frontend/src/scenes/trends/viz/ActionsPie.tsx | 4 +-- frontend/src/types.ts | 1 + .../legacy_compatibility/filter_to_query.py | 1 + posthog/schema.py | 2 ++ 19 files changed, 84 insertions(+), 48 deletions(-) diff --git a/frontend/src/queries/nodes/InsightQuery/utils/filtersToQueryNode.ts b/frontend/src/queries/nodes/InsightQuery/utils/filtersToQueryNode.ts index ee5d2af61f5ce..4460b987041cf 100644 --- a/frontend/src/queries/nodes/InsightQuery/utils/filtersToQueryNode.ts +++ b/frontend/src/queries/nodes/InsightQuery/utils/filtersToQueryNode.ts @@ -433,6 +433,7 @@ export const stickinessFilterToQuery = (filters: Record): Stickines export const lifecycleFilterToQuery = (filters: Record): LifecycleFilter => { return objectCleanWithEmpty({ + showLegend: filters.show_legend, toggledLifecycles: filters.toggledLifecycles, showValuesOnSeries: filters.show_values_on_series, }) diff --git a/frontend/src/queries/nodes/InsightQuery/utils/queryNodeToFilter.ts b/frontend/src/queries/nodes/InsightQuery/utils/queryNodeToFilter.ts index 1ff191df58c9a..e4aed1942bfc5 100644 --- a/frontend/src/queries/nodes/InsightQuery/utils/queryNodeToFilter.ts +++ b/frontend/src/queries/nodes/InsightQuery/utils/queryNodeToFilter.ts @@ -295,6 +295,8 @@ export const queryNodeToFilter = (query: InsightQueryNode): Partial delete queryCopy.stickinessFilter?.showValuesOnSeries } else if (isLifecycleQuery(queryCopy)) { camelCasedLifecycleProps.show_values_on_series = queryCopy.lifecycleFilter?.showValuesOnSeries + camelCasedLifecycleProps.show_legend = queryCopy.lifecycleFilter?.showLegend + delete queryCopy.lifecycleFilter?.showLegend delete queryCopy.lifecycleFilter?.showValuesOnSeries } Object.assign(filters, camelCasedTrendsProps) diff --git a/frontend/src/queries/nodes/InsightViz/InsightDisplayConfig.tsx b/frontend/src/queries/nodes/InsightViz/InsightDisplayConfig.tsx index 3d0ef02c729f7..2bb9c1b693b20 100644 --- a/frontend/src/queries/nodes/InsightViz/InsightDisplayConfig.tsx +++ b/frontend/src/queries/nodes/InsightViz/InsightDisplayConfig.tsx @@ -63,7 +63,7 @@ export function InsightDisplayConfig(): JSX.Element { !trendsFilter?.compare && (!display || display === ChartDisplayType.ActionsLineGraph) - const { showValueOnSeries, mightContainFractionalNumbers } = useValues(trendsDataLogic(insightProps)) + const { showValuesOnSeries, mightContainFractionalNumbers } = useValues(trendsDataLogic(insightProps)) const advancedOptions: LemonMenuItems = [ ...(supportsValueOnSeries || supportsPercentStackView || hasLegend @@ -96,7 +96,7 @@ export function InsightDisplayConfig(): JSX.Element { : []), ] const advancedOptionsCount: number = - (supportsValueOnSeries && showValueOnSeries ? 1 : 0) + + (supportsValueOnSeries && showValuesOnSeries ? 1 : 0) + (showPercentStackView ? 1 : 0) + (!showPercentStackView && isTrends && diff --git a/frontend/src/queries/nodes/InsightViz/utils.ts b/frontend/src/queries/nodes/InsightViz/utils.ts index 60d7aa143c0d2..1d46f68a57887 100644 --- a/frontend/src/queries/nodes/InsightViz/utils.ts +++ b/frontend/src/queries/nodes/InsightViz/utils.ts @@ -89,11 +89,13 @@ export const getShowLegend = (query: InsightQueryNode): boolean | undefined => { return query.stickinessFilter?.showLegend } else if (isTrendsQuery(query)) { return query.trendsFilter?.showLegend + } else if (isLifecycleQuery(query)) { + return query.lifecycleFilter?.showLegend } return undefined } -export const getShowValueOnSeries = (query: InsightQueryNode): boolean | undefined => { +export const getShowValuesOnSeries = (query: InsightQueryNode): boolean | undefined => { if (isLifecycleQuery(query)) { return query.lifecycleFilter?.showValuesOnSeries } else if (isStickinessQuery(query)) { diff --git a/frontend/src/queries/schema.json b/frontend/src/queries/schema.json index 02b74802864c2..784e381513325 100644 --- a/frontend/src/queries/schema.json +++ b/frontend/src/queries/schema.json @@ -3179,6 +3179,9 @@ "LifecycleFilter": { "additionalProperties": false, "properties": { + "showLegend": { + "type": "boolean" + }, "showValuesOnSeries": { "type": "boolean" }, @@ -3195,6 +3198,9 @@ "additionalProperties": false, "description": "`LifecycleFilterType` minus everything inherited from `FilterType`", "properties": { + "show_legend": { + "type": "boolean" + }, "show_values_on_series": { "type": "boolean" }, diff --git a/frontend/src/queries/schema.ts b/frontend/src/queries/schema.ts index c283f93a41ca7..d82d62176d416 100644 --- a/frontend/src/queries/schema.ts +++ b/frontend/src/queries/schema.ts @@ -860,6 +860,7 @@ export type LifecycleFilterLegacy = Omit { - updateInsightFilter({ showValuesOnSeries: checked }) + checked={!!showValuesOnSeries} + onChange={() => { + updateInsightFilter({ showValuesOnSeries: !showValuesOnSeries }) }} label={Show values on series} size="small" diff --git a/frontend/src/scenes/insights/InsightNav/insightNavLogic.tsx b/frontend/src/scenes/insights/InsightNav/insightNavLogic.tsx index 15a58a8c13e1d..22e0893446d97 100644 --- a/frontend/src/scenes/insights/InsightNav/insightNavLogic.tsx +++ b/frontend/src/scenes/insights/InsightNav/insightNavLogic.tsx @@ -10,7 +10,7 @@ import { filterTestAccountsDefaultsLogic } from 'scenes/settings/project/filterT import { examples, TotalEventsTable } from '~/queries/examples' import { insightMap } from '~/queries/nodes/InsightQuery/utils/queryNodeToFilter' -import { getDisplay, getShowPercentStackView, getShowValueOnSeries } from '~/queries/nodes/InsightViz/utils' +import { getDisplay, getShowPercentStackView, getShowValuesOnSeries } from '~/queries/nodes/InsightViz/utils' import { ActionsNode, DataWarehouseNode, @@ -87,10 +87,9 @@ const cleanSeriesEntityMath = ( } else if (mathAvailability === MathAvailability.ActorsOnly) { // return entity with default actors only availability math set return { ...baseEntity, math: BaseMathType.UniqueUsers } - } else { - // return entity without math properties for insights that don't support it - return baseEntity } + // return entity without math properties for insights that don't support it + return baseEntity } const cleanSeriesMath = ( @@ -154,15 +153,12 @@ export const insightNavLogic = kea([ return InsightType.SQL } else if (isInsightVizNode(query)) { return insightMap[query.source.kind] || InsightType.TRENDS - } else { - return InsightType.JSON } - } else { - return filters.insight || InsightType.TRENDS + return InsightType.JSON } - } else { - return userSelectedView + return filters.insight || InsightType.TRENDS } + return userSelectedView }, ], tabs: [ @@ -401,7 +397,7 @@ const mergeCachedProperties = (query: InsightQueryNode, cache: QueryPropertyCach // TODO: fix an issue where switching between trends and funnels with the option enabled would // result in an error before uncommenting // ...(getCompare(node) ? { compare: getCompare(node) } : {}), - ...(getShowValueOnSeries(node) ? { showValuesOnSeries: getShowValueOnSeries(node) } : {}), + ...(getShowValuesOnSeries(node) ? { showValuesOnSeries: getShowValuesOnSeries(node) } : {}), ...(getShowPercentStackView(node) ? { showPercentStackView: getShowPercentStackView(node) } : {}), ...(getDisplay(node) ? { display: getDisplay(node) } : {}), } diff --git a/frontend/src/scenes/insights/insightVizDataLogic.ts b/frontend/src/scenes/insights/insightVizDataLogic.ts index 4a4d94938e305..09405262dd56a 100644 --- a/frontend/src/scenes/insights/insightVizDataLogic.ts +++ b/frontend/src/scenes/insights/insightVizDataLogic.ts @@ -29,7 +29,7 @@ import { getShowLabelsOnSeries, getShowLegend, getShowPercentStackView, - getShowValueOnSeries, + getShowValuesOnSeries, } from '~/queries/nodes/InsightViz/utils' import { BreakdownFilter, @@ -166,7 +166,7 @@ export const insightVizDataLogic = kea([ properties: [(s) => [s.querySource], (q) => (q ? q.properties : null)], samplingFactor: [(s) => [s.querySource], (q) => (q ? q.samplingFactor : null)], showLegend: [(s) => [s.querySource], (q) => (q ? getShowLegend(q) : null)], - showValueOnSeries: [(s) => [s.querySource], (q) => (q ? getShowValueOnSeries(q) : null)], + showValuesOnSeries: [(s) => [s.querySource], (q) => (q ? getShowValuesOnSeries(q) : null)], showLabelOnSeries: [(s) => [s.querySource], (q) => (q ? getShowLabelsOnSeries(q) : null)], showPercentStackView: [(s) => [s.querySource], (q) => (q ? getShowPercentStackView(q) : null)], vizSpecificOptions: [(s) => [s.query], (q: Node) => (isInsightVizNode(q) ? q.vizSpecificOptions : null)], @@ -270,10 +270,10 @@ export const insightVizDataLogic = kea([ ], hasLegend: [ - (s) => [s.isTrends, s.isStickiness, s.display], - (isTrends, isStickiness, display) => - (isTrends || isStickiness) && - !DISPLAY_TYPES_WITHOUT_LEGEND.includes(display || ChartDisplayType.ActionsLineGraph), + (s) => [s.isTrends, s.isStickiness, s.isLifecycle, s.display], + (isTrends, isStickiness, isLifecycle, display) => + (isTrends || isStickiness || isLifecycle) && + !(display && DISPLAY_TYPES_WITHOUT_LEGEND.includes(display)), ], hasFormula: [(s) => [s.formula], (formula) => formula !== undefined], diff --git a/frontend/src/scenes/insights/views/LineGraph/LineGraph.tsx b/frontend/src/scenes/insights/views/LineGraph/LineGraph.tsx index bc74a14c89d09..99f5b788e5337 100644 --- a/frontend/src/scenes/insights/views/LineGraph/LineGraph.tsx +++ b/frontend/src/scenes/insights/views/LineGraph/LineGraph.tsx @@ -1,5 +1,7 @@ import 'chartjs-adapter-dayjs-3' +import { LegendOptions } from 'chart.js' +import { _DeepPartialObject } from 'chart.js/types/utils' import ChartDataLabels from 'chartjs-plugin-datalabels' import ChartjsPluginStacked100, { ExtendedChartData } from 'chartjs-plugin-stacked100' import clsx from 'clsx' @@ -62,9 +64,8 @@ export function ensureTooltip(): [Root, HTMLElement] { function truncateString(str: string, num: number): string { if (str.length > num) { return str.slice(0, num) + ' ...' - } else { - return str } + return str } export function onChartClick( @@ -230,12 +231,13 @@ export interface LineGraphProps { trendsFilter?: TrendsFilter | null formula?: string | null compare?: boolean | null - showValueOnSeries?: boolean | null + showValuesOnSeries?: boolean | null showPercentStackView?: boolean | null supportsPercentStackView?: boolean hideAnnotations?: boolean hideXAxis?: boolean hideYAxis?: boolean + legend?: _DeepPartialObject> } export const LineGraph = (props: LineGraphProps): JSX.Element => { @@ -263,12 +265,13 @@ export function LineGraph_({ labelGroupType, trendsFilter, formula, - showValueOnSeries, + showValuesOnSeries, showPercentStackView, supportsPercentStackView, hideAnnotations, hideXAxis, hideYAxis, + legend, }: LineGraphProps): JSX.Element { let datasets = _datasets @@ -422,10 +425,10 @@ export function LineGraph_({ }, display: (context) => { const datum = context.dataset.data[context.dataIndex] - if (showValueOnSeries && inSurveyView) { + if (showValuesOnSeries && inSurveyView) { return true } - return showValueOnSeries === true && typeof datum === 'number' && datum !== 0 ? 'auto' : false + return showValuesOnSeries === true && typeof datum === 'number' && datum !== 0 ? 'auto' : false }, formatter: (value: number, context) => { const data = context.chart.data as ExtendedChartData @@ -437,9 +440,7 @@ export function LineGraph_({ borderRadius: 4, borderColor: 'white', }, - legend: { - display: false, - }, + legend: legend, tooltip: { ...tooltipOptions, external({ tooltip }: { chart: Chart; tooltip: TooltipModel }) { @@ -739,7 +740,7 @@ export function LineGraph_({ }) setMyLineChart(newChart) return () => newChart.destroy() - }, [datasets, hiddenLegendKeys, isDarkModeOn, trendsFilter, formula, showValueOnSeries, showPercentStackView]) + }, [datasets, hiddenLegendKeys, isDarkModeOn, trendsFilter, formula, showValuesOnSeries, showPercentStackView]) return (
diff --git a/frontend/src/scenes/insights/views/LineGraph/PieChart.tsx b/frontend/src/scenes/insights/views/LineGraph/PieChart.tsx index 3e08f7771372d..b6a1613fd6a79 100644 --- a/frontend/src/scenes/insights/views/LineGraph/PieChart.tsx +++ b/frontend/src/scenes/insights/views/LineGraph/PieChart.tsx @@ -66,7 +66,7 @@ export function PieChart({ ['data-attr']: dataAttr, trendsFilter, formula, - showValueOnSeries, + showValuesOnSeries, showLabelOnSeries, supportsPercentStackView, showPercentStackView, @@ -145,7 +145,7 @@ export function PieChart({ }, display: (context) => { const percentage = getPercentageForDataPoint(context) - const showValueForSeries = showValueOnSeries !== false && context.dataset.data.length > 1 // show if true or unset + const showValueForSeries = showValuesOnSeries !== false && context.dataset.data.length > 1 // show if true or unset return (showValueForSeries || showLabelOnSeries) && percentage > 5 ? 'auto' : false }, padding: (context) => { diff --git a/frontend/src/scenes/surveys/surveyViewViz.tsx b/frontend/src/scenes/surveys/surveyViewViz.tsx index 580fdaf1ae433..90f9ff4974533 100644 --- a/frontend/src/scenes/surveys/surveyViewViz.tsx +++ b/frontend/src/scenes/surveys/surveyViewViz.tsx @@ -206,7 +206,7 @@ export function RatingQuestionBarChart({ {surveySingleChoiceResults[questionIndex].data.map((count: number, i: number) => { @@ -417,7 +416,7 @@ export function MultipleChoiceQuestionBarChart({ inSurveyView={true} hideYAxis={true} hideXAxis={true} - showValueOnSeries={true} + showValuesOnSeries={true} labelGroupType={1} data-attr="survey-multiple-choice" type={GraphType.HorizontalBar} diff --git a/frontend/src/scenes/trends/trendsDataLogic.ts b/frontend/src/scenes/trends/trendsDataLogic.ts index cd85bbcb204a1..14d09586c9b58 100644 --- a/frontend/src/scenes/trends/trendsDataLogic.ts +++ b/frontend/src/scenes/trends/trendsDataLogic.ts @@ -52,7 +52,7 @@ export const trendsDataLogic = kea([ 'compare', 'interval', 'breakdownFilter', - 'showValueOnSeries', + 'showValuesOnSeries', 'showLabelOnSeries', 'showPercentStackView', 'supportsPercentStackView', @@ -65,6 +65,7 @@ export const trendsDataLogic = kea([ 'isNonTimeSeriesDisplay', 'isSingleSeries', 'hasLegend', + 'showLegend', 'vizSpecificOptions', ], ], diff --git a/frontend/src/scenes/trends/viz/ActionsHorizontalBar.tsx b/frontend/src/scenes/trends/viz/ActionsHorizontalBar.tsx index a8dc5b7a80928..9bc96ff2ceb26 100644 --- a/frontend/src/scenes/trends/viz/ActionsHorizontalBar.tsx +++ b/frontend/src/scenes/trends/viz/ActionsHorizontalBar.tsx @@ -32,7 +32,7 @@ export function ActionsHorizontalBar({ showPersonsModal = true }: ChartParams): const { insightProps, hiddenLegendKeys } = useValues(insightLogic) const { featureFlags } = useValues(featureFlagLogic) const { isTrends, query } = useValues(insightVizDataLogic(insightProps)) - const { indexedResults, labelGroupType, trendsFilter, formula, showValueOnSeries, isDataWarehouseSeries } = + const { indexedResults, labelGroupType, trendsFilter, formula, showValuesOnSeries, isDataWarehouseSeries } = useValues(trendsDataLogic(insightProps)) function updateData(): void { @@ -95,7 +95,7 @@ export function ActionsHorizontalBar({ showPersonsModal = true }: ChartParams): showPersonsModal={showPersonsModal} trendsFilter={trendsFilter} formula={formula} - showValueOnSeries={showValueOnSeries} + showValuesOnSeries={showValuesOnSeries} onClick={ !showPersonsModal || trendsFilter?.formula || isDataWarehouseSeries ? undefined diff --git a/frontend/src/scenes/trends/viz/ActionsLineGraph.tsx b/frontend/src/scenes/trends/viz/ActionsLineGraph.tsx index 268e2054ce4cf..f06cd6a6fda84 100644 --- a/frontend/src/scenes/trends/viz/ActionsLineGraph.tsx +++ b/frontend/src/scenes/trends/viz/ActionsLineGraph.tsx @@ -1,4 +1,7 @@ +import { ChartType, defaults, LegendOptions } from 'chart.js' +import { _DeepPartialObject } from 'chart.js/types/utils' import { useValues } from 'kea' +import { Chart } from 'lib/Chart' import { DateDisplay } from 'lib/components/DateDisplay' import { PropertyKeyInfo } from 'lib/components/PropertyKeyInfo' import { FEATURE_FLAGS } from 'lib/constants' @@ -37,7 +40,7 @@ export function ActionsLineGraph({ compare, display, interval, - showValueOnSeries, + showValuesOnSeries, showPercentStackView, supportsPercentStackView, trendsFilter, @@ -45,6 +48,7 @@ export function ActionsLineGraph({ isStickiness, isTrends, isDataWarehouseSeries, + showLegend, } = useValues(trendsDataLogic(insightProps)) const labels = @@ -75,6 +79,24 @@ export function ActionsLineGraph({ isInsightVizNode(query) && isTrendsQuery(query.source) + const shortenLifecycleLabels = (s: string | undefined): string => + capitalizeFirstLetter(s?.split(' - ')?.[1] ?? s ?? 'None') + + const legend: _DeepPartialObject> = { + display: !!showLegend, + } + if (isLifecycle) { + legend.labels = { + generateLabels: (chart: Chart) => { + const labelElements = defaults.plugins.legend.labels.generateLabels(chart) + labelElements.forEach((elt) => { + elt.text = shortenLifecycleLabels(elt.text) + }) + return labelElements + }, + } + } + if ( !(indexedResults && indexedResults[0]?.data && indexedResults.filter((result) => result.count !== 0).length > 0) ) { @@ -93,7 +115,7 @@ export function ActionsLineGraph({ showPersonsModal={showPersonsModal} trendsFilter={trendsFilter} formula={formula} - showValueOnSeries={showValueOnSeries} + showValuesOnSeries={showValuesOnSeries} showPercentStackView={showPercentStackView} supportsPercentStackView={supportsPercentStackView} tooltip={ @@ -104,7 +126,7 @@ export function ActionsLineGraph({ return date }, renderSeries: (_, datum) => { - return capitalizeFirstLetter(datum.label?.split(' - ')?.[1] ?? datum.label ?? 'None') + return shortenLifecycleLabels(datum.label) }, } : undefined @@ -113,6 +135,7 @@ export function ActionsLineGraph({ isInProgress={!isStickiness && incompletenessOffsetFromEnd < 0} isArea={display === ChartDisplayType.ActionsAreaGraph} incompletenessOffsetFromEnd={incompletenessOffsetFromEnd} + legend={legend} onClick={ !showPersonsModal || isMultiSeriesFormula(formula) || isDataWarehouseSeries ? undefined diff --git a/frontend/src/scenes/trends/viz/ActionsPie.tsx b/frontend/src/scenes/trends/viz/ActionsPie.tsx index 480939739a228..2539b7cae8115 100644 --- a/frontend/src/scenes/trends/viz/ActionsPie.tsx +++ b/frontend/src/scenes/trends/viz/ActionsPie.tsx @@ -42,7 +42,7 @@ export function ActionsPie({ labelGroupType, trendsFilter, formula, - showValueOnSeries, + showValuesOnSeries, showLabelOnSeries, supportsPercentStackView, showPercentStackView, @@ -147,7 +147,7 @@ export function ActionsPie({ showPersonsModal={showPersonsModal} trendsFilter={trendsFilter} formula={formula} - showValueOnSeries={showValueOnSeries} + showValuesOnSeries={showValuesOnSeries} showLabelOnSeries={showLabelOnSeries} supportsPercentStackView={supportsPercentStackView} showPercentStackView={showPercentStackView} diff --git a/frontend/src/types.ts b/frontend/src/types.ts index b5b420ebf1ada..b8f5d63ca0a34 100644 --- a/frontend/src/types.ts +++ b/frontend/src/types.ts @@ -2095,6 +2095,7 @@ export interface LifecycleFilterType extends FilterType { shown_as?: ShownAsValue // frontend only + show_legend?: boolean show_values_on_series?: boolean toggledLifecycles?: LifecycleToggle[] } diff --git a/posthog/hogql_queries/legacy_compatibility/filter_to_query.py b/posthog/hogql_queries/legacy_compatibility/filter_to_query.py index fdeb74fc9076f..e9528499222d2 100644 --- a/posthog/hogql_queries/legacy_compatibility/filter_to_query.py +++ b/posthog/hogql_queries/legacy_compatibility/filter_to_query.py @@ -366,6 +366,7 @@ def _insight_filter(filter: dict): insight_filter = { "lifecycleFilter": LifecycleFilter( toggledLifecycles=filter.get("toggledLifecycles"), + showLegend=filter.get("show_legend"), showValuesOnSeries=filter.get("show_values_on_series"), ) } diff --git a/posthog/schema.py b/posthog/schema.py index 44de044357cbe..ea349fe8fff8e 100644 --- a/posthog/schema.py +++ b/posthog/schema.py @@ -1349,6 +1349,7 @@ class LifecycleFilter(BaseModel): model_config = ConfigDict( extra="forbid", ) + showLegend: Optional[bool] = None showValuesOnSeries: Optional[bool] = None toggledLifecycles: Optional[list[LifecycleToggle]] = None @@ -1357,6 +1358,7 @@ class LifecycleFilterLegacy(BaseModel): model_config = ConfigDict( extra="forbid", ) + show_legend: Optional[bool] = None show_values_on_series: Optional[bool] = None toggledLifecycles: Optional[list[LifecycleToggle]] = None