Skip to content

Commit

Permalink
fix(notebooks): fix insight updates in notebooks (part 2) (#18577)
Browse files Browse the repository at this point in the history
  • Loading branch information
thmsobrmlr authored Nov 15, 2023
1 parent 74c0309 commit 7f851ce
Show file tree
Hide file tree
Showing 6 changed files with 82 additions and 173 deletions.
16 changes: 10 additions & 6 deletions frontend/src/lib/components/CompareFilter/CompareFilter.tsx
Original file line number Diff line number Diff line change
@@ -1,12 +1,15 @@
import { useValues, useActions } from 'kea'
import { compareFilterLogic } from './compareFilterLogic'
import { insightLogic } from 'scenes/insights/insightLogic'
import { LemonCheckbox } from '@posthog/lemon-ui'
import { insightVizDataLogic } from 'scenes/insights/insightVizDataLogic'

export function CompareFilter(): JSX.Element | null {
const { insightProps } = useValues(insightLogic)
const { compare, disabled } = useValues(compareFilterLogic(insightProps))
const { setCompare } = useActions(compareFilterLogic(insightProps))
const { insightProps, canEditInsight } = useValues(insightLogic)

const { compare, supportsCompare } = useValues(insightVizDataLogic(insightProps))
const { updateInsightFilter } = useActions(insightVizDataLogic(insightProps))

const disabled: boolean = !canEditInsight || !supportsCompare

// Hide compare filter control when disabled to avoid states where control is "disabled but checked"
if (disabled) {
Expand All @@ -15,9 +18,10 @@ export function CompareFilter(): JSX.Element | null {

return (
<LemonCheckbox
onChange={setCompare}
onChange={(compare: boolean) => {
updateInsightFilter({ compare })
}}
checked={!!compare}
disabled={disabled}
label={<span className="font-normal">Compare to previous period</span>}
bordered
size="small"
Expand Down
43 changes: 0 additions & 43 deletions frontend/src/lib/components/CompareFilter/compareFilterLogic.ts

This file was deleted.

75 changes: 45 additions & 30 deletions frontend/src/queries/nodes/InsightViz/InsightDisplayConfig.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import { ReactNode } from 'react'
import { useValues } from 'kea'

import { insightLogic } from 'scenes/insights/insightLogic'
import { insightDisplayConfigLogic } from './insightDisplayConfigLogic'

import { InsightDateFilter } from 'scenes/insights/filters/InsightDateFilter'
import { IntervalFilter } from 'lib/components/IntervalFilter'
Expand All @@ -23,47 +22,63 @@ import { LemonButton } from '@posthog/lemon-ui'
import { axisLabel } from 'scenes/insights/aggregationAxisFormat'
import { ChartDisplayType } from '~/types'
import { ShowLegendFilter } from 'scenes/insights/EditorFilters/ShowLegendFilter'
import { FEATURE_FLAGS, NON_TIME_SERIES_DISPLAY_TYPES } from 'lib/constants'
import { insightVizDataLogic } from 'scenes/insights/insightVizDataLogic'
import { funnelDataLogic } from 'scenes/funnels/funnelDataLogic'
import { featureFlagLogic } from 'lib/logic/featureFlagLogic'

export function InsightDisplayConfig(): JSX.Element {
const { insightProps } = useValues(insightLogic)
const { featureFlags } = useValues(featureFlagLogic)

const {
showDateRange,
disableDateRange,
showCompare,
showValueOnSeries,
showPercentStackView,
showUnit,
showChart,
showInterval,
showSmoothing,
showRetention,
showPaths,
showFunnelDisplayLayout,
showFunnelBins,
isTrends,
isFunnels,
isRetention,
isPaths,
isStickiness,
isLifecycle,
supportsDisplay,
display,
breakdown,
trendsFilter,
hasLegend,
showLegend,
} = useValues(insightDisplayConfigLogic(insightProps))

const { showPercentStackView: isPercentStackViewOn, showValueOnSeries: isValueOnSeriesOn } = useValues(
trendsDataLogic(insightProps)
supportsValueOnSeries,
showPercentStackView,
} = useValues(insightVizDataLogic(insightProps))
const { isTrendsFunnel, isStepsFunnel, isTimeToConvertFunnel, isEmptyFunnel } = useValues(
funnelDataLogic(insightProps)
)

const showCompare = (isTrends && display !== ChartDisplayType.ActionsAreaGraph) || isStickiness
const showInterval =
isTrendsFunnel ||
isLifecycle ||
((isTrends || isStickiness) && !(display && NON_TIME_SERIES_DISPLAY_TYPES.includes(display)))
const showSmoothing =
isTrends &&
!breakdown?.breakdown_type &&
!trendsFilter?.compare &&
(!display || display === ChartDisplayType.ActionsLineGraph) &&
featureFlags[FEATURE_FLAGS.SMOOTHING_INTERVAL]

const { showPercentStackView: isPercentStackViewOn, showValueOnSeries } = useValues(trendsDataLogic(insightProps))

const advancedOptions: LemonMenuItems = [
...(showValueOnSeries || showPercentStackView || hasLegend
...(supportsValueOnSeries || showPercentStackView || hasLegend
? [
{
title: 'Display',
items: [
...(showValueOnSeries ? [{ label: () => <ValueOnSeriesFilter /> }] : []),
...(supportsValueOnSeries ? [{ label: () => <ValueOnSeriesFilter /> }] : []),
...(showPercentStackView ? [{ label: () => <PercentStackViewFilter /> }] : []),
...(hasLegend ? [{ label: () => <ShowLegendFilter /> }] : []),
],
},
]
: []),
...(!isPercentStackViewOn && showUnit
...(!isPercentStackViewOn && isTrends
? [
{
title: axisLabel(display || ChartDisplayType.ActionsLineGraph),
Expand All @@ -73,10 +88,10 @@ export function InsightDisplayConfig(): JSX.Element {
: []),
]
const advancedOptionsCount: number =
(showValueOnSeries && isValueOnSeriesOn ? 1 : 0) +
(supportsValueOnSeries && showValueOnSeries ? 1 : 0) +
(showPercentStackView && isPercentStackViewOn ? 1 : 0) +
(!isPercentStackViewOn &&
showUnit &&
isTrends &&
trendsFilter?.aggregation_axis_format &&
trendsFilter.aggregation_axis_format !== 'numeric'
? 1
Expand All @@ -86,9 +101,9 @@ export function InsightDisplayConfig(): JSX.Element {
return (
<div className="flex justify-between items-center flex-wrap" data-attr="insight-filters">
<div className="flex items-center gap-x-2 flex-wrap my-2 gap-y-2">
{showDateRange && (
{!isRetention && (
<ConfigFilter>
<InsightDateFilter disabled={disableDateRange} />
<InsightDateFilter disabled={isFunnels && !!isEmptyFunnel} />
</ConfigFilter>
)}

Expand All @@ -104,14 +119,14 @@ export function InsightDisplayConfig(): JSX.Element {
</ConfigFilter>
)}

{showRetention && (
{!!isRetention && (
<ConfigFilter>
<RetentionDatePicker />
<RetentionReferencePicker />
</ConfigFilter>
)}

{showPaths && (
{!!isPaths && (
<ConfigFilter>
<PathStepPicker />
</ConfigFilter>
Expand All @@ -133,17 +148,17 @@ export function InsightDisplayConfig(): JSX.Element {
</LemonButton>
</LemonMenu>
)}
{showChart && (
{supportsDisplay && (
<ConfigFilter>
<ChartFilter />
</ConfigFilter>
)}
{showFunnelDisplayLayout && (
{!!isStepsFunnel && (
<ConfigFilter>
<FunnelDisplayLayoutPicker />
</ConfigFilter>
)}
{showFunnelBins && (
{!!isTimeToConvertFunnel && (
<ConfigFilter>
<FunnelBinsPicker />
</ConfigFilter>
Expand Down
89 changes: 0 additions & 89 deletions frontend/src/queries/nodes/InsightViz/insightDisplayConfigLogic.ts

This file was deleted.

6 changes: 3 additions & 3 deletions frontend/src/scenes/insights/insightCommandLogic.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import { Command, commandPaletteLogic } from 'lib/components/CommandPalette/commandPaletteLogic'
import { kea, props, key, path, connect, events } from 'kea'
import type { insightCommandLogicType } from './insightCommandLogicType'
import { compareFilterLogic } from 'lib/components/CompareFilter/compareFilterLogic'
import { dateMapping } from 'lib/utils'
import { InsightLogicProps } from '~/types'
import { keyForInsightLogicProps } from 'scenes/insights/sharedUtils'
Expand All @@ -15,7 +14,7 @@ export const insightCommandLogic = kea<insightCommandLogicType>([
key(keyForInsightLogicProps('new')),
path((key) => ['scenes', 'insights', 'insightCommandLogic', key]),

connect((props: InsightLogicProps) => [commandPaletteLogic, compareFilterLogic(props), insightVizDataLogic(props)]),
connect((props: InsightLogicProps) => [commandPaletteLogic, insightVizDataLogic(props)]),
events(({ props }) => ({
afterMount: () => {
const funnelCommands: Command[] = [
Expand All @@ -26,7 +25,8 @@ export const insightCommandLogic = kea<insightCommandLogicType>([
icon: IconTrendingUp,
display: 'Toggle "Compare Previous" on Graph',
executor: () => {
compareFilterLogic(props).actions.toggleCompare()
const compare = insightVizDataLogic(props).values.compare
insightVizDataLogic(props).actions.updateInsightFilter({ compare: !compare })
},
},
...dateMapping.map(({ key, values }) => ({
Expand Down
26 changes: 24 additions & 2 deletions frontend/src/scenes/insights/insightVizDataLogic.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,11 @@ import {
isTrendsQuery,
nodeKindToFilterProperty,
} from '~/queries/utils'
import { NON_TIME_SERIES_DISPLAY_TYPES, PERCENT_STACK_VIEW_DISPLAY_TYPE } from 'lib/constants'
import {
NON_TIME_SERIES_DISPLAY_TYPES,
NON_VALUES_ON_SERIES_DISPLAY_TYPES,
PERCENT_STACK_VIEW_DISPLAY_TYPE,
} from 'lib/constants'
import {
getBreakdown,
getCompare,
Expand Down Expand Up @@ -127,13 +131,31 @@ export const insightVizDataLogic = kea<insightVizDataLogicType>([
isLifecycle: [(s) => [s.querySource], (q) => isLifecycleQuery(q)],
isTrendsLike: [(s) => [s.querySource], (q) => isTrendsQuery(q) || isLifecycleQuery(q) || isStickinessQuery(q)],
supportsDisplay: [(s) => [s.querySource], (q) => isTrendsQuery(q) || isStickinessQuery(q)],
supportsCompare: [(s) => [s.querySource], (q) => isTrendsQuery(q) || isStickinessQuery(q)],
supportsCompare: [
(s) => [s.querySource, s.display, s.dateRange],
(q, display, dateRange) =>
(isTrendsQuery(q) || isStickinessQuery(q)) &&
display !== ChartDisplayType.WorldMap &&
dateRange?.date_from !== 'all',
],
supportsPercentStackView: [
(s) => [s.querySource, s.display],
(q, display) =>
isTrendsQuery(q) &&
PERCENT_STACK_VIEW_DISPLAY_TYPE.includes(display || ChartDisplayType.ActionsLineGraph),
],
supportsValueOnSeries: [
(s) => [s.isTrends, s.isStickiness, s.isLifecycle, s.display],
(isTrends, isStickiness, isLifecycle, display) => {
if (isTrends || isStickiness) {
return !NON_VALUES_ON_SERIES_DISPLAY_TYPES.includes(display || ChartDisplayType.ActionsLineGraph)
} else if (isLifecycle) {
return true
} else {
return false
}
},
],

dateRange: [(s) => [s.querySource], (q) => (q ? q.dateRange : null)],
breakdown: [(s) => [s.querySource], (q) => (q ? getBreakdown(q) : null)],
Expand Down

0 comments on commit 7f851ce

Please sign in to comment.