Skip to content

Commit

Permalink
fix(insights): keep interval choice on date filter change (#26793)
Browse files Browse the repository at this point in the history
  • Loading branch information
adamleithp authored Dec 10, 2024
1 parent 49f8a68 commit 3607d71
Show file tree
Hide file tree
Showing 3 changed files with 58 additions and 20 deletions.
49 changes: 33 additions & 16 deletions frontend/src/lib/components/IntervalFilter/IntervalFilter.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { LemonSelect, LemonSelectOption } from '@posthog/lemon-ui'
import { IconPin } from '@posthog/icons'
import { LemonButton, LemonSelect, LemonSelectOption } from '@posthog/lemon-ui'
import { useActions, useValues } from 'kea'
import { insightLogic } from 'scenes/insights/insightLogic'
import { insightVizDataLogic } from 'scenes/insights/insightVizDataLogic'
Expand All @@ -12,27 +13,43 @@ interface IntervalFilterProps {

export function IntervalFilter({ disabled }: IntervalFilterProps): JSX.Element {
const { insightProps } = useValues(insightLogic)
const { interval, enabledIntervals } = useValues(insightVizDataLogic(insightProps))
const { updateQuerySource } = useActions(insightVizDataLogic(insightProps))
const { interval, enabledIntervals, isIntervalManuallySet } = useValues(insightVizDataLogic(insightProps))
const { updateQuerySource, setIsIntervalManuallySet } = useActions(insightVizDataLogic(insightProps))

return (
<>
<span>
<span className="hidden md:inline">grouped </span>by
</span>
<IntervalFilterStandalone
disabled={disabled}
interval={interval || 'day'}
onIntervalChange={(value) => {
updateQuerySource({ interval: value } as Partial<InsightQueryNode>)
}}
options={Object.entries(enabledIntervals).map(([value, { label, disabledReason, hidden }]) => ({
value: value as IntervalType,
label,
hidden,
disabledReason,
}))}
/>
{isIntervalManuallySet ? (
<LemonButton
type="secondary"
onClick={() => {
setIsIntervalManuallySet(false)
}}
tooltip="Unpin interval"
className="flex-1"
center
size="small"
icon={<IconPin color="var(--content-warning)" />}
>
{interval || 'day'}
</LemonButton>
) : (
<IntervalFilterStandalone
disabled={disabled}
interval={interval || 'day'}
onIntervalChange={(value) => {
updateQuerySource({ interval: value } as Partial<InsightQueryNode>)
}}
options={Object.entries(enabledIntervals).map(([value, { label, disabledReason, hidden }]) => ({
value: value as IntervalType,
label,
hidden,
disabledReason,
}))}
/>
)}
</>
)
}
Expand Down
27 changes: 23 additions & 4 deletions frontend/src/scenes/insights/insightVizDataLogic.ts
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ export const insightVizDataLogic = kea<insightVizDataLogicType>([
updateDisplay: (display: ChartDisplayType | undefined) => ({ display }),
updateHiddenLegendIndexes: (hiddenLegendIndexes: number[] | undefined) => ({ hiddenLegendIndexes }),
setTimedOutQueryId: (id: string | null) => ({ id }),
setIsIntervalManuallySet: (isIntervalManuallySet: boolean) => ({ isIntervalManuallySet }),
}),

reducers({
Expand All @@ -109,6 +110,18 @@ export const insightVizDataLogic = kea<insightVizDataLogicType>([
setTimedOutQueryId: (_, { id }) => id,
},
],

// Whether the interval has been manually set by the user. If true, prevents auto-adjusting the interval when date range changes. Reference: https://github.com/PostHog/posthog/issues/22785
isIntervalManuallySet: [
false,
{
updateQuerySource: (state, { querySource }) => {
// If interval is explicitly included in the update, mark it as manually set
return 'interval' in querySource ? true : state
},
setIsIntervalManuallySet: (_, { isIntervalManuallySet }) => isIntervalManuallySet,
},
],
}),

selectors({
Expand Down Expand Up @@ -332,7 +345,7 @@ export const insightVizDataLogic = kea<insightVizDataLogicType>([
// We use 512 for query timeouts
// Async queries put the error message on data.error_message, while synchronous ones use detail
return insightDataError?.status === 400 || insightDataError?.status === 512
? (insightDataError.detail || insightDataError.data?.error_message)?.replace('Try ', 'Try ') // Add unbreakable space for better line breaking
? (insightDataError.detail || insightDataError.data?.error_message)?.replace('Try ', 'Try ') // Add unbreakable space for better line breaking
: null
},
],
Expand Down Expand Up @@ -401,7 +414,11 @@ export const insightVizDataLogic = kea<insightVizDataLogicType>([
...values.query,
source: {
...values.querySource,
...handleQuerySourceUpdateSideEffects(querySource, values.querySource as InsightQueryNode),
...handleQuerySourceUpdateSideEffects(
querySource,
values.querySource as InsightQueryNode,
values.isIntervalManuallySet
),
},
} as Node)
},
Expand Down Expand Up @@ -487,7 +504,8 @@ const getActiveUsersMath = (

const handleQuerySourceUpdateSideEffects = (
update: QuerySourceUpdate,
currentState: InsightQueryNode
currentState: InsightQueryNode,
isIntervalManuallySet: boolean
): QuerySourceUpdate => {
const mergedUpdate = { ...update } as InsightQueryNode

Expand Down Expand Up @@ -536,7 +554,8 @@ const handleQuerySourceUpdateSideEffects = (
update.dateRange &&
update.dateRange.date_from &&
(update.dateRange.date_from !== currentState.dateRange?.date_from ||
update.dateRange.date_to !== currentState.dateRange?.date_to)
update.dateRange.date_to !== currentState.dateRange?.date_to) &&
!isIntervalManuallySet // Only auto-adjust interval if not manually set
) {
const { date_from, date_to } = { ...currentState.dateRange, ...update.dateRange }

Expand Down
2 changes: 2 additions & 0 deletions frontend/src/styles/global.scss
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,7 @@ Only 400 (`normal`), 500 (`var(--font-medium)`), 600 (`var(--font-semibold)`), o
--content-link: var(--brand-500);
--content-link-hover: var(--brand-400);
--content-link-pressed: var(--brand-600);
--content-warning: var(--orange-400);
--content-warning-bold: var(--orange-700);
--content-danger: var(--red-500);
--content-danger-bold: var(--red-600);
Expand Down Expand Up @@ -577,6 +578,7 @@ body {
&[theme='dark'] {
// Semantic colors (Dark mode) WIP
--content-primary: var(--neutral-cool-100);
--content-warning: var(--orange-300);
--content-warning-bold: var(--orange-100);
--content-danger-bold: var(--red-100);
--content-success-bold: var(--green-100);
Expand Down

0 comments on commit 3607d71

Please sign in to comment.