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

chore(settings): Clarify persons on events setting #22953

Merged
merged 30 commits into from
Jun 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
871256a
chore(settings): Clarify persons on events setting
Twixes Jun 7, 2024
40ec23b
Update UI snapshots for `chromium` (1)
github-actions[bot] Jun 13, 2024
9780550
Update UI snapshots for `chromium` (2)
github-actions[bot] Jun 13, 2024
dee2874
Update UI snapshots for `chromium` (2)
github-actions[bot] Jun 13, 2024
e5af593
Update UI snapshots for `chromium` (2)
github-actions[bot] Jun 13, 2024
b5e80fa
Merge branch 'master' into poe-setting-clearer
Twixes Jun 13, 2024
8617413
Add docs link
Twixes Jun 14, 2024
eef4b2f
Merge branch 'master' into poe-setting-clearer
Twixes Jun 14, 2024
384b3ef
Update UI snapshots for `chromium` (1)
github-actions[bot] Jun 14, 2024
6c0b6be
Update UI snapshots for `chromium` (1)
github-actions[bot] Jun 14, 2024
8bc4a8b
Update UI snapshots for `chromium` (2)
github-actions[bot] Jun 14, 2024
6591052
Update UI snapshots for `chromium` (2)
github-actions[bot] Jun 14, 2024
eb1e5d5
Update UI snapshots for `chromium` (2)
github-actions[bot] Jun 14, 2024
55c3d04
Update UI snapshots for `chromium` (2)
github-actions[bot] Jun 14, 2024
8c23a39
Update UI snapshots for `chromium` (1)
github-actions[bot] Jun 14, 2024
de03754
Update UI snapshots for `chromium` (1)
github-actions[bot] Jun 14, 2024
0a6352c
Say "Recommended"
Twixes Jun 19, 2024
744549b
Update UI snapshots for `chromium` (1)
github-actions[bot] Jun 19, 2024
e372770
Update UI snapshots for `chromium` (1)
github-actions[bot] Jun 19, 2024
246ac0c
Update UI snapshots for `chromium` (2)
github-actions[bot] Jun 19, 2024
a298e9f
Update UI snapshots for `chromium` (1)
github-actions[bot] Jun 19, 2024
3de619b
Update UI snapshots for `chromium` (2)
github-actions[bot] Jun 19, 2024
503edc3
Update UI snapshots for `chromium` (1)
github-actions[bot] Jun 19, 2024
3d561e1
Update UI snapshots for `chromium` (2)
github-actions[bot] Jun 19, 2024
60e10a4
Update UI snapshots for `chromium` (2)
github-actions[bot] Jun 19, 2024
b8661e7
Update UI snapshots for `chromium` (2)
github-actions[bot] Jun 19, 2024
74b89ec
Update UI snapshots for `chromium` (2)
github-actions[bot] Jun 19, 2024
8a09619
Update UI snapshots for `chromium` (2)
github-actions[bot] Jun 19, 2024
6978e41
Update UI snapshots for `chromium` (2)
github-actions[bot] Jun 19, 2024
1d2563c
Update UI snapshots for `chromium` (2)
github-actions[bot] Jun 19, 2024
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
Binary file modified frontend/__snapshots__/lemon-ui-lemon-radio--default--dark.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified frontend/__snapshots__/lemon-ui-lemon-radio--default--light.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file not shown.
Binary file not shown.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
16 changes: 11 additions & 5 deletions frontend/src/lib/lemon-ui/LemonRadio/LemonRadio.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ const meta: Meta<typeof LemonRadio> = {
options: [
{ value: 'calendar', label: 'Calendar' },
{ value: 'calculator', label: 'Calculator' },
{ value: 'banana', label: 'Banana' },
{ value: 'banana', label: 'Banana', disabledReason: 'Bananas are not allowed on pizza' },
{ value: 'settings', label: 'Settings' },
] as LemonRadioOption<string>[],
},
Expand All @@ -38,11 +38,17 @@ const Template: StoryFn<typeof LemonRadio> = (props: Omit<LemonRadioProps<any>,
export const Default: Story = Template.bind({})
Default.args = {}

export const Disabled: Story = Template.bind({})
Disabled.args = {
export const WithDescriptions: Story = Template.bind({})
WithDescriptions.args = {
options: [
{ value: 'calendar', label: 'Calendar' },
{ value: 'calculator', label: 'Calculator' },
{ value: 'banana', label: 'Banana', disabledReason: 'Bananas are not allowed' },
{ value: 'calculator', label: 'Calculator', description: '2.1 + 2.01 = 4.109999999999999' },
{
value: 'banana',
label: 'Banana',
disabledReason: 'Bananas are not allowed on pizza',
description:
'Note: The banana addon ships from Costa Rica, which will add 2 working days of a delay to your order.',
},
],
}
8 changes: 6 additions & 2 deletions frontend/src/lib/lemon-ui/LemonRadio/LemonRadio.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { Tooltip } from 'lib/lemon-ui/Tooltip'

export interface LemonRadioOption<T extends React.Key> {
label: string | JSX.Element
description?: string | JSX.Element
value: T
disabledReason?: string
'data-attr'?: string
Expand All @@ -25,12 +26,12 @@ export function LemonRadio<T extends React.Key>({
}: LemonRadioProps<T>): JSX.Element {
return (
<div className={clsx('flex flex-col gap-2 font-medium', className)}>
{options.map(({ value, label, disabledReason, ...optionProps }) => {
{options.map(({ value, label, disabledReason, description, ...optionProps }) => {
const content = (
<label
key={value}
className={clsx(
'flex items-center space-x-2',
'grid items-center gap-x-2 grid-cols-[min-content_auto] text-sm',
disabledReason ? 'text-muted cursor-not-allowed' : 'cursor-pointer'
)}
>
Expand All @@ -48,6 +49,9 @@ export function LemonRadio<T extends React.Key>({
{...optionProps}
/>
<span>{label}</span>
{description && (
<div className="text-muted row-start-2 col-start-2 text-pretty">{description}</div>
)}
</label>
)

Expand Down
54 changes: 31 additions & 23 deletions frontend/src/scenes/settings/project/PersonsOnEvents.tsx
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { LemonTag, Link } from '@posthog/lemon-ui'
import { useActions, useValues } from 'kea'
import { LemonButton } from 'lib/lemon-ui/LemonButton'
import { LemonRadio, LemonRadioOption } from 'lib/lemon-ui/LemonRadio'
Expand All @@ -9,39 +10,38 @@ import { HogQLQueryModifiers } from '~/queries/schema'

type PoEMode = NonNullable<HogQLQueryModifiers['personsOnEventsMode']>

const poeOptions: LemonRadioOption<PoEMode>[] = [
const POE_OPTIONS: LemonRadioOption<PoEMode>[] = [
{
value: 'person_id_no_override_properties_on_events',
value: 'person_id_override_properties_on_events',
label: (
<span className="inline-flex items-center gap-1.5">
Use person properties from the time of the event<LemonTag>RECOMMENDED</LemonTag>
</span>
),
description: (
<>
<div>Use person ids and properties from the time of the event.</div>
<div className="text-muted">
May show higher unique user counts due to not using latest person ids. You probably want one of the
other options. Fastest queries.
</div>
Fast queries. If the person property is updated, query results on past data <em>won't</em> change.
</>
),
},
{
value: 'person_id_override_properties_on_events',
label: (
value: 'person_id_override_properties_joined',
label: 'Use person properties as of running the query',
description: (
<>
<div>Use person properties from the time of the event.</div>
<div className="text-muted">
Fast queries. If person property is updated, then query results on past data won't change.
</div>
Slower queries. If the person property is updated, query results on past data <em>will</em> change
accordingly.
</>
),
},
{
value: 'person_id_override_properties_joined',
label: (
value: 'person_id_no_override_properties_on_events',
label: 'Use person IDs and person properties from the time of the event',
description: (
<>
<div>Use latest person properties.</div>
<div className="text-muted">
Slower queries. If person property is updated, then query results on past data will change to
reflect it.
</div>
Fastest queries,{' '}
<span className="underline">but funnels and unique user counts will be inaccurate</span>. If the person
property is updated, query results on past data <em>won't</em> change.
</>
),
},
Expand All @@ -51,7 +51,7 @@ export function PersonsOnEvents(): JSX.Element {
const { updateCurrentTeam } = useActions(teamLogic)
const { reportPoEModeUpdated } = useActions(eventUsageLogic)
const { currentTeam } = useValues(teamLogic)
const savedPoEMode =
const savedPoEMode: PoEMode =
currentTeam?.modifiers?.personsOnEventsMode ?? currentTeam?.default_modifiers?.personsOnEventsMode ?? 'disabled'
const [poeMode, setPoeMode] = useState<PoEMode>(savedPoEMode)

Expand All @@ -62,8 +62,16 @@ export function PersonsOnEvents(): JSX.Element {

return (
<>
<p>Choose how to query your event data with person filters.</p>
<LemonRadio value={poeMode} onChange={setPoeMode} options={poeOptions} />
<p>
Choose the behavior of person property filters.{' '}
<Link
to="https://posthog.com/docs/how-posthog-works/queries#filtering-on-person-properties"
target="blank"
>
Learn about the details in our docs.
</Link>
</p>
<LemonRadio value={poeMode} onChange={setPoeMode} options={POE_OPTIONS} />
<div className="mt-4">
<LemonButton
type="primary"
Expand Down
Loading