Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(data-exploration): replace trendsLogic with trendsDataLogic #16576

Merged
merged 45 commits into from
Jul 18, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
45 commits
Select commit Hold shift + click to select a range
f2c6c20
replace trendsLogic in ActionsHorizontalBar
thmsobrmlr Jul 13, 2023
e4afdec
replace trendsLogic with trendDataLogic
thmsobrmlr Jul 14, 2023
776e34f
fix lint error
thmsobrmlr Jul 14, 2023
be33ff7
replace satisfies with as
thmsobrmlr Jul 14, 2023
97a474c
fix legend display types
thmsobrmlr Jul 14, 2023
82cdd8e
move isSingleSeries to insightVizDataLogic
thmsobrmlr Jul 14, 2023
03413bd
replace resultsLoading
thmsobrmlr Jul 14, 2023
57b0f4f
move timezone to viz data logic
thmsobrmlr Jul 14, 2023
cebef01
replace insight in LineGraph
thmsobrmlr Jul 14, 2023
e5b39b5
remove trendsLogic tests
thmsobrmlr Jul 14, 2023
ba02107
clean up trendsDataLogic tests
thmsobrmlr Jul 14, 2023
29edb00
fix types
thmsobrmlr Jul 14, 2023
ca5db65
Update UI snapshots for `chromium` (2)
github-actions[bot] Jul 14, 2023
98687ec
fix InsightsTable stories
thmsobrmlr Jul 14, 2023
489a46b
Update UI snapshots for `chromium` (1)
github-actions[bot] Jul 14, 2023
9f0c4ae
Update UI snapshots for `chromium` (2)
github-actions[bot] Jul 14, 2023
09ce789
Update UI snapshots for `chromium` (1)
github-actions[bot] Jul 14, 2023
6040f49
Update UI snapshots for `chromium` (1)
github-actions[bot] Jul 14, 2023
081aca3
Update UI snapshots for `chromium` (1)
github-actions[bot] Jul 14, 2023
6aeecfd
handle load more for trends with breakdowns via trendsDataLogic
thmsobrmlr Jul 14, 2023
f538ae9
fix lint issue
thmsobrmlr Jul 14, 2023
beb490a
remove setFilters from insightDateFilterLogic
thmsobrmlr Jul 14, 2023
49664c1
Merge branch 'master' into thomas/replace-trends-logic
thmsobrmlr Jul 14, 2023
791df70
remove setFilters from chartFilterLogic
thmsobrmlr Jul 14, 2023
b0c74b8
remove setFilters from compareFilterLogic
thmsobrmlr Jul 14, 2023
e22cc5b
remove setFilters from intervalFilterLogic
thmsobrmlr Jul 14, 2023
6754243
use logics instead of props passing
thmsobrmlr Jul 14, 2023
58a1b66
cleanup
thmsobrmlr Jul 17, 2023
258ecc1
Merge branch 'master' into thomas/replace-trends-logic
thmsobrmlr Jul 17, 2023
91f1748
remove outdated test
thmsobrmlr Jul 17, 2023
518d9dc
fix InsightsTable stories
thmsobrmlr Jul 17, 2023
fc3db02
Update UI snapshots for `chromium` (2)
github-actions[bot] Jul 17, 2023
4e12aac
make FunnelsCue independent of setFilters
thmsobrmlr Jul 17, 2023
7886abe
fix types
thmsobrmlr Jul 17, 2023
c8fda32
remove unused action
thmsobrmlr Jul 17, 2023
6be52dc
fix types
thmsobrmlr Jul 17, 2023
5bc72ee
remove subscription on insightData updating insight
thmsobrmlr Jul 17, 2023
dac56d9
update insight result from cached insight (fixes #16558, closes #16559)
thmsobrmlr Jul 17, 2023
cd840a8
remove empty query default for dataNodeLogic to prevent race condition
thmsobrmlr Jul 17, 2023
9a711b9
only display last refresh when there is data
thmsobrmlr Jul 17, 2023
094ec39
remove unnecessary async
thmsobrmlr Jul 17, 2023
2780994
fix race condition between onMount and propsChanged
thmsobrmlr Jul 17, 2023
95d6f98
add breakpoint
thmsobrmlr Jul 17, 2023
1eaaadd
Merge branch 'master' into thomas/replace-trends-logic
thmsobrmlr Jul 17, 2023
8a4d99c
fix types
thmsobrmlr Jul 17, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion frontend/src/lib/colors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ export function getColorVar(variable: string): string {
*/
export function getSeriesColor(
index: number | undefined = 0,
comparePrevious: boolean = false,
comparePrevious: boolean | null = false,
asBackgroundHighlight?: boolean
): string {
const adjustedIndex = (comparePrevious ? Math.floor(index / 2) : index) % dataColorVars.length
Expand Down
23 changes: 10 additions & 13 deletions frontend/src/lib/components/ChartFilter/ChartFilter.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,21 +11,18 @@ import {
IconPublic,
} from 'lib/lemon-ui/icons'

import { ChartDisplayType, FilterType, TrendsFilterType } from '~/types'
import { ChartDisplayType } from '~/types'
import { insightLogic } from 'scenes/insights/insightLogic'
import { LemonSelect, LemonSelectOptions } from '@posthog/lemon-ui'
import { isTrendsFilter } from 'scenes/insights/sharedUtils'
import { insightVizDataLogic } from 'scenes/insights/insightVizDataLogic'

interface ChartFilterProps {
filters: FilterType
}

export function ChartFilter({ filters }: ChartFilterProps): JSX.Element {
const { insightProps, isSingleSeries } = useValues(insightLogic)
export function ChartFilter(): JSX.Element {
const { insightProps } = useValues(insightLogic)
const { chartFilter } = useValues(chartFilterLogic(insightProps))
const { setChartFilter } = useActions(chartFilterLogic(insightProps))

const isTrends = isTrendsFilter(filters)
const { isTrends, isSingleSeries, formula, breakdown } = useValues(insightVizDataLogic(insightProps))

const trendsOnlyDisabledReason = !isTrends ? 'This type is only available in Trends.' : undefined
const singleSeriesOnlyDisabledReason = !isSingleSeries
? 'This type currently only supports insights with one series, and this insight has multiple series.'
Expand Down Expand Up @@ -96,11 +93,11 @@ export function ChartFilter({ filters }: ChartFilterProps): JSX.Element {
tooltip: 'Visualize data by country.',
disabledReason:
trendsOnlyDisabledReason ||
((filters as TrendsFilterType).formula
(formula
? "This type isn't available, because it doesn't support formulas."
: !!filters.breakdown &&
filters.breakdown !== '$geoip_country_code' &&
filters.breakdown !== '$geoip_country_name'
: !!breakdown?.breakdown &&
breakdown.breakdown !== '$geoip_country_code' &&
breakdown.breakdown !== '$geoip_country_name'
? "This type isn't available, because there's a breakdown other than by Country Code or Country Name properties."
: undefined),
},
Expand Down
39 changes: 5 additions & 34 deletions frontend/src/lib/components/ChartFilter/chartFilterLogic.ts
Original file line number Diff line number Diff line change
@@ -1,57 +1,28 @@
import { kea } from 'kea'
import { objectsEqual } from 'lib/utils'
import type { chartFilterLogicType } from './chartFilterLogicType'
import { ChartDisplayType, InsightLogicProps, TrendsFilterType } from '~/types'
import {
isFilterWithDisplay,
isStickinessFilter,
isTrendsFilter,
keyForInsightLogicProps,
} from 'scenes/insights/sharedUtils'
import { insightLogic } from 'scenes/insights/insightLogic'
import { ChartDisplayType, InsightLogicProps } from '~/types'
import { keyForInsightLogicProps } from 'scenes/insights/sharedUtils'
import { insightVizDataLogic } from 'scenes/insights/insightVizDataLogic'

export const chartFilterLogic = kea<chartFilterLogicType>({
props: {} as InsightLogicProps,
key: keyForInsightLogicProps('new'),
path: (key) => ['lib', 'components', 'ChartFilter', 'chartFilterLogic', key],
connect: (props: InsightLogicProps) => ({
actions: [
insightLogic(props),
['setFilters'],
insightVizDataLogic(props),
['updateInsightFilter', 'updateBreakdown'],
],
values: [
insightLogic(props),
['filters'],
insightVizDataLogic(props),
['isTrends', 'isStickiness', 'display', 'series'],
],
actions: [insightVizDataLogic(props), ['updateInsightFilter', 'updateBreakdown']],
values: [insightVizDataLogic(props), ['isTrends', 'isStickiness', 'display', 'series']],
}),

actions: () => ({
setChartFilter: (chartFilter: ChartDisplayType) => ({ chartFilter }),
}),

selectors: {
chartFilter: [
(s) => [s.filters],
(filters): ChartDisplayType | null => {
return (isFilterWithDisplay(filters) ? filters.display : null) || null
},
],
chartFilter: [(s) => [s.display], (display): ChartDisplayType | null | undefined => display],
},

listeners: ({ actions, values }) => ({
setChartFilter: ({ chartFilter }) => {
if (isTrendsFilter(values.filters) || isStickinessFilter(values.filters)) {
if (!objectsEqual(values.filters.display, chartFilter)) {
const newFilteres: Partial<TrendsFilterType> = { ...values.filters, display: chartFilter }
actions.setFilters(newFilteres)
}
}

const { isTrends, isStickiness, display, series } = values
const newDisplay = chartFilter as ChartDisplayType

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ export function CompareFilter(): JSX.Element | null {
return (
<LemonCheckbox
onChange={setCompare}
checked={compare}
checked={!!compare}
disabled={disabled}
label={<span className="font-normal">Compare to previous time period</span>}
bordered
Expand Down
47 changes: 10 additions & 37 deletions frontend/src/lib/components/CompareFilter/compareFilterLogic.ts
Original file line number Diff line number Diff line change
@@ -1,26 +1,22 @@
import { kea } from 'kea'
import { objectsEqual } from 'lib/utils'
import { ChartDisplayType, InsightLogicProps, InsightType, TrendsFilterType } from '~/types'
import { ChartDisplayType, InsightLogicProps } from '~/types'
import type { compareFilterLogicType } from './compareFilterLogicType'
import { isStickinessFilter, keyForInsightLogicProps } from 'scenes/insights/sharedUtils'
import { keyForInsightLogicProps } from 'scenes/insights/sharedUtils'
import { insightLogic } from 'scenes/insights/insightLogic'
import { isTrendsFilter } from 'scenes/insights/sharedUtils'
import { StickinessFilter, TrendsFilter } from '~/queries/schema'
import { filterForQuery, isInsightQueryNode } from '~/queries/utils'
import { insightVizDataLogic } from 'scenes/insights/insightVizDataLogic'

export const compareFilterLogic = kea<compareFilterLogicType>({
props: {} as InsightLogicProps,
key: keyForInsightLogicProps('new'),
path: (key) => ['lib', 'components', 'CompareFilter', 'compareFilterLogic', key],
connect: (props: InsightLogicProps) => ({
actions: [insightLogic(props), ['setFilters'], insightVizDataLogic(props), ['updateInsightFilter']],
values: [
insightLogic(props),
['filters as inflightFilters', 'canEditInsight'],
['canEditInsight'],
insightVizDataLogic(props),
['querySource'],
['compare', 'display', 'insightFilter', 'isLifecycle', 'dateRange'],
],
actions: [insightVizDataLogic(props), ['updateInsightFilter']],
}),

actions: () => ({
Expand All @@ -29,42 +25,19 @@ export const compareFilterLogic = kea<compareFilterLogicType>({
}),

selectors: {
filters: [
(s) => [s.inflightFilters],
(inflightFilters): Partial<TrendsFilterType> =>
inflightFilters && (isTrendsFilter(inflightFilters) || isStickinessFilter(inflightFilters))
? inflightFilters
: {},
],
compare: [
(s) => [s.filters],
(filters) => filters && (isTrendsFilter(filters) || isStickinessFilter(filters)) && !!filters.compare,
],
disabled: [
(s) => [s.filters, s.canEditInsight],
({ insight, date_from, display }, canEditInsight) =>
(s) => [s.canEditInsight, s.isLifecycle, s.display, s.dateRange],
(canEditInsight, isLifecycle, display, dateRange) =>
!canEditInsight ||
insight === InsightType.LIFECYCLE ||
isLifecycle ||
display === ChartDisplayType.WorldMap ||
date_from === 'all',
dateRange?.date_from === 'all',
],
},

listeners: ({ values, actions }) => ({
setCompare: ({ compare }) => {
if (!objectsEqual(compare, values.compare)) {
const newFilters: Partial<TrendsFilterType> = { ...values.filters, compare }
actions.setFilters(newFilters)
}

if (isInsightQueryNode(values.querySource)) {
const currentCompare = (
filterForQuery(values.querySource) as TrendsFilter | StickinessFilter | undefined
)?.compare
if (currentCompare !== compare) {
actions.updateInsightFilter({ compare })
}
}
actions.updateInsightFilter({ compare })
},
toggleCompare: () => {
actions.setCompare(!values.compare)
Expand Down
19 changes: 11 additions & 8 deletions frontend/src/lib/components/InsightLegend/InsightLegend.tsx
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
import './InsightLegend.scss'
import { useActions, useValues } from 'kea'
import { insightLogic } from 'scenes/insights/insightLogic'
import { trendsLogic } from 'scenes/trends/trendsLogic'
import clsx from 'clsx'
import { InsightLegendRow } from './InsightLegendRow'
import { shouldShowLegend, shouldHighlightThisRow } from './utils'
import { shouldHighlightThisRow } from './utils'
import { trendsDataLogic } from 'scenes/trends/trendsDataLogic'

export interface InsightLegendProps {
readOnly?: boolean
Expand All @@ -13,12 +13,13 @@ export interface InsightLegendProps {
}

export function InsightLegend({ horizontal, inCardView, readOnly = false }: InsightLegendProps): JSX.Element | null {
const { insightProps, filters, highlightedSeries, isSingleSeries } = useValues(insightLogic)
const logic = trendsLogic(insightProps)
const { indexedResults, hiddenLegendKeys } = useValues(logic)
const { toggleVisibility } = useActions(logic)
const { insightProps, highlightedSeries, hiddenLegendKeys } = useValues(insightLogic)
const { toggleVisibility } = useActions(insightLogic)
const { indexedResults, compare, display, trendsFilter, hasLegend, isSingleSeries } = useValues(
trendsDataLogic(insightProps)
)

return shouldShowLegend(filters) ? (
return hasLegend ? (
<div
className={clsx('InsightLegendMenu', 'flex overflow-auto border rounded', {
'InsightLegendMenu--horizontal': horizontal,
Expand All @@ -37,7 +38,9 @@ export function InsightLegend({ horizontal, inCardView, readOnly = false }: Insi
hasMultipleSeries={!isSingleSeries}
highlighted={shouldHighlightThisRow(hiddenLegendKeys, index, highlightedSeries)}
toggleVisibility={toggleVisibility}
filters={filters}
compare={compare}
display={display}
trendsFilter={trendsFilter}
/>
))}
</div>
Expand Down
38 changes: 22 additions & 16 deletions frontend/src/lib/components/InsightLegend/InsightLegendRow.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,29 +2,35 @@ import { InsightLabel } from 'lib/components/InsightLabel'
import { getSeriesColor } from 'lib/colors'
import { LemonCheckbox } from 'lib/lemon-ui/LemonCheckbox'
import { formatCompareLabel } from 'scenes/insights/views/InsightsTable/columns/SeriesColumn'
import { ChartDisplayType, FilterType } from '~/types'
import { ChartDisplayType } from '~/types'
import { formatAggregationAxisValue } from 'scenes/insights/aggregationAxisFormat'
import { IndexedTrendResult } from 'scenes/trends/types'
import { useEffect, useRef } from 'react'
import { isTrendsFilter } from 'scenes/insights/sharedUtils'
import { TrendsFilter } from '~/queries/schema'

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,
filters,
compare,
display,
trendsFilter,
highlighted,
}: {
hiddenLegendKeys: Record<string, boolean | undefined>
rowIndex: number
item: IndexedTrendResult
hasMultipleSeries: boolean
toggleVisibility: (index: number) => void
filters: Partial<FilterType>
highlighted: boolean
}): JSX.Element {
}: InsightLegendRowProps): JSX.Element {
const highlightStyle: Record<string, any> = highlighted
? {
style: { backgroundColor: getSeriesColor(item.seriesIndex, false, true) },
Expand All @@ -38,8 +44,6 @@ export function InsightLegendRow({
}
}, [highlighted])

const compare = isTrendsFilter(filters) && !!filters.compare

return (
<div key={item.id} className="InsightLegendMenu-item p-2 flex flex-row" ref={rowRef} {...highlightStyle}>
<div className="grow">
Expand All @@ -64,8 +68,10 @@ export function InsightLegendRow({
}
/>
</div>
{isTrendsFilter(filters) && filters.display === ChartDisplayType.ActionsPie && (
<div className="text-muted grow-0">{formatAggregationAxisValue(filters, item.aggregated_value)}</div>
{display === ChartDisplayType.ActionsPie && (
<div className="text-muted grow-0">
{formatAggregationAxisValue(trendsFilter, item.aggregated_value)}
</div>
)}
</div>
)
Expand Down
8 changes: 2 additions & 6 deletions frontend/src/lib/components/InsightLegend/utils.ts
Original file line number Diff line number Diff line change
@@ -1,16 +1,12 @@
import { ChartDisplayType, FilterType } from '~/types'
import { isFilterWithDisplay } from 'scenes/insights/sharedUtils'
import { ChartDisplayType } from '~/types'

export const displayTypesWithoutLegend = [
export const DISPLAY_TYPES_WITHOUT_LEGEND = [
ChartDisplayType.WorldMap,
ChartDisplayType.ActionsTable,
ChartDisplayType.BoldNumber,
ChartDisplayType.ActionsBarValue,
]

export const shouldShowLegend = (filters: Partial<FilterType>): boolean =>
isFilterWithDisplay(filters) && !!filters.display && !displayTypesWithoutLegend.includes(filters.display)

export function shouldHighlightThisRow(
hiddenLegendKeys: Record<string, boolean | undefined>,
rowIndex: number,
Expand Down

This file was deleted.

Loading