fix(funnel): always use total step count for funnel chart label #14993
Conversation
|
Followup: The experience is weird when you deselect specific breakdowns from the visualization. We're not subtracting from the aggregate which maybe we should as it leads to a confusing visualization |
📸 UI snapshots have been updated1 snapshot changes in total. 0 added, 1 modified, 0 deleted:
Triggered by this commit. |
| @@ -114,14 +114,14 @@ export function FunnelBarChartComponent({ | |||
| <td key={stepIndex}> | |||
| {isUsingDataExploration ? ( | |||
| <StepLegendDataExploration | |||
| step={step.nested_breakdown?.length ? step.nested_breakdown[0] : step} | |||
| step={step} | |||
There was a problem hiding this comment.
wow this is clever!
I'm slightly scared this doesn't work for other existing use cases, but not sure, basic ones seem to all work
There was a problem hiding this comment.
it should mirror how the logic is handled on horiztonal bar charts. (those don't have the same bug)
There was a problem hiding this comment.
I hope this won't break anything but I think it might but let's see xD (why did we have step.nested_breakdown[0] originally ? )
There was a problem hiding this comment.
I would assume for a funnel like this https://app.posthog.com/insights/IlZLZRIH it might break but I could be wrong
There was a problem hiding this comment.
This PR is literally a pure revert of this one: #10750 – so I guess this might break the persons modal link. Can you give this a check?
There was a problem hiding this comment.
Awesome, seems to be correct. Let's replace this Chesterton's fence
| @@ -114,14 +114,14 @@ export function FunnelBarChartComponent({ | |||
| <td key={stepIndex}> | |||
| {isUsingDataExploration ? ( | |||
| <StepLegendDataExploration | |||
| step={step.nested_breakdown?.length ? step.nested_breakdown[0] : step} | |||
| step={step} | |||
There was a problem hiding this comment.
This PR is literally a pure revert of this one: #10750 – so I guess this might break the persons modal link. Can you give this a check?
| @@ -114,14 +114,14 @@ export function FunnelBarChartComponent({ | |||
| <td key={stepIndex}> | |||
| {isUsingDataExploration ? ( | |||
| <StepLegendDataExploration | |||
| step={step.nested_breakdown?.length ? step.nested_breakdown[0] : step} | |||
| step={step} | |||
There was a problem hiding this comment.
Awesome, seems to be correct. Let's replace this Chesterton's fence
* master: fix(flags): don't enclose in overall transaction so we get latest reads (#15003) fix(tests): make getEventsByPerson output stable to avoid flakes (#15009) feat(data-exploration): convert funnel correlation to data exploration (#14963) fix: Set hobby deployments to 'latest' by default (#14956) feat(hogql): lambdas (#14987) feat(hogql): arrays and tuples (#14986) fix(funnel): always use total step count for funnel chart label (#14993)
Problem
Changes
Before:
After:
👉 Stay up-to-date with PostHog coding conventions for a smoother review.
How did you test this code?