fix(insights): Fix property filter group and expression UI

.github/workflows/storybook-chromatic.yml

@@ -61,7 +61,7 @@ jobs:
         env:
             SHARD_COUNT: '2'
             CYPRESS_INSTALL_BINARY: '0'
-            NODE_OPTIONS: --max-old-space-size=4096
+            NODE_OPTIONS: --max-old-space-size=6144
             JEST_IMAGE_SNAPSHOT_TRACK_OBSOLETE: '1' # Remove obsolete snapshots
         outputs:
             # The below have to be manually listed unfortunately, as GitHub Actions doesn't allow matrix-dependent outputs

frontend/src/lib/components/HogQLEditor/HogQLEditor.tsx

@@ -2,6 +2,7 @@ import { useEffect, useState } from 'react'
 import { LemonTextArea } from 'lib/lemon-ui/LemonTextArea/LemonTextArea'
 import { CLICK_OUTSIDE_BLOCK_CLASS } from 'lib/hooks/useOutsideClickHandler'
 import { LemonButton } from 'lib/lemon-ui/LemonButton'
+import { IconInfo } from 'lib/lemon-ui/icons'
 
 export interface HogQLEditorProps {
     onChange: (value: string) => void
@@ -43,7 +44,7 @@ export function HogQLEditor({
                 }
                 onPressCmdEnter={disableCmdEnter ? undefined : () => onChange(localValue)}
                 className={`font-mono ${CLICK_OUTSIDE_BLOCK_CLASS}`}
-                minRows={6}
+                minRows={3}
                 maxRows={6}
                 placeholder={
                     placeholder ??
@@ -53,6 +54,7 @@ export function HogQLEditor({
                 }
             />
             <LemonButton
+                className="mt-2"
                 fullWidth
                 type="primary"
                 onClick={() => onChange(localValue)}
@@ -63,11 +65,18 @@ export function HogQLEditor({
             </LemonButton>
             <div className="flex mt-1 gap-1">
                 {disablePersonProperties ? (
-                    <div className="flex-1 text-muted">
-                        Note: <code>person.properties</code> can't be used here.
+                    <div className="flex-1 flex items-center text-muted select-none">
+                        <IconInfo className="text-base mr-1" />
+                        <span>
+                            <code>person.properties</code> can't be used here.
+                        </span>
                     </div>
                 ) : null}
-                <div className={`${disablePersonProperties ? '' : 'w-full '}text-right ${CLICK_OUTSIDE_BLOCK_CLASS}`}>
+                <div
+                    className={`${
+                        disablePersonProperties ? '' : 'w-full '
+                    }text-right select-none ${CLICK_OUTSIDE_BLOCK_CLASS}`}
+                >
                     <a href="https://posthog.com/manual/hogql" target={'_blank'}>
                         Learn more about HogQL
                     </a>

frontend/src/lib/components/PropertyFilters/PropertyFilters.scss

@@ -2,23 +2,22 @@
     display: flex;
     gap: 0.5rem;
 
-    .PropertyFilters-content {
+    .PropertyFilters__content {
         flex: 1;
         display: flex;
         flex-wrap: wrap;
         gap: 0.5rem;
         overflow: hidden;
     }
 
-    .PropertyFilters-prefix {
+    .PropertyFilters__prefix {
         color: #c4c4c4;
         font-size: 18px;
         user-select: none;
         padding: 0px 5px;
     }
 
     .logical-row-divider {
-        color: var(--primary-alt);
         font-weight: 600;
         font-size: 12px;
         text-transform: uppercase;

frontend/src/lib/components/PropertyFilters/PropertyFilters.tsx

@@ -57,8 +57,8 @@ export function PropertyFilters({
 
     return (
         <div className="PropertyFilters" style={style}>
-            {showNestedArrow && !disablePopover && <div className="PropertyFilters-prefix">{<>&#8627;</>}</div>}
-            <div className="PropertyFilters-content">
+            {showNestedArrow && !disablePopover && <div className="PropertyFilters__prefix">{<>&#8627;</>}</div>}
+            <div className="PropertyFilters__content">
                 <BindLogic logic={propertyFilterLogic} props={logicProps}>
                     {filtersWithNew.map((item: AnyPropertyFilter, index: number) => {
                         return (

frontend/src/lib/components/PropertyFilters/components/FilterRow.tsx

@@ -53,11 +53,7 @@ export const FilterRow = React.memo(function FilterRow({
         <>
             <Row
                 align="middle"
-                className={clsx(
-                    'property-filter-row',
-                    !disablePopover && 'wrap-filters',
-                    orFiltering && index !== 0 && 'mt-2'
-                )}
+                className={clsx('property-filter-row', !disablePopover && 'wrap-filters')}
                 data-attr={'property-filter-' + index}
                 wrap={false}
             >

frontend/src/lib/components/PropertyFilters/components/TaxonomicPropertyFilter.tsx

@@ -131,16 +131,8 @@ export function TaxonomicPropertyFilter({
                             {orFiltering ? (
                                 <>
                                     {propertyGroupType && index !== 0 && filter?.key && (
-                                        <div>
-                                            {propertyGroupType === FilterLogicalOperator.And ? (
-                                                <span className="text-sm">
-                                                    <strong>{'&'}</strong>
-                                                </span>
-                                            ) : (
-                                                <span className="text-xs">
-                                                    <strong>{propertyGroupType}</strong>
-                                                </span>
-                                            )}
+                                        <div className="text-sm font-medium">
+                                            {propertyGroupType === FilterLogicalOperator.And ? '&' : propertyGroupType}
                                         </div>
                                     )}
                                 </>

frontend/src/lib/lemon-ui/LemonSelect/LemonSelect.stories.tsx

@@ -115,12 +115,12 @@ CustomElement.args = {
     options: [
         {
             value: 1,
-            labelInMenuExtra: <i> (Surprised)</i>,
+            labelInMenu: <i>Wow (Surprised)</i>,
             label: 'Wow',
         },
         {
             value: 2,
-            labelInMenuExtra: <i> (Blushing)</i>,
+            labelInMenu: <i>Ohh (Blushing)</i>,
             label: 'Ohh',
         },
     ],

frontend/src/lib/lemon-ui/LemonSelect/LemonSelect.tsx

@@ -16,18 +16,22 @@ import {
     isLemonMenuSection,
 } from '../LemonMenu/LemonMenu'
 
-type LemonSelectOptionBase = Omit<LemonMenuItemBase, 'active' | 'status'> // Select handles active state internally
+// Select options are basically menu items that handle onClick and active state internally
+interface LemonSelectOptionBase extends Omit<LemonMenuItemBase, 'active' | 'status'> {
+    /** Support this option if it already is selected, but otherwise don't allow selecting it by hiding it. */
+    hidden?: boolean
+}
 
+type LemonSelectCustomControl<T> = ({ onSelect }: { onSelect: OnSelect<T> }) => JSX.Element
 export interface LemonSelectOptionLeaf<T> extends LemonSelectOptionBase {
     value: T
-    /** Extra element shown next to the label in the select menu. */
-    labelInMenuExtra?: React.ReactElement
     /**
-     * If you really need something more advanced than a button, you can provide a custom control component.
-     * This will be displayed instead of the label in the select menu.
-     * Can be for example a textarea with a "Use custom expression" button hooked up to `onSelect`.
+     * Label for display inside the dropdown menu.
+     *
+     * If you really need something more advanced than a button, this also allows providing a custom control component,
+     * which takes an `onSelect` prop. Can be for example a textarea with an "Apply value" button. Use this sparingly!
      */
-    CustomControl?: ({ onSelect }: { onSelect: OnSelect<T> }) => JSX.Element
+    labelInMenu?: JSX.Element | LemonSelectCustomControl<T>
 }
 
 export interface LemonSelectOptionNode<T> extends LemonSelectOptionBase {
@@ -174,9 +178,9 @@ function convertSelectOptionsToMenuItems<T>(
     onSelect: OnSelect<T>
 ): [(LemonMenuItem | LemonMenuSection)[], LemonSelectOptionLeaf<T>[]] {
     const leafOptionsAccumulator: LemonSelectOptionLeaf<T>[] = []
-    const items: (LemonMenuItem | LemonMenuSection)[] = options.map((option) =>
-        convertToMenuSingle(option, activeValue, onSelect, leafOptionsAccumulator)
-    )
+    const items = options
+        .map((option) => convertToMenuSingle(option, activeValue, onSelect, leafOptionsAccumulator))
+        .filter(Boolean) as (LemonMenuItem | LemonMenuSection)[]
     return [items, leafOptionsAccumulator]
 }
 
@@ -185,37 +189,51 @@ function convertToMenuSingle<T>(
     activeValue: T | null,
     onSelect: OnSelect<T>,
     acc: LemonSelectOptionLeaf<T>[]
-): LemonMenuItem | LemonMenuSection {
+): LemonMenuItem | LemonMenuSection | null {
     if (isLemonSelectSection(option)) {
         const { options: childOptions, ...section } = option
+        const items = option.options.map((o) => convertToMenuSingle(o, activeValue, onSelect, acc)).filter(Boolean)
+        if (!items.length) {
+            // Add hidden options to the accumulator (by calling convertToMenuSingle), but don't show
+            return null
+        }
         return {
             ...section,
-            items: option.options.map((o) => convertToMenuSingle(o, activeValue, onSelect, acc)),
+            items,
         } as LemonMenuSection
     } else if (isLemonSelectOptionNode(option)) {
         const { options: childOptions, ...node } = option
+        const items = childOptions.map((o) => convertToMenuSingle(o, activeValue, onSelect, acc)).filter(Boolean)
+        if (option.hidden) {
+            // Add hidden options to the accumulator (by calling convertToMenuSingle), but don't show
+            return null
+        }
         return {
             ...node,
             active: doOptionsContainActiveValue(childOptions, activeValue),
-            items: childOptions.map((o) => convertToMenuSingle(o, activeValue, onSelect, acc)),
+            items,
         } as LemonMenuItemNode
     } else {
         acc.push(option)
-        const { value, label, labelInMenuExtra, CustomControl, ...leaf } = option
+        if (option.hidden) {
+            // Add hidden options to the accumulator, but don't show
+            return null
+        }
+        const { value, label, labelInMenu, ...leaf } = option
+        let CustomControl: LemonSelectCustomControl<T> | undefined
+        if (typeof labelInMenu === 'function') {
+            CustomControl = labelInMenu
+        }
         return {
             ...leaf,
-            label: CustomControl ? (
-                function LabelWrapped() {
-                    return <CustomControl onSelect={onSelect} />
-                }
-            ) : labelInMenuExtra ? (
-                <>
-                    {label}
-                    {labelInMenuExtra}
-                </>
-            ) : (
-                label
-            ),
+            label: CustomControl
+                ? function LabelWrapped() {
+                      if (!CustomControl) {
+                          throw new Error('CustomControl became undefined')
+                      }
+                      return <CustomControl onSelect={onSelect} />
+                  }
+                : labelInMenu || label,
             active: value === activeValue,
             onClick: () => onSelect(value),
         } as LemonMenuItemLeaf

frontend/src/queries/nodes/InsightViz/PropertyGroupFilters/AndOrFilterSelect.tsx

@@ -28,10 +28,10 @@ export function AndOrFilterSelect({
                     {
                         label: 'all',
                         value: FilterLogicalOperator.And,
-                        labelInMenuExtra: (
+                        labelInMenu: (
                             <SelectOption
-                                title="All filter"
-                                description="All filters must be met (logical and)"
+                                title="All"
+                                description="Every single filter must match"
                                 value={FilterLogicalOperator.And}
                                 selectedValue={value}
                             />
@@ -40,10 +40,10 @@ export function AndOrFilterSelect({
                     {
                         label: 'any',
                         value: FilterLogicalOperator.Or,
-                        labelInMenuExtra: (
+                        labelInMenu: (
                             <SelectOption
-                                title="Any filter"
-                                description="Any filter can be met (logical or)"
+                                title="Any"
+                                description="One or more filters must match"
                                 value={FilterLogicalOperator.Or}
                                 selectedValue={value}
                             />
@@ -67,13 +67,13 @@ type SelectOptionProps = {
 const SelectOption = ({ title, description, value, selectedValue }: SelectOptionProps): JSX.Element => (
     <div className="flex p-2 items-center">
         <div
-            className={`flex font-bold w-10 h-10 mr-3 justify-center items-center rounded text-xs ${
+            className={`flex shrink-0 font-bold w-10 h-10 mr-3 justify-center items-center rounded text-xs ${
                 value === selectedValue ? 'bg-primary text-white' : 'bg-mid text-primary-alt'
             }`}
         >
             {value}
         </div>
-        <div>
+        <div className="w-32">
             <div className="font-bold">{title}</div>
             <div className="font-normal">{description}</div>
         </div>

frontend/src/queries/nodes/InsightViz/PropertyGroupFilters/PropertyGroupFilters.tsx

@@ -78,19 +78,21 @@ export function PropertyGroupFilters({
                                                         }
                                                         value={group.type}
                                                     />
-                                                    <LemonDivider />
-                                                    <LemonButton
-                                                        icon={<IconCopy />}
-                                                        status="primary-alt"
-                                                        onClick={() => duplicateFilterGroup(propertyGroupIndex)}
-                                                        size="small"
-                                                    />
-                                                    <LemonButton
-                                                        icon={<IconDelete />}
-                                                        status="primary-alt"
-                                                        onClick={() => removeFilterGroup(propertyGroupIndex)}
-                                                        size="small"
-                                                    />
+                                                    <LemonDivider className="flex-1 mx-2" />
+                                                    <div className="flex items-center space-x-2">
+                                                        <LemonButton
+                                                            icon={<IconCopy />}
+                                                            status="primary-alt"
+                                                            onClick={() => duplicateFilterGroup(propertyGroupIndex)}
+                                                            size="small"
+                                                        />
+                                                        <LemonButton
+                                                            icon={<IconDelete />}
+                                                            status="primary-alt"
+                                                            onClick={() => removeFilterGroup(propertyGroupIndex)}
+                                                            size="small"
+                                                        />
+                                                    </div>
                                                 </div>
                                                 <PropertyFilters
                                                     addButton={