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

Refactor old cohort event selector #8416

Merged
merged 5 commits into from
Feb 3, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
29 changes: 22 additions & 7 deletions frontend/src/lib/components/TaxonomicPopup/TaxonomicPopup.tsx
Original file line number Diff line number Diff line change
@@ -1,22 +1,36 @@
import './TaxonomicPopup.scss'
import { Popup } from 'lib/components/Popup/Popup'
import { TaxonomicFilter } from 'lib/components/TaxonomicFilter/TaxonomicFilter'
import { TaxonomicFilterGroupType } from 'lib/components/TaxonomicFilter/types'
import { TaxonomicFilterGroupType, TaxonomicFilterValue } from 'lib/components/TaxonomicFilter/types'
import React, { useState } from 'react'
import { Button } from 'antd'
import { DownOutlined } from '@ant-design/icons'
import clsx from 'clsx'

export interface TaxonomicPopupProps {
export interface TaxonomicPopupProps<ValueType = TaxonomicFilterValue> {
groupType: TaxonomicFilterGroupType
value?: string
onChange: (value: string, groupType: TaxonomicFilterGroupType) => void
value?: ValueType
onChange: (value: ValueType, groupType: TaxonomicFilterGroupType) => void

groupTypes?: TaxonomicFilterGroupType[]
renderValue?: (value: string) => JSX.Element
renderValue?: (value: ValueType) => JSX.Element
dataAttr?: string
eventNames?: string[]
placeholder?: React.ReactNode
style?: React.CSSProperties
fullWidth?: boolean
}

/** Like TaxonomicPopup, but convenient when you know you will only use string values */
export function TaxonomicStringPopup(props: TaxonomicPopupProps<string>): JSX.Element {
return (
<TaxonomicPopup
{...props}
value={String(props.value)}
onChange={(value, groupType) => props.onChange?.(String(value), groupType)}
renderValue={(value) => props.renderValue?.(String(value)) ?? <>{String(props.value)}</>}
/>
)
}

export function TaxonomicPopup({
Expand All @@ -29,6 +43,7 @@ export function TaxonomicPopup({
eventNames = [],
placeholder = 'Please select',
style,
fullWidth = true,
}: TaxonomicPopupProps): JSX.Element {
const [visible, setVisible] = useState(false)

Expand All @@ -39,7 +54,7 @@ export function TaxonomicPopup({
groupType={groupType}
value={value}
onChange={({ type }, payload) => {
onChange?.(String(payload), type)
onChange?.(payload, type)
setVisible(false)
}}
taxonomicGroupTypes={groupTypes ?? [groupType]}
Expand All @@ -54,7 +69,7 @@ export function TaxonomicPopup({
data-attr={dataAttr}
onClick={() => setVisible(!visible)}
ref={setRef}
className="TaxonomicPopup__button"
className={clsx('TaxonomicPopup__button', { 'full-width': fullWidth })}
style={style}
>
<span className="text-overflow" style={{ maxWidth: '100%' }}>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import React, { useState } from 'react'
import { TaxonomicPopup } from '../TaxonomicPopup'
import { TaxonomicPopup, TaxonomicStringPopup } from '../TaxonomicPopup'
import { personPropertiesModel } from '~/models/personPropertiesModel'
import { cohortsModel } from '~/models/cohortsModel'
import { TaxonomicFilterGroupType } from 'lib/components/TaxonomicFilter/types'
Expand All @@ -11,7 +11,7 @@ export default {
title: 'PostHog/Components/TaxonomicPopup',
}

export const OneCategory = (): JSX.Element => {
export const TaxonomicStringPopupOneCategory = (): JSX.Element => {
const [value, setValue] = useState<string | undefined>('$browser')

return (
Expand All @@ -22,7 +22,7 @@ export const OneCategory = (): JSX.Element => {
defaultFilterMocks()
}}
>
<TaxonomicPopup
<TaxonomicStringPopup
groupType={TaxonomicFilterGroupType.PersonProperties}
value={value}
onChange={setValue}
Expand All @@ -33,7 +33,7 @@ export const OneCategory = (): JSX.Element => {
}

export const MultipleCategories = (): JSX.Element => {
const [value, setValue] = useState<string | undefined>(undefined)
const [value, setValue] = useState<string | number | undefined>(undefined)
const [group, setGroup] = useState(TaxonomicFilterGroupType.PersonProperties)

return (
Expand Down
4 changes: 4 additions & 0 deletions frontend/src/models/actionsModel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,10 @@ interface ActionsModelProps {
params?: string
}

export function findActionName(id: number): string | null {
return actionsModel.findMounted()?.values.actions.find((a) => a.id === id)?.name || null
}

export const actionsModel = kea<actionsModelType<ActionsModelProps>>({
path: ['models', 'actionsModel'],
props: {} as ActionsModelProps,
Expand Down
6 changes: 3 additions & 3 deletions frontend/src/scenes/actions/EventName.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import React from 'react'
import { PropertyKeyInfo } from 'lib/components/PropertyKeyInfo'
import { TaxonomicPopup } from 'lib/components/TaxonomicPopup/TaxonomicPopup'
import { TaxonomicStringPopup } from 'lib/components/TaxonomicPopup/TaxonomicPopup'
import { TaxonomicFilterGroupType } from 'lib/components/TaxonomicFilter/types'

interface EventNameInterface {
Expand All @@ -10,11 +10,11 @@ interface EventNameInterface {

export function EventName({ value, onChange }: EventNameInterface): JSX.Element {
return (
<TaxonomicPopup
<TaxonomicStringPopup
groupType={TaxonomicFilterGroupType.Events}
onChange={onChange}
value={value}
style={{ width: '100%', maxWidth: '24rem' }}
style={{ maxWidth: '24rem' }}
placeholder="Choose an event"
dataAttr="event-name-box"
renderValue={(v) => <PropertyKeyInfo value={v} />}
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -11,20 +11,20 @@ export function CohortMatchingCriteriaSection({ logic }: { logic: BuiltLogic<coh
const { setCohort, onCriteriaChange } = useActions(logic)
const { cohort, submitted } = useValues(logic)
const onAddGroup = (): void => {
cohort.groups = [
...cohort.groups,
{
id: Math.random().toString().substr(2, 5),
matchType: PROPERTY_MATCH_TYPE,
properties: [],
},
]
setCohort({ ...cohort })
setCohort({
...cohort,
groups: [
...cohort.groups,
{
id: Math.random().toString().substr(2, 5),
matchType: PROPERTY_MATCH_TYPE,
properties: [],
},
],
})
}

const onRemoveGroup = (index: number): void => {
cohort.groups.splice(index, 1)
setCohort({ ...cohort })
setCohort({ ...cohort, groups: cohort.groups.filter((_, i) => i !== index) })
}
const addButton = (
<div style={{ marginTop: 8, marginBottom: 8 }}>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
import React, { useState } from 'react'
import { Button, Col, Input, Row, Select } from 'antd'
import { CohortEntityFilterBox } from './CohortEntityFilterBox'
import React from 'react'
import { Col, Input, Row, Select } from 'antd'
import { PropertyKeyInfo } from 'lib/components/PropertyKeyInfo'
import { SelectDownIcon } from 'lib/components/SelectDownIcon'
import { CohortGroupType, MatchType } from '~/types'
import { ACTION_TYPE, ENTITY_MATCH_TYPE, EVENT_TYPE, PROPERTY_MATCH_TYPE } from 'lib/constants'
import { ENTITY_MATCH_TYPE, PROPERTY_MATCH_TYPE } from 'lib/constants'
import { PropertyFilters } from 'lib/components/PropertyFilters/PropertyFilters'
import { DeleteOutlined } from '@ant-design/icons'
import { TaxonomicFilterGroupType } from 'lib/components/TaxonomicFilter/types'
import { TaxonomicPopup } from 'lib/components/TaxonomicPopup/TaxonomicPopup'
import { findActionName } from '~/models/actionsModel'

const { Option } = Select

Expand Down Expand Up @@ -128,8 +128,6 @@ function EntityCriteriaRow({
onEntityCriteriaChange: (group: Partial<CohortGroupType>) => void
group: CohortGroupType
}): JSX.Element {
const [open, setOpen] = useState(false)

const { label, days, count_operator, count } = group

const onOperatorChange = (newCountOperator: string): void => {
Expand All @@ -144,33 +142,30 @@ function EntityCriteriaRow({
onEntityCriteriaChange({ count: newCount })
}

const onEntityChange = (type: any, id: string | number, newLabel: string): void => {
if (type === EVENT_TYPE && typeof id === 'string') {
onEntityCriteriaChange({ event_id: id, label: newLabel })
} else if (type === ACTION_TYPE && typeof id === 'number') {
onEntityCriteriaChange({ action_id: id, label: newLabel })
const onEntityChange = (type: TaxonomicFilterGroupType, id: string | number): void => {
if (type === TaxonomicFilterGroupType.Events && typeof id === 'string') {
onEntityCriteriaChange({ event_id: id, action_id: undefined, label: id })
} else if (type === TaxonomicFilterGroupType.Actions && typeof id === 'number') {
onEntityCriteriaChange({
action_id: id,
event_id: undefined,
label: findActionName(id) || `Action #${id}`,
})
}
setOpen(false)
}

return (
<div style={{ marginTop: 16, width: '100%' }}>
<Row gutter={8}>
<Col flex="auto">
<Button
onClick={() => setOpen(!open)}
className="full-width"
style={{
display: 'flex',
justifyContent: 'space-between',
alignItems: 'center',
}}
data-attr="edit-cohort-entity-filter"
>
<PropertyKeyInfo value={label || 'Select an event'} />
<SelectDownIcon className="text-muted" />
</Button>
<CohortEntityFilterBox open={open} onSelect={onEntityChange} />
<TaxonomicPopup
groupTypes={[TaxonomicFilterGroupType.Events, TaxonomicFilterGroupType.Actions]}
groupType={group.action_id ? TaxonomicFilterGroupType.Actions : TaxonomicFilterGroupType.Events}
value={group.action_id || group.event_id}
onChange={(value, groupType) => onEntityChange(groupType, value)}
renderValue={() => <PropertyKeyInfo value={label || 'Select an event'} disablePopover={true} />}
dataAttr="edit-cohort-entity-filter"
/>
</Col>
<Col span={4}>
<OperatorSelect value={count_operator} onChange={onOperatorChange} />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ import { apiValueToMathType, mathsLogic, mathTypeToApiValues } from 'scenes/tren
import { GroupsIntroductionOption } from 'lib/introductions/GroupsIntroductionOption'
import { actionsModel } from '~/models/actionsModel'
import { PropertyKeyInfo } from 'lib/components/PropertyKeyInfo'
import { TaxonomicPopup } from 'lib/components/TaxonomicPopup/TaxonomicPopup'
import { TaxonomicStringPopup } from 'lib/components/TaxonomicPopup/TaxonomicPopup'

const determineFilterLabel = (visible: boolean, filter: Partial<ActionFilter>): string => {
if (visible) {
Expand Down Expand Up @@ -373,7 +373,7 @@ export function ActionFilterRow({
maxWidth: `calc(50% - 16px${showSeriesIndicator ? ' - 32px' : ''})`,
}}
>
<TaxonomicPopup
<TaxonomicStringPopup
groupType={TaxonomicFilterGroupType.NumericalEventProperties}
value={mathProperty}
onChange={(currentValue) => onMathPropertySelect(index, currentValue)}
Expand Down