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(onboarding-templates): allow someone to select pageview as variable #24899

Merged
merged 7 commits into from
Sep 12, 2024
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
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,7 @@ export const authorizedUrlListLogic = kea<authorizedUrlListLogicType>([
[] as string[],
{
setAuthorizedUrls: (_, { authorizedUrls }) => authorizedUrls,
addUrl: (state, { url }) => state.concat([url]),
addUrl: (state, { url }) => (!state.includes(url) ? state.concat([url]) : state),
updateUrl: (state, { index, url }) => Object.assign([...state], { [index]: url }),
removeUrl: (state, { index }) => {
const newUrls = [...state]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ export function IframedToolbarBrowser({
}): JSX.Element | null {
const logic = iframedToolbarBrowserLogic({ iframeRef, userIntent: userIntent })

const { browserUrl } = useValues(logic)
const { browserUrl, initialPath } = useValues(logic)
const { onIframeLoad, setIframeWidth } = useActions(logic)

const { width: iframeWidth } = useResizeObserver<HTMLIFrameElement>({ ref: iframeRef })
Expand All @@ -55,7 +55,7 @@ export function IframedToolbarBrowser({
<iframe
ref={iframeRef}
className="w-full h-full"
src={appEditorUrl(browserUrl, {
src={appEditorUrl(browserUrl + '/' + initialPath, {
userIntent: userIntent,
})}
// eslint-disable-next-line react/forbid-dom-props
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,8 @@ export const iframedToolbarBrowserLogic = kea<iframedToolbarBrowserLogicType>([
disableElementSelector: true,
setNewActionName: (name: string | null) => ({ name }),
toolbarMessageReceived: (type: PostHogAppToolbarEvent, payload: Record<string, any>) => ({ type, payload }),
setCurrentPath: (path: string) => ({ path }),
setInitialPath: (path: string) => ({ path }),
}),

reducers(({ props }) => ({
Expand Down Expand Up @@ -99,6 +101,22 @@ export const iframedToolbarBrowserLogic = kea<iframedToolbarBrowserLogicType>([
setBrowserUrl: (_, { url }) => url,
},
],
currentPath: [
// this does not have the leading / because we always need that to be a given since this value is user-editable
'' as string,
{
setCurrentPath: (_, { path }) => path,
},
],
initialPath: [
// similar to currentPath, this also does not have the leading /
// this is used to set the initial browser URL if the user provides a path to navigate to
// we can't do this from within the iFrame with window.location.href = currentPath because we get XSS errors
'' as string,
{
setInitialPath: (_, { path }) => path,
},
],
loading: [
false as boolean,
{
Expand Down Expand Up @@ -133,6 +151,12 @@ export const iframedToolbarBrowserLogic = kea<iframedToolbarBrowserLogicType>([
return iframeWidth ? calculateViewportRange(heatmapFilters, iframeWidth) : { min: 0, max: 1800 }
},
],
currentFullUrl: [
(s) => [s.browserUrl, s.currentPath],
(browserUrl, currentPath) => {
return browserUrl + '/' + currentPath
},
],
}),

listeners(({ actions, cache, props, values }) => ({
Expand Down Expand Up @@ -255,6 +279,9 @@ export const iframedToolbarBrowserLogic = kea<iframedToolbarBrowserLogicType>([
actions.setNewActionName(null)
actions.disableElementSelector()
return
case PostHogAppToolbarEvent.PH_TOOLBAR_NAVIGATED:
// remove leading / from path
return actions.setCurrentPath(payload.path.replace(/^\/+/, ''))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function returns void so technically you should be doing something like this:

Suggested change
return actions.setCurrentPath(payload.path.replace(/^\/+/, ''))
actions.setCurrentPath(payload.path.replace(/^\/+/, ''))
return void

default:
console.warn(`[PostHog Heatmaps] Received unknown child window message: ${type}`)
}
Expand All @@ -270,7 +297,6 @@ export const iframedToolbarBrowserLogic = kea<iframedToolbarBrowserLogicType>([
actions.startTrackingLoading()
}
},

startTrackingLoading: () => {
actions.setIframeBanner(null)

Expand Down
1 change: 1 addition & 0 deletions frontend/src/lib/components/IframedToolbarBrowser/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ export enum PostHogAppToolbarEvent {
PH_ELEMENT_SELECTOR = 'ph-element-selector',
PH_NEW_ACTION_NAME = 'ph-new-action-name',
PH_NEW_ACTION_CREATED = 'ph-new-action-created',
PH_TOOLBAR_NAVIGATED = 'ph-toolbar-navigated',
}

export const DEFAULT_HEATMAP_FILTERS: HeatmapFilters = {
Expand Down
11 changes: 10 additions & 1 deletion frontend/src/lib/lemon-ui/LemonInputSelect/LemonInputSelect.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,9 @@ export type LemonInputSelectProps = Pick<
onInputChange?: (newValue: string) => void
'data-attr'?: string
popoverClassName?: string
size?: 'xsmall' | 'small' | 'medium' | 'large'
borderless?: boolean
transparentBackground?: boolean
}

export function LemonInputSelect({
Expand All @@ -65,6 +68,9 @@ export function LemonInputSelect({
autoFocus = false,
popoverClassName,
'data-attr': dataAttr,
size = 'medium',
borderless,
transparentBackground,
}: LemonInputSelectProps): JSX.Element {
const [showPopover, setShowPopover] = useState(false)
const [inputValue, _setInputValue] = useState('')
Expand Down Expand Up @@ -444,16 +450,19 @@ export function LemonInputSelect({
onKeyDown={_onKeyDown}
disabled={disabled}
autoFocus={autoFocus}
transparentBackground={transparentBackground}
className={clsx(
'flex-wrap h-auto min-w-24',
// Putting button-like text styling on the single-select unfocused placeholder
mode === 'single' && values.length > 0 && 'placeholder:*:font-medium',
mode === 'single' &&
values.length > 0 &&
!showPopover &&
'cursor-pointer placeholder:*:text-default'
'cursor-pointer placeholder:*:text-default',
borderless && 'border-none'
)}
data-attr={dataAttr}
size={size}
/>
</LemonDropdown>
)
Expand Down
14 changes: 7 additions & 7 deletions frontend/src/scenes/dashboard/Dashboards.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { urls } from 'scenes/urls'

import { mswDecorator } from '~/mocks/browser'
import { useAvailableFeatures } from '~/mocks/features'
import { DashboardMode } from '~/types'
import { BaseMathType, DashboardMode, EntityTypes } from '~/types'

import { dashboardTemplatesLogic } from './dashboards/templates/dashboardTemplatesLogic'

Expand Down Expand Up @@ -76,8 +76,8 @@ export const NewSelectVariables = (): JSX.Element => {
type: 'event',
default: {
id: '$pageview',
math: 'dau',
type: 'events',
math: BaseMathType.UniqueUsers,
type: EntityTypes.EVENTS,
},
required: true,
description: 'Add the current_url filter that matches your sign up page',
Expand All @@ -88,8 +88,8 @@ export const NewSelectVariables = (): JSX.Element => {
type: 'event',
default: {
id: '$pageview',
math: 'dau',
type: 'events',
math: BaseMathType.UniqueUsers,
type: EntityTypes.EVENTS,
},
required: true,
description:
Expand All @@ -101,8 +101,8 @@ export const NewSelectVariables = (): JSX.Element => {
type: 'event',
default: {
id: '$pageview',
math: 'dau',
type: 'events',
math: BaseMathType.UniqueUsers,
type: EntityTypes.EVENTS,
},
required: false,
description: 'Select the event which best represents when a user is activated',
Expand Down
27 changes: 25 additions & 2 deletions frontend/src/scenes/dashboard/dashboardTemplateVariablesLogic.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@ export interface DashboardTemplateVariablesLogicProps {

const FALLBACK_EVENT = {
id: '$pageview',
math: 'dau',
type: 'events',
math: BaseMathType.UniqueUsers,
type: EntityTypes.EVENTS,
}

export const dashboardTemplateVariablesLogic = kea<dashboardTemplateVariablesLogicType>([
Expand All @@ -39,6 +39,7 @@ export const dashboardTemplateVariablesLogic = kea<dashboardTemplateVariablesLog
filterGroup,
}),
setVariableFromAction: (variableName: string, action: ActionType) => ({ variableName, action }),
setVariableForPageview: (variableName: string, url: string) => ({ variableName, url }),
setActiveVariableIndex: (index: number) => ({ index }),
incrementActiveVariableIndex: true,
possiblyIncrementActiveVariableIndex: true,
Expand Down Expand Up @@ -162,6 +163,28 @@ export const dashboardTemplateVariablesLogic = kea<dashboardTemplateVariablesLog
actions.setVariable(originalVariableName, filterGroup)
actions.setIsCurrentlySelectingElement(false)
},
setVariableForPageview: ({ variableName, url }) => {
const step: TemplateVariableStep = {
id: '$pageview',
math: BaseMathType.UniqueUsers,
type: EntityTypes.EVENTS,
order: 0,
name: '$pageview',
properties: [
{
key: '$current_url',
value: url,
operator: 'icontains',
type: 'event',
},
],
}
const filterGroup: FilterType = {
events: [step],
}
actions.setVariable(variableName, filterGroup)
actions.setIsCurrentlySelectingElement(false)
},
toolbarMessageReceived: ({ type, payload }) => {
if (type === PostHogAppToolbarEvent.PH_NEW_ACTION_CREATED) {
actions.setVariableFromAction(payload.action.name, payload.action as ActionType)
Expand Down
2 changes: 1 addition & 1 deletion frontend/src/scenes/dashboard/newDashboardLogic.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ export function applyTemplate(obj: DashboardTile | JsonType, variables: Dashboar
const variableId = obj.substring(1, obj.length - 1)
const variable = variables.find((variable) => variable.id === variableId)
if (variable && variable.default) {
return variable.default
return variable.default as JsonType
}
return obj
}
Expand Down
Loading
Loading