From b39586197638d8b73da076703477c80efa2edd13 Mon Sep 17 00:00:00 2001 From: dylan Date: Thu, 6 Jun 2024 15:26:07 -0700 Subject: [PATCH 01/23] this works a treat --- src/extensions/surveys.tsx | 57 ++++++++++++++++++++---------------- src/posthog-surveys-types.ts | 3 +- 2 files changed, 34 insertions(+), 26 deletions(-) diff --git a/src/extensions/surveys.tsx b/src/extensions/surveys.tsx index c4dcd37ce..82826117a 100644 --- a/src/extensions/surveys.tsx +++ b/src/extensions/surveys.tsx @@ -200,7 +200,7 @@ export function SurveyPopup({ style?: React.CSSProperties previewPageIndex?: number | undefined }) { - const [isPopupVisible, setIsPopupVisible] = useState(true) + const [isPopupVisible, setIsPopupVisible] = useState(false) const [isSurveySent, setIsSurveySent] = useState(false) const shouldShowConfirmation = isSurveySent || previewPageIndex === survey.questions.length const isPreviewMode = Number.isInteger(previewPageIndex) @@ -219,33 +219,40 @@ export function SurveyPopup({ return } - window.dispatchEvent(new Event('PHSurveyShown')) - posthog.capture('survey shown', { - $survey_name: survey.name, - $survey_id: survey.id, - $survey_iteration: survey.current_iteration, - $survey_iteration_start_date: survey.current_iteration_start_date, - sessionRecordingUrl: posthog.get_session_replay_url?.(), - }) - localStorage.setItem(`lastSeenSurveyDate`, new Date().toISOString()) + const delay = survey.appearance?.surveyPopupDelay || 3000 + + const showPopup = () => { + setIsPopupVisible(true) + window.dispatchEvent(new Event('PHSurveyShown')) + posthog.capture('survey shown', { + $survey_name: survey.name, + $survey_id: survey.id, + $survey_iteration: survey.current_iteration, + $survey_iteration_start_date: survey.current_iteration_start_date, + sessionRecordingUrl: posthog.get_session_replay_url?.(), + }) + localStorage.setItem(`lastSeenSurveyDate`, new Date().toISOString()) - window.addEventListener('PHSurveyClosed', () => { - setIsPopupVisible(false) - }) - window.addEventListener('PHSurveySent', () => { - if (!survey.appearance?.displayThankYouMessage) { - return setIsPopupVisible(false) - } + window.addEventListener('PHSurveyClosed', () => { + setIsPopupVisible(false) + }) + window.addEventListener('PHSurveySent', () => { + if (!survey.appearance?.displayThankYouMessage) { + return setIsPopupVisible(false) + } - setIsSurveySent(true) + setIsSurveySent(true) - if (survey.appearance?.autoDisappear) { - setTimeout(() => { - setIsPopupVisible(false) - }, 5000) - } - }) - }, []) + if (survey.appearance?.autoDisappear) { + setTimeout(() => { + setIsPopupVisible(false) + }, 5000) + } + }) + } + + setTimeout(showPopup, delay) + }, [isPreviewMode, posthog, survey]) return isPopupVisible ? ( SurveyAppearance, but used in site app maxWidth?: string zIndex?: string - shuffleQuestions?: boolean } export enum SurveyType { From b252c5b161c64db392fe5748057f5111b4df7baf Mon Sep 17 00:00:00 2001 From: dylan Date: Thu, 6 Jun 2024 15:52:33 -0700 Subject: [PATCH 02/23] this works without looping survey send events --- src/extensions/surveys.tsx | 45 ++++++++++++++++++++++++-------------- 1 file changed, 29 insertions(+), 16 deletions(-) diff --git a/src/extensions/surveys.tsx b/src/extensions/surveys.tsx index 82826117a..6ffc96273 100644 --- a/src/extensions/surveys.tsx +++ b/src/extensions/surveys.tsx @@ -36,10 +36,15 @@ const handleWidget = (posthog: PostHog, survey: Survey) => { Preact.render(, shadow) } +const visibleSurveys = new Set() + export const callSurveys = (posthog: PostHog, forceReload: boolean = false) => { posthog?.getActiveMatchingSurveys((surveys) => { const nonAPISurveys = surveys.filter((survey) => survey.type !== 'api') nonAPISurveys.forEach((survey) => { + if (visibleSurveys.has(survey.id)) { + return // skip if survey is already visible + } if (survey.type === SurveyType.Widget) { if ( survey.appearance?.widgetType === 'tab' && @@ -99,6 +104,7 @@ export const callSurveys = (posthog: PostHog, forceReload: boolean = false) => { } if (!localStorage.getItem(getSurveySeenKey(survey))) { + visibleSurveys.add(survey.id) const shadow = createShadow(style(survey?.appearance), survey.id) Preact.render(, shadow) } @@ -206,7 +212,6 @@ export function SurveyPopup({ const isPreviewMode = Number.isInteger(previewPageIndex) const confirmationBoxLeftStyle = style?.left && isNumber(style?.left) ? { left: style.left - 40 } : {} - // Ensure the popup stays in the same position for the preview if (isPreviewMode) { style = style || {} style.left = 'unset' @@ -232,26 +237,34 @@ export function SurveyPopup({ sessionRecordingUrl: posthog.get_session_replay_url?.(), }) localStorage.setItem(`lastSeenSurveyDate`, new Date().toISOString()) + } - window.addEventListener('PHSurveyClosed', () => { - setIsPopupVisible(false) - }) - window.addEventListener('PHSurveySent', () => { - if (!survey.appearance?.displayThankYouMessage) { - return setIsPopupVisible(false) - } + const handleSurveyClosed = () => setIsPopupVisible(false) - setIsSurveySent(true) + const handleSurveySent = () => { + if (!survey.appearance?.displayThankYouMessage) { + return setIsPopupVisible(false) + } - if (survey.appearance?.autoDisappear) { - setTimeout(() => { - setIsPopupVisible(false) - }, 5000) - } - }) + setIsSurveySent(true) + + if (survey.appearance?.autoDisappear) { + setTimeout(() => { + setIsPopupVisible(false) + }, 5000) + } } - setTimeout(showPopup, delay) + const timeoutId = setTimeout(showPopup, delay) + + window.addEventListener('PHSurveyClosed', handleSurveyClosed) + window.addEventListener('PHSurveySent', handleSurveySent) + + return () => { + clearTimeout(timeoutId) + window.removeEventListener('PHSurveyClosed', handleSurveyClosed) + window.removeEventListener('PHSurveySent', handleSurveySent) + } }, [isPreviewMode, posthog, survey]) return isPopupVisible ? ( From c0067bce604a3e1e7a929716c44c71a2840d944d Mon Sep 17 00:00:00 2001 From: dylan Date: Thu, 6 Jun 2024 15:56:09 -0700 Subject: [PATCH 03/23] some conversion magic --- src/extensions/surveys.tsx | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/extensions/surveys.tsx b/src/extensions/surveys.tsx index 6ffc96273..8996bf1b1 100644 --- a/src/extensions/surveys.tsx +++ b/src/extensions/surveys.tsx @@ -224,7 +224,8 @@ export function SurveyPopup({ return } - const delay = survey.appearance?.surveyPopupDelay || 3000 + // surveyPopupDelay is passed to us in seconds, but we need to convert it to milliseconds + const delay = survey.appearance?.surveyPopupDelay ? survey.appearance.surveyPopupDelay * 1000 : 0 const showPopup = () => { setIsPopupVisible(true) From 852f5399d3a18638cbcc8f7596277e0f2611cae2 Mon Sep 17 00:00:00 2001 From: dylan Date: Thu, 6 Jun 2024 16:42:43 -0700 Subject: [PATCH 04/23] okay so this appears to work now --- src/extensions/surveys.tsx | 270 +++++++++++++++++++++++++++++++++---- 1 file changed, 245 insertions(+), 25 deletions(-) diff --git a/src/extensions/surveys.tsx b/src/extensions/surveys.tsx index 8996bf1b1..a13977dd0 100644 --- a/src/extensions/surveys.tsx +++ b/src/extensions/surveys.tsx @@ -193,6 +193,207 @@ export function generateSurveys(posthog: PostHog) { }, 3000) } +// export function SurveyPopup({ +// survey, +// forceDisableHtml, +// posthog, +// style, +// previewPageIndex, +// }: { +// survey: Survey +// forceDisableHtml?: boolean +// posthog?: PostHog +// style?: React.CSSProperties +// previewPageIndex?: number | undefined +// }) { +// const [isPopupVisible, setIsPopupVisible] = useState(false) +// const [isSurveySent, setIsSurveySent] = useState(false) +// const shouldShowConfirmation = isSurveySent || previewPageIndex === survey.questions.length +// const isPreviewMode = Number.isInteger(previewPageIndex) +// const confirmationBoxLeftStyle = style?.left && isNumber(style?.left) ? { left: style.left - 40 } : {} + +// if (isPreviewMode) { +// style = style || {} +// style.left = 'unset' +// style.right = 'unset' +// style.transform = 'unset' +// } + +// useEffect(() => { +// if (isPreviewMode || !posthog) { +// return +// } + +// // surveyPopupDelay is passed to us in seconds, but we need to convert it to milliseconds +// const delay = survey.appearance?.surveyPopupDelay ? survey.appearance.surveyPopupDelay * 1000 : 0 + +// const showPopup = () => { +// setIsPopupVisible(true) +// window.dispatchEvent(new Event('PHSurveyShown')) +// posthog.capture('survey shown', { +// $survey_name: survey.name, +// $survey_id: survey.id, +// $survey_iteration: survey.current_iteration, +// $survey_iteration_start_date: survey.current_iteration_start_date, +// sessionRecordingUrl: posthog.get_session_replay_url?.(), +// }) +// localStorage.setItem(`lastSeenSurveyDate`, new Date().toISOString()) +// } + +// const handleSurveyClosed = () => setIsPopupVisible(false) + +// const handleSurveySent = () => { +// if (!survey.appearance?.displayThankYouMessage) { +// return setIsPopupVisible(false) +// } + +// setIsSurveySent(true) + +// if (survey.appearance?.autoDisappear) { +// setTimeout(() => { +// setIsPopupVisible(false) +// }, 5000) +// } +// } + +// const timeoutId = setTimeout(showPopup, delay) + +// window.addEventListener('PHSurveyClosed', handleSurveyClosed) +// window.addEventListener('PHSurveySent', handleSurveySent) + +// return () => { +// clearTimeout(timeoutId) +// window.removeEventListener('PHSurveyClosed', handleSurveyClosed) +// window.removeEventListener('PHSurveySent', handleSurveySent) +// } +// }, [isPreviewMode, posthog, survey]) + +// return isPopupVisible ? ( +// dismissedSurveyEvent(survey, posthog, isPreviewMode), +// }} +// > +// {!shouldShowConfirmation ? ( +// +// ) : ( +// setIsPopupVisible(false)} +// /> +// )} +// +// ) : ( +// <> +// ) +// } + +// export function SurveyPopup({ +// survey, +// forceDisableHtml, +// posthog, +// style, +// previewPageIndex, +// }: { +// survey: Survey +// forceDisableHtml?: boolean +// posthog?: PostHog +// style?: React.CSSProperties +// previewPageIndex?: number | undefined +// }) { +// const [isPopupVisible, setIsPopupVisible] = useState(true) +// const [isSurveySent, setIsSurveySent] = useState(false) +// const shouldShowConfirmation = isSurveySent || previewPageIndex === survey.questions.length +// const isPreviewMode = Number.isInteger(previewPageIndex) +// const confirmationBoxLeftStyle = style?.left && isNumber(style?.left) ? { left: style.left - 40 } : {} + +// // Ensure the popup stays in the same position for the preview +// if (isPreviewMode) { +// style = style || {} +// style.left = 'unset' +// style.right = 'unset' +// style.transform = 'unset' +// } + +// useEffect(() => { +// if (isPreviewMode || !posthog) { +// return +// } + +// setIsPopupVisible(true) + +// window.dispatchEvent(new Event('PHSurveyShown')) +// posthog.capture('survey shown', { +// $survey_name: survey.name, +// $survey_id: survey.id, +// $survey_iteration: survey.current_iteration, +// $survey_iteration_start_date: survey.current_iteration_start_date, +// sessionRecordingUrl: posthog.get_session_replay_url?.(), +// }) +// localStorage.setItem(`lastSeenSurveyDate`, new Date().toISOString()) + +// window.addEventListener('PHSurveyClosed', () => { +// setIsPopupVisible(false) +// }) +// window.addEventListener('PHSurveySent', () => { +// if (!survey.appearance?.displayThankYouMessage) { +// return setIsPopupVisible(false) +// } + +// setIsSurveySent(true) + +// if (survey.appearance?.autoDisappear) { +// setTimeout(() => { +// setIsPopupVisible(false) +// }, 5000) +// } +// }) +// }, []) + +// return isPopupVisible ? ( +// dismissedSurveyEvent(survey, posthog, isPreviewMode), +// }} +// > +// {!shouldShowConfirmation ? ( +// +// ) : ( +// setIsPopupVisible(false)} +// /> +// )} +// +// ) : ( +// <> +// ) +// } + export function SurveyPopup({ survey, forceDisableHtml, @@ -206,12 +407,14 @@ export function SurveyPopup({ style?: React.CSSProperties previewPageIndex?: number | undefined }) { - const [isPopupVisible, setIsPopupVisible] = useState(false) + const isPreviewMode = Number.isInteger(previewPageIndex) + const delay = survey.appearance?.surveyPopupDelay ? survey.appearance.surveyPopupDelay * 1000 : 0 + const [isPopupVisible, setIsPopupVisible] = useState(isPreviewMode || delay === 0) const [isSurveySent, setIsSurveySent] = useState(false) const shouldShowConfirmation = isSurveySent || previewPageIndex === survey.questions.length - const isPreviewMode = Number.isInteger(previewPageIndex) const confirmationBoxLeftStyle = style?.left && isNumber(style?.left) ? { left: style.left - 40 } : {} + // Ensure the popup stays in the same position for the preview if (isPreviewMode) { style = style || {} style.left = 'unset' @@ -224,24 +427,10 @@ export function SurveyPopup({ return } - // surveyPopupDelay is passed to us in seconds, but we need to convert it to milliseconds - const delay = survey.appearance?.surveyPopupDelay ? survey.appearance.surveyPopupDelay * 1000 : 0 - - const showPopup = () => { - setIsPopupVisible(true) - window.dispatchEvent(new Event('PHSurveyShown')) - posthog.capture('survey shown', { - $survey_name: survey.name, - $survey_id: survey.id, - $survey_iteration: survey.current_iteration, - $survey_iteration_start_date: survey.current_iteration_start_date, - sessionRecordingUrl: posthog.get_session_replay_url?.(), - }) - localStorage.setItem(`lastSeenSurveyDate`, new Date().toISOString()) + const handleSurveyClosed = () => { + setIsPopupVisible(false) } - const handleSurveyClosed = () => setIsPopupVisible(false) - const handleSurveySent = () => { if (!survey.appearance?.displayThankYouMessage) { return setIsPopupVisible(false) @@ -256,17 +445,48 @@ export function SurveyPopup({ } } - const timeoutId = setTimeout(showPopup, delay) - window.addEventListener('PHSurveyClosed', handleSurveyClosed) window.addEventListener('PHSurveySent', handleSurveySent) - return () => { - clearTimeout(timeoutId) - window.removeEventListener('PHSurveyClosed', handleSurveyClosed) - window.removeEventListener('PHSurveySent', handleSurveySent) + if (delay > 0) { + const timeoutId = setTimeout(() => { + setIsPopupVisible(true) + + window.dispatchEvent(new Event('PHSurveyShown')) + posthog.capture('survey shown', { + $survey_name: survey.name, + $survey_id: survey.id, + $survey_iteration: survey.current_iteration, + $survey_iteration_start_date: survey.current_iteration_start_date, + sessionRecordingUrl: posthog.get_session_replay_url?.(), + }) + localStorage.setItem(`lastSeenSurveyDate`, new Date().toISOString()) + }, delay) + + return () => { + clearTimeout(timeoutId) + window.removeEventListener('PHSurveyClosed', handleSurveyClosed) + window.removeEventListener('PHSurveySent', handleSurveySent) + } + } else { + setIsPopupVisible(true) + + window.dispatchEvent(new Event('PHSurveyShown')) + posthog.capture('survey shown', { + $survey_name: survey.name, + $survey_id: survey.id, + $survey_iteration: survey.current_iteration, + $survey_iteration_start_date: survey.current_iteration_start_date, + sessionRecordingUrl: posthog.get_session_replay_url?.(), + }) + localStorage.setItem(`lastSeenSurveyDate`, new Date().toISOString()) + + return () => { + window.removeEventListener('PHSurveyClosed', handleSurveyClosed) + window.removeEventListener('PHSurveySent', handleSurveySent) + } } - }, [isPreviewMode, posthog, survey]) + }, []) return isPopupVisible ? ( Date: Thu, 6 Jun 2024 16:44:50 -0700 Subject: [PATCH 05/23] oops, don't need this --- src/extensions/surveys.tsx | 201 ------------------------------------- 1 file changed, 201 deletions(-) diff --git a/src/extensions/surveys.tsx b/src/extensions/surveys.tsx index a13977dd0..95f4d2b79 100644 --- a/src/extensions/surveys.tsx +++ b/src/extensions/surveys.tsx @@ -193,207 +193,6 @@ export function generateSurveys(posthog: PostHog) { }, 3000) } -// export function SurveyPopup({ -// survey, -// forceDisableHtml, -// posthog, -// style, -// previewPageIndex, -// }: { -// survey: Survey -// forceDisableHtml?: boolean -// posthog?: PostHog -// style?: React.CSSProperties -// previewPageIndex?: number | undefined -// }) { -// const [isPopupVisible, setIsPopupVisible] = useState(false) -// const [isSurveySent, setIsSurveySent] = useState(false) -// const shouldShowConfirmation = isSurveySent || previewPageIndex === survey.questions.length -// const isPreviewMode = Number.isInteger(previewPageIndex) -// const confirmationBoxLeftStyle = style?.left && isNumber(style?.left) ? { left: style.left - 40 } : {} - -// if (isPreviewMode) { -// style = style || {} -// style.left = 'unset' -// style.right = 'unset' -// style.transform = 'unset' -// } - -// useEffect(() => { -// if (isPreviewMode || !posthog) { -// return -// } - -// // surveyPopupDelay is passed to us in seconds, but we need to convert it to milliseconds -// const delay = survey.appearance?.surveyPopupDelay ? survey.appearance.surveyPopupDelay * 1000 : 0 - -// const showPopup = () => { -// setIsPopupVisible(true) -// window.dispatchEvent(new Event('PHSurveyShown')) -// posthog.capture('survey shown', { -// $survey_name: survey.name, -// $survey_id: survey.id, -// $survey_iteration: survey.current_iteration, -// $survey_iteration_start_date: survey.current_iteration_start_date, -// sessionRecordingUrl: posthog.get_session_replay_url?.(), -// }) -// localStorage.setItem(`lastSeenSurveyDate`, new Date().toISOString()) -// } - -// const handleSurveyClosed = () => setIsPopupVisible(false) - -// const handleSurveySent = () => { -// if (!survey.appearance?.displayThankYouMessage) { -// return setIsPopupVisible(false) -// } - -// setIsSurveySent(true) - -// if (survey.appearance?.autoDisappear) { -// setTimeout(() => { -// setIsPopupVisible(false) -// }, 5000) -// } -// } - -// const timeoutId = setTimeout(showPopup, delay) - -// window.addEventListener('PHSurveyClosed', handleSurveyClosed) -// window.addEventListener('PHSurveySent', handleSurveySent) - -// return () => { -// clearTimeout(timeoutId) -// window.removeEventListener('PHSurveyClosed', handleSurveyClosed) -// window.removeEventListener('PHSurveySent', handleSurveySent) -// } -// }, [isPreviewMode, posthog, survey]) - -// return isPopupVisible ? ( -// dismissedSurveyEvent(survey, posthog, isPreviewMode), -// }} -// > -// {!shouldShowConfirmation ? ( -// -// ) : ( -// setIsPopupVisible(false)} -// /> -// )} -// -// ) : ( -// <> -// ) -// } - -// export function SurveyPopup({ -// survey, -// forceDisableHtml, -// posthog, -// style, -// previewPageIndex, -// }: { -// survey: Survey -// forceDisableHtml?: boolean -// posthog?: PostHog -// style?: React.CSSProperties -// previewPageIndex?: number | undefined -// }) { -// const [isPopupVisible, setIsPopupVisible] = useState(true) -// const [isSurveySent, setIsSurveySent] = useState(false) -// const shouldShowConfirmation = isSurveySent || previewPageIndex === survey.questions.length -// const isPreviewMode = Number.isInteger(previewPageIndex) -// const confirmationBoxLeftStyle = style?.left && isNumber(style?.left) ? { left: style.left - 40 } : {} - -// // Ensure the popup stays in the same position for the preview -// if (isPreviewMode) { -// style = style || {} -// style.left = 'unset' -// style.right = 'unset' -// style.transform = 'unset' -// } - -// useEffect(() => { -// if (isPreviewMode || !posthog) { -// return -// } - -// setIsPopupVisible(true) - -// window.dispatchEvent(new Event('PHSurveyShown')) -// posthog.capture('survey shown', { -// $survey_name: survey.name, -// $survey_id: survey.id, -// $survey_iteration: survey.current_iteration, -// $survey_iteration_start_date: survey.current_iteration_start_date, -// sessionRecordingUrl: posthog.get_session_replay_url?.(), -// }) -// localStorage.setItem(`lastSeenSurveyDate`, new Date().toISOString()) - -// window.addEventListener('PHSurveyClosed', () => { -// setIsPopupVisible(false) -// }) -// window.addEventListener('PHSurveySent', () => { -// if (!survey.appearance?.displayThankYouMessage) { -// return setIsPopupVisible(false) -// } - -// setIsSurveySent(true) - -// if (survey.appearance?.autoDisappear) { -// setTimeout(() => { -// setIsPopupVisible(false) -// }, 5000) -// } -// }) -// }, []) - -// return isPopupVisible ? ( -// dismissedSurveyEvent(survey, posthog, isPreviewMode), -// }} -// > -// {!shouldShowConfirmation ? ( -// -// ) : ( -// setIsPopupVisible(false)} -// /> -// )} -// -// ) : ( -// <> -// ) -// } - export function SurveyPopup({ survey, forceDisableHtml, From bd6e9c019cdacd1c472df08bfab7a3c57be8f200 Mon Sep 17 00:00:00 2001 From: dylan Date: Fri, 7 Jun 2024 16:04:14 -0700 Subject: [PATCH 06/23] added better testing, woohoo --- package.json | 2 + pnpm-lock.yaml | 103 +++++++++++++++++++++++ src/__tests__/extensions/surveys.test.ts | 102 +++++++++++++++++++++- src/extensions/surveys.tsx | 66 +++++++++------ 4 files changed, 247 insertions(+), 26 deletions(-) diff --git a/package.json b/package.json index 135d89510..d7f61201e 100644 --- a/package.json +++ b/package.json @@ -50,6 +50,8 @@ "@rrweb/types": "2.0.0-alpha.13", "@sentry/types": "7.37.2", "@testing-library/dom": "^9.3.0", + "@testing-library/jest-dom": "^6.4.5", + "@testing-library/preact": "^3.2.4", "@types/eslint": "^8.44.6", "@types/jest": "^29.5.1", "@types/react-dom": "^18.0.10", diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index fe32c36bc..041ba94e1 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -57,6 +57,12 @@ devDependencies: '@testing-library/dom': specifier: ^9.3.0 version: 9.3.0 + '@testing-library/jest-dom': + specifier: ^6.4.5 + version: 6.4.5(@types/jest@29.5.1)(jest@27.5.1) + '@testing-library/preact': + specifier: ^3.2.4 + version: 3.2.4(preact@10.19.3) '@types/eslint': specifier: ^8.44.6 version: 8.44.6 @@ -209,6 +215,10 @@ packages: engines: {node: '>=0.10.0'} dev: true + /@adobe/css-tools@4.4.0: + resolution: {integrity: sha512-Ff9+ksdQQB3rMncgqDK78uLznstjyfIf2Arnh22pW8kBpLs6rpKDwgnZT46hin5Hl1WzazzK64DOrhSwYpS7bQ==} + dev: true + /@ampproject/remapping@2.2.0: resolution: {integrity: sha512-qRmjj8nj9qmLTQXXmaR1cck3UXSRMPrbsLJAasZpF+t3riI71BXed5ebIOYwQntykeZuhjsdweEc9BxH5Jc26w==} engines: {node: '>=6.0.0'} @@ -2710,6 +2720,20 @@ packages: resolution: {integrity: sha512-+iTbntw2IZPb/anVDbypzfQa+ay64MW0Zo8aJ8gZPWMMK6/OubMVb6lUPMagqjOPnmtauXnFCACVl3O7ogjeqQ==} dev: true + /@testing-library/dom@8.20.1: + resolution: {integrity: sha512-/DiOQ5xBxgdYRC8LNk7U+RWat0S3qRLeIw3ZIkMQ9kkVlRmwD/Eg8k8CqIpD6GW7u20JIUOfMKbxtiLutpjQ4g==} + engines: {node: '>=12'} + dependencies: + '@babel/code-frame': 7.23.5 + '@babel/runtime': 7.13.9 + '@types/aria-query': 5.0.1 + aria-query: 5.1.3 + chalk: 4.1.2 + dom-accessibility-api: 0.5.16 + lz-string: 1.5.0 + pretty-format: 27.5.1 + dev: true + /@testing-library/dom@9.3.0: resolution: {integrity: sha512-Dffe68pGwI6WlLRYR2I0piIkyole9cSBH5jGQKCGMRpHW5RHCqAUaqc2Kv0tUyd4dU4DLPKhJIjyKOnjv4tuUw==} engines: {node: '>=14'} @@ -2724,6 +2748,49 @@ packages: pretty-format: 27.5.1 dev: true + /@testing-library/jest-dom@6.4.5(@types/jest@29.5.1)(jest@27.5.1): + resolution: {integrity: sha512-AguB9yvTXmCnySBP1lWjfNNUwpbElsaQ567lt2VdGqAdHtpieLgjmcVyv1q7PMIvLbgpDdkWV5Ydv3FEejyp2A==} + engines: {node: '>=14', npm: '>=6', yarn: '>=1'} + peerDependencies: + '@jest/globals': '>= 28' + '@types/bun': latest + '@types/jest': '>= 28' + jest: '>= 28' + vitest: '>= 0.32' + peerDependenciesMeta: + '@jest/globals': + optional: true + '@types/bun': + optional: true + '@types/jest': + optional: true + jest: + optional: true + vitest: + optional: true + dependencies: + '@adobe/css-tools': 4.4.0 + '@babel/runtime': 7.13.9 + '@types/jest': 29.5.1 + aria-query: 5.1.3 + chalk: 3.0.0 + css.escape: 1.5.1 + dom-accessibility-api: 0.6.3 + jest: 27.5.1 + lodash: 4.17.21 + redent: 3.0.0 + dev: true + + /@testing-library/preact@3.2.4(preact@10.19.3): + resolution: {integrity: sha512-F+kJ243LP6VmEK1M809unzTE/ijg+bsMNuiRN0JEDIJBELKKDNhdgC/WrUSZ7klwJvtlO3wQZ9ix+jhObG07Fg==} + engines: {node: '>= 12'} + peerDependencies: + preact: '>=10 || ^10.0.0-alpha.0 || ^10.0.0-beta.0' + dependencies: + '@testing-library/dom': 8.20.1 + preact: 10.19.3 + dev: true + /@tootallnate/once@1.1.2: resolution: {integrity: sha512-RbzJvlNzmRq5c3O09UipeuXno4tA1FE6ikOjxZK0tuxVv3412l64l5t1W5pj4+rJq9vpkm/kwiR07aZXnsKPxw==} engines: {node: '>= 6'} @@ -4027,6 +4094,14 @@ packages: supports-color: 5.5.0 dev: true + /chalk@3.0.0: + resolution: {integrity: sha512-4D3B6Wf41KOYRFdszmDqMCGq5VV/uMAB273JILmO+3jAlh8X4qDtdtgCR3fxtbLEMzSx22QdhnDcJvu2u1fVwg==} + engines: {node: '>=8'} + dependencies: + ansi-styles: 4.2.1 + supports-color: 7.1.0 + dev: true + /chalk@4.1.1: resolution: {integrity: sha512-diHzdDKxcU+bAsUboHLPEDQiw0qEe0qd7SYUn3HgcFlWgbDcfLGswOHYeGrHKzG9z6UYf01d9VFMfZxPM1xZSg==} engines: {node: '>=10'} @@ -4359,6 +4434,10 @@ packages: engines: {iojs: '>=1.0.0', node: '>=0.5.2'} dev: true + /css.escape@1.5.1: + resolution: {integrity: sha512-YUifsXXuknHlUsmlgyY0PKzgPOr7/FjCePfHNt0jxm83wHZi44VDMQ7/fGNkjY3/jV1MC+1CmZbaHzugyeRtpg==} + dev: true + /css@2.2.3: resolution: {integrity: sha512-0W171WccAjQGGTKLhw4m2nnl0zPHUlTO/I8td4XzJgIB8Hg3ZZx71qT4G4eX8OVsSiaAKiUMy73E3nsbPlg2DQ==} dependencies: @@ -4719,6 +4798,10 @@ packages: resolution: {integrity: sha512-X7BJ2yElsnOJ30pZF4uIIDfBEVgF4XEBxL9Bxhy6dnrm5hkzqmsWHGTiHqRiITNhMyFLyAiWndIJP7Z1NTteDg==} dev: true + /dom-accessibility-api@0.6.3: + resolution: {integrity: sha512-7ZgogeTnjuHbo+ct10G9Ffp0mif17idi0IyWNVA/wcwcm7NPOD/WEHVP3n7n3MhXqxoIYm8d6MuZohYWIZ4T3w==} + dev: true + /dom-walk@0.1.2: resolution: {integrity: sha512-6QvTW9mrGeIegrFXdtQi9pk7O/nSK6lSdXW2eqUspN5LWD7UTji2Fqw5V2YLjBpHEoU9Xl/eUWNpDeZvoyOv2w==} dev: true @@ -7948,6 +8031,11 @@ packages: dom-walk: 0.1.2 dev: true + /min-indent@1.0.1: + resolution: {integrity: sha512-I9jwMn07Sy/IwOj3zVkVik2JTvgpaykDZEigL6Rx6N9LbMywwUSMtxET+7lVoDLLd3O3IXwJwvuuns8UB/HeAg==} + engines: {node: '>=4'} + dev: true + /minimatch@3.1.2: resolution: {integrity: sha512-J7p63hRiAjw1NDEww1W7i37+ByIrOWO5XQQAzZ3VOcL0PNybwpfmV/N05zFAzwQ9USyEcX6t3UO+K5aqBQOIHw==} dependencies: @@ -8882,6 +8970,14 @@ packages: picomatch: 2.3.1 dev: true + /redent@3.0.0: + resolution: {integrity: sha512-6tDA8g98We0zd0GvVeMT9arEOnTw9qM03L9cJXaCjrip1OO764RDBLBfrB4cwzNGDj5OA5ioymC9GkizgWJDUg==} + engines: {node: '>=8'} + dependencies: + indent-string: 4.0.0 + strip-indent: 3.0.0 + dev: true + /regenerate-unicode-properties@10.0.1: resolution: {integrity: sha512-vn5DU6yg6h8hP/2OkQo3K7uVILvY4iu0oI4t3HFa81UPkhGJwkRwM10JEc3upjdhHjs/k8GJY1sRBhk5sr69Bw==} engines: {node: '>=4'} @@ -9752,6 +9848,13 @@ packages: engines: {node: '>=6'} dev: true + /strip-indent@3.0.0: + resolution: {integrity: sha512-laJTa3Jb+VQpaC6DseHhF7dXVqHTfJPCRDaEbid/drOhgitgYku/letMUqOXFoWV0zIIUbjpdH2t+tYj4bQMRQ==} + engines: {node: '>=8'} + dependencies: + min-indent: 1.0.1 + dev: true + /strip-json-comments@3.1.1: resolution: {integrity: sha512-6fPc+R4ihwqP6N/aIv2f1gMH8lOVtWQHoqC4yK6oSDVVocumAsfCqjkXnqiYMhmMwS/mEHLp7Vehlt3ql6lEig==} engines: {node: '>=8'} diff --git a/src/__tests__/extensions/surveys.test.ts b/src/__tests__/extensions/surveys.test.ts index 1ca29ce76..1732848ab 100644 --- a/src/__tests__/extensions/surveys.test.ts +++ b/src/__tests__/extensions/surveys.test.ts @@ -1,6 +1,14 @@ -import { generateSurveys, renderSurveysPreview, renderFeedbackWidgetPreview } from '../../extensions/surveys' +import { + generateSurveys, + renderSurveysPreview, + renderFeedbackWidgetPreview, + usePopupVisibility, +} from '../../extensions/surveys' import { createShadow } from '../../extensions/surveys/surveys-utils' import { Survey, SurveyQuestionType, SurveyType } from '../../posthog-surveys-types' +import { renderHook, act } from '@testing-library/preact' +import '@testing-library/jest-dom' +import { PostHog } from '../../posthog-core' describe('survey display logic', () => { beforeEach(() => { @@ -37,6 +45,7 @@ describe('survey display logic', () => { ], }, ] + const mockPostHog = { getActiveMatchingSurveys: jest.fn().mockImplementation((callback) => callback(mockSurveys)), get_session_replay_url: jest.fn(), @@ -56,6 +65,97 @@ describe('survey display logic', () => { }) }) +describe('usePopupVisibility', () => { + const mockSurvey = { + id: 'testSurvey1', + name: 'Test survey 1', + type: SurveyType.Popover, + appearance: {}, + start_date: '2021-01-01T00:00:00.000Z', + questions: [ + { + question: 'How satisfied are you with our newest product?', + description: 'This is a question description', + type: SurveyQuestionType.Rating, + display: 'number', + scale: 10, + lowerBoundLabel: 'Not Satisfied', + upperBoundLabel: 'Very Satisfied', + }, + ], + conditions: {}, + end_date: null, + targeting_flag_key: null, + } as Survey + + const mockPostHog = { + getActiveMatchingSurveys: jest.fn().mockImplementation((callback) => callback([mockSurvey])), + get_session_replay_url: jest.fn(), + capture: jest.fn().mockImplementation((eventName) => eventName), + } as unknown as PostHog + + test('should set isPopupVisible to true immediately if delay is 0', () => { + const { result } = renderHook(() => usePopupVisibility(mockSurvey, mockPostHog, 0, false)) + expect(result.current.isPopupVisible).toBe(true) + }) + + test('should set isPopupVisible to true after delay', () => { + jest.useFakeTimers() + const { result } = renderHook(() => usePopupVisibility(mockSurvey, mockPostHog, 1000, false)) + expect(result.current.isPopupVisible).toBe(false) + act(() => { + jest.advanceTimersByTime(1000) + }) + expect(result.current.isPopupVisible).toBe(true) + jest.useRealTimers() + }) + + test('should hide popup when PHSurveyClosed event is dispatched', () => { + const { result } = renderHook(() => usePopupVisibility(mockSurvey, mockPostHog, 0, false)) + act(() => { + window.dispatchEvent(new Event('PHSurveyClosed')) + }) + expect(result.current.isPopupVisible).toBe(false) + }) + + test('should show thank you message when survey is sent and handle auto disappear', () => { + jest.useFakeTimers() + mockSurvey.appearance = { + displayThankYouMessage: true, + autoDisappear: true, + thankYouMessageHeader: 'Thank you!', + thankYouMessageDescription: 'We appreciate your feedback.', + } + + const { result } = renderHook(() => usePopupVisibility(mockSurvey, mockPostHog, 0, false)) + act(() => { + window.dispatchEvent(new Event('PHSurveySent')) + }) + + expect(result.current.isSurveySent).toBe(true) + expect(result.current.isPopupVisible).toBe(true) + + act(() => { + jest.advanceTimersByTime(5000) + }) + + expect(result.current.isPopupVisible).toBe(false) + jest.useRealTimers() + }) + + test('should clean up event listeners and timers on unmount', () => { + jest.useFakeTimers() + const { unmount } = renderHook(() => usePopupVisibility(mockSurvey, mockPostHog, 1000, false)) + const removeEventListenerSpy = jest.spyOn(window, 'removeEventListener') + + unmount() + + expect(removeEventListenerSpy).toHaveBeenCalledWith('PHSurveyClosed', expect.any(Function)) + expect(removeEventListenerSpy).toHaveBeenCalledWith('PHSurveySent', expect.any(Function)) + jest.useRealTimers() + }) +}) + describe('preview renders', () => { beforeEach(() => { // we have to manually reset the DOM before each test diff --git a/src/extensions/surveys.tsx b/src/extensions/surveys.tsx index 95f4d2b79..cb63d2f5e 100644 --- a/src/extensions/surveys.tsx +++ b/src/extensions/surveys.tsx @@ -193,33 +193,14 @@ export function generateSurveys(posthog: PostHog) { }, 3000) } -export function SurveyPopup({ - survey, - forceDisableHtml, - posthog, - style, - previewPageIndex, -}: { - survey: Survey - forceDisableHtml?: boolean - posthog?: PostHog - style?: React.CSSProperties - previewPageIndex?: number | undefined -}) { - const isPreviewMode = Number.isInteger(previewPageIndex) - const delay = survey.appearance?.surveyPopupDelay ? survey.appearance.surveyPopupDelay * 1000 : 0 +export function usePopupVisibility( + survey: Survey, + posthog: PostHog | undefined, + delay: number, + isPreviewMode: boolean +) { const [isPopupVisible, setIsPopupVisible] = useState(isPreviewMode || delay === 0) const [isSurveySent, setIsSurveySent] = useState(false) - const shouldShowConfirmation = isSurveySent || previewPageIndex === survey.questions.length - const confirmationBoxLeftStyle = style?.left && isNumber(style?.left) ? { left: style.left - 40 } : {} - - // Ensure the popup stays in the same position for the preview - if (isPreviewMode) { - style = style || {} - style.left = 'unset' - style.right = 'unset' - style.transform = 'unset' - } useEffect(() => { if (isPreviewMode || !posthog) { @@ -287,6 +268,41 @@ export function SurveyPopup({ } }, []) + return { isPopupVisible, setIsPopupVisible, isSurveySent, setIsSurveySent } +} + +export function SurveyPopup({ + survey, + forceDisableHtml, + posthog, + style, + previewPageIndex, +}: { + survey: Survey + forceDisableHtml?: boolean + posthog?: PostHog + style?: React.CSSProperties + previewPageIndex?: number | undefined +}) { + const isPreviewMode = Number.isInteger(previewPageIndex) + const delay = survey.appearance?.surveyPopupDelay ? survey.appearance.surveyPopupDelay * 1000 : 0 + + const { isPopupVisible, isSurveySent, setIsPopupVisible } = usePopupVisibility( + survey, + posthog, + delay, + isPreviewMode + ) + const shouldShowConfirmation = isSurveySent || previewPageIndex === survey.questions.length + const confirmationBoxLeftStyle = style?.left && isNumber(style?.left) ? { left: style.left - 40 } : {} + + if (isPreviewMode) { + style = style || {} + style.left = 'unset' + style.right = 'unset' + style.transform = 'unset' + } + return isPopupVisible ? ( Date: Mon, 10 Jun 2024 16:15:53 -0700 Subject: [PATCH 07/23] this WORKS --- src/extensions/surveys.tsx | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/src/extensions/surveys.tsx b/src/extensions/surveys.tsx index cb63d2f5e..7eba5a7bb 100644 --- a/src/extensions/surveys.tsx +++ b/src/extensions/surveys.tsx @@ -37,12 +37,21 @@ const handleWidget = (posthog: PostHog, survey: Survey) => { } const visibleSurveys = new Set() +let processingSurveyId: string | null = null export const callSurveys = (posthog: PostHog, forceReload: boolean = false) => { posthog?.getActiveMatchingSurveys((surveys) => { const nonAPISurveys = surveys.filter((survey) => survey.type !== 'api') + + // Sort the surveys by surveyPopupDelay (ascending order) + nonAPISurveys.sort((a, b) => { + const delayA = a.appearance?.surveyPopupDelay || 0 + const delayB = b.appearance?.surveyPopupDelay || 0 + return delayA - delayB + }) + nonAPISurveys.forEach((survey) => { - if (visibleSurveys.has(survey.id)) { + if (visibleSurveys.has(survey.id) || processingSurveyId) { return // skip if survey is already visible } if (survey.type === SurveyType.Widget) { @@ -105,6 +114,7 @@ export const callSurveys = (posthog: PostHog, forceReload: boolean = false) => { if (!localStorage.getItem(getSurveySeenKey(survey))) { visibleSurveys.add(survey.id) + processingSurveyId = survey.id // Set the processing survey const shadow = createShadow(style(survey?.appearance), survey.id) Preact.render(, shadow) } @@ -209,10 +219,12 @@ export function usePopupVisibility( const handleSurveyClosed = () => { setIsPopupVisible(false) + processingSurveyId = null // Clear the processing survey } const handleSurveySent = () => { if (!survey.appearance?.displayThankYouMessage) { + processingSurveyId = null // Clear the processing survey return setIsPopupVisible(false) } @@ -220,7 +232,8 @@ export function usePopupVisibility( if (survey.appearance?.autoDisappear) { setTimeout(() => { - setIsPopupVisible(false) + processingSurveyId = null // Clear the processing survey + return setIsPopupVisible(false) }, 5000) } } @@ -240,6 +253,7 @@ export function usePopupVisibility( $survey_iteration_start_date: survey.current_iteration_start_date, sessionRecordingUrl: posthog.get_session_replay_url?.(), }) + processingSurveyId = null // Clear the processing survey localStorage.setItem(`lastSeenSurveyDate`, new Date().toISOString()) }, delay) @@ -268,7 +282,7 @@ export function usePopupVisibility( } }, []) - return { isPopupVisible, setIsPopupVisible, isSurveySent, setIsSurveySent } + return { isPopupVisible, isSurveySent, setIsPopupVisible } } export function SurveyPopup({ From 86ed4db0429b1b73e8214c0c74d1db7bca8b2e08 Mon Sep 17 00:00:00 2001 From: dylan Date: Tue, 11 Jun 2024 10:57:58 -0700 Subject: [PATCH 08/23] confirmed that this works for my use case --- src/extensions/surveys.tsx | 134 +++++++++++++++++++------------------ 1 file changed, 70 insertions(+), 64 deletions(-) diff --git a/src/extensions/surveys.tsx b/src/extensions/surveys.tsx index 7eba5a7bb..8356fb52f 100644 --- a/src/extensions/surveys.tsx +++ b/src/extensions/surveys.tsx @@ -29,15 +29,8 @@ import { const window = _window as Window & typeof globalThis const document = _document as Document -const handleWidget = (posthog: PostHog, survey: Survey) => { - const shadow = createWidgetShadow(survey) - const surveyStyleSheet = style(survey.appearance) - shadow.appendChild(Object.assign(document.createElement('style'), { innerText: surveyStyleSheet })) - Preact.render(, shadow) -} - -const visibleSurveys = new Set() -let processingSurveyId: string | null = null +const displayedSurveys = new Set() +let surveyIdBeingDisplayed: string | null = null // TODO this could be a bool instead of a string, we don't need to know the id export const callSurveys = (posthog: PostHog, forceReload: boolean = false) => { posthog?.getActiveMatchingSurveys((surveys) => { @@ -51,7 +44,7 @@ export const callSurveys = (posthog: PostHog, forceReload: boolean = false) => { }) nonAPISurveys.forEach((survey) => { - if (visibleSurveys.has(survey.id) || processingSurveyId) { + if (displayedSurveys.has(survey.id) || surveyIdBeingDisplayed) { return // skip if survey is already visible } if (survey.type === SurveyType.Widget) { @@ -113,8 +106,8 @@ export const callSurveys = (posthog: PostHog, forceReload: boolean = false) => { } if (!localStorage.getItem(getSurveySeenKey(survey))) { - visibleSurveys.add(survey.id) - processingSurveyId = survey.id // Set the processing survey + displayedSurveys.add(survey.id) + surveyIdBeingDisplayed = survey.id // Set the processing survey const shadow = createShadow(style(survey?.appearance), survey.id) Preact.render(, shadow) } @@ -166,6 +159,7 @@ export const renderSurveysPreview = ({ parentElement ) } + export const renderFeedbackWidgetPreview = ({ survey, root, @@ -197,10 +191,10 @@ export function generateSurveys(posthog: PostHog) { } callSurveys(posthog, true) - // recalculate surveys every 3 seconds to check if URL or selectors have changed + // recalculate surveys every second to check if URL or selectors have changed setInterval(() => { callSurveys(posthog, false) - }, 3000) + }, 1000) } export function usePopupVisibility( @@ -218,13 +212,15 @@ export function usePopupVisibility( } const handleSurveyClosed = () => { - setIsPopupVisible(false) - processingSurveyId = null // Clear the processing survey + displayedSurveys.delete(survey.id) + surveyIdBeingDisplayed = null // Clear the processing survey + return setIsPopupVisible(false) } const handleSurveySent = () => { if (!survey.appearance?.displayThankYouMessage) { - processingSurveyId = null // Clear the processing survey + displayedSurveys.delete(survey.id) + surveyIdBeingDisplayed = null // Clear the processing survey return setIsPopupVisible(false) } @@ -232,7 +228,8 @@ export function usePopupVisibility( if (survey.appearance?.autoDisappear) { setTimeout(() => { - processingSurveyId = null // Clear the processing survey + displayedSurveys.delete(survey.id) + surveyIdBeingDisplayed = null // Clear the processing survey return setIsPopupVisible(false) }, 5000) } @@ -253,7 +250,7 @@ export function usePopupVisibility( $survey_iteration_start_date: survey.current_iteration_start_date, sessionRecordingUrl: posthog.get_session_replay_url?.(), }) - processingSurveyId = null // Clear the processing survey + surveyIdBeingDisplayed = null // Clear the processing survey // TODO WHY DO THIS HERE? localStorage.setItem(`lastSeenSurveyDate`, new Date().toISOString()) }, delay) @@ -322,7 +319,8 @@ export function SurveyPopup({ value={{ isPreviewMode, previewPageIndex: previewPageIndex, - handleCloseSurveyPopup: () => dismissedSurveyEvent(survey, posthog, isPreviewMode), + handleCloseSurveyPopup: () => + displayedSurveys.delete(survey.id) && dismissedSurveyEvent(survey, posthog, isPreviewMode), }} > {!shouldShowConfirmation ? ( @@ -349,50 +347,6 @@ export function SurveyPopup({ ) } -interface GetQuestionComponentProps { - question: SurveyQuestion - forceDisableHtml: boolean - displayQuestionIndex: number - appearance: SurveyAppearance - onSubmit: (res: string | string[] | number | null) => void -} - -const getQuestionComponent = ({ - question, - forceDisableHtml, - displayQuestionIndex, - appearance, - onSubmit, -}: GetQuestionComponentProps): JSX.Element => { - const questionComponents = { - [SurveyQuestionType.Open]: OpenTextQuestion, - [SurveyQuestionType.Link]: LinkQuestion, - [SurveyQuestionType.Rating]: RatingQuestion, - [SurveyQuestionType.SingleChoice]: MultipleChoiceQuestion, - [SurveyQuestionType.MultipleChoice]: MultipleChoiceQuestion, - } - - const commonProps = { - question, - forceDisableHtml, - appearance, - onSubmit, - } - - const additionalProps: Record = { - [SurveyQuestionType.Open]: {}, - [SurveyQuestionType.Link]: {}, - [SurveyQuestionType.Rating]: { displayQuestionIndex }, - [SurveyQuestionType.SingleChoice]: { displayQuestionIndex }, - [SurveyQuestionType.MultipleChoice]: { displayQuestionIndex }, - } - - const Component = questionComponents[question.type] - const componentProps = { ...commonProps, ...additionalProps[question.type] } - - return -} - export function Questions({ survey, forceDisableHtml, @@ -432,6 +386,7 @@ export function Questions({ originalQuestionIndex === 0 ? `$survey_response` : `$survey_response_${originalQuestionIndex}` if (isLastDisplayedQuestion) { + displayedSurveys.delete(survey.id) return sendSurveyEvent({ ...questionsResponses, [responseKey]: res }, survey, posthog) } else { setQuestionsResponses({ ...questionsResponses, [responseKey]: res }) @@ -544,3 +499,54 @@ export function FeedbackWidget({ ) } + +interface GetQuestionComponentProps { + question: SurveyQuestion + forceDisableHtml: boolean + displayQuestionIndex: number + appearance: SurveyAppearance + onSubmit: (res: string | string[] | number | null) => void +} + +const getQuestionComponent = ({ + question, + forceDisableHtml, + displayQuestionIndex, + appearance, + onSubmit, +}: GetQuestionComponentProps): JSX.Element => { + const questionComponents = { + [SurveyQuestionType.Open]: OpenTextQuestion, + [SurveyQuestionType.Link]: LinkQuestion, + [SurveyQuestionType.Rating]: RatingQuestion, + [SurveyQuestionType.SingleChoice]: MultipleChoiceQuestion, + [SurveyQuestionType.MultipleChoice]: MultipleChoiceQuestion, + } + + const commonProps = { + question, + forceDisableHtml, + appearance, + onSubmit, + } + + const additionalProps: Record = { + [SurveyQuestionType.Open]: {}, + [SurveyQuestionType.Link]: {}, + [SurveyQuestionType.Rating]: { displayQuestionIndex }, + [SurveyQuestionType.SingleChoice]: { displayQuestionIndex }, + [SurveyQuestionType.MultipleChoice]: { displayQuestionIndex }, + } + + const Component = questionComponents[question.type] + const componentProps = { ...commonProps, ...additionalProps[question.type] } + + return +} + +const handleWidget = (posthog: PostHog, survey: Survey) => { + const shadow = createWidgetShadow(survey) + const surveyStyleSheet = style(survey.appearance) + shadow.appendChild(Object.assign(document.createElement('style'), { innerText: surveyStyleSheet })) + Preact.render(, shadow) +} From ff23a5fd74bb361bc04c5d44da7d271150c1c2ba Mon Sep 17 00:00:00 2001 From: dylan Date: Tue, 11 Jun 2024 15:09:35 -0700 Subject: [PATCH 09/23] this works for all the use cases --- src/__tests__/extensions/surveys.test.ts | 63 +++++++- src/extensions/surveys.tsx | 141 ++++++++---------- .../surveys/components/QuestionTypes.tsx | 2 +- 3 files changed, 126 insertions(+), 80 deletions(-) diff --git a/src/__tests__/extensions/surveys.test.ts b/src/__tests__/extensions/surveys.test.ts index 1732848ab..d5ad44d6a 100644 --- a/src/__tests__/extensions/surveys.test.ts +++ b/src/__tests__/extensions/surveys.test.ts @@ -57,11 +57,11 @@ describe('survey display logic', () => { jest.spyOn(global, 'setInterval') generateSurveys(mockPostHog) expect(mockPostHog.getActiveMatchingSurveys).toBeCalledTimes(1) - expect(setInterval).toHaveBeenLastCalledWith(expect.any(Function), 3000) + expect(setInterval).toHaveBeenLastCalledWith(expect.any(Function), 1000) - jest.advanceTimersByTime(3000) + jest.advanceTimersByTime(1000) expect(mockPostHog.getActiveMatchingSurveys).toBeCalledTimes(2) - expect(setInterval).toHaveBeenLastCalledWith(expect.any(Function), 3000) + expect(setInterval).toHaveBeenLastCalledWith(expect.any(Function), 1000) }) }) @@ -154,8 +154,65 @@ describe('usePopupVisibility', () => { expect(removeEventListenerSpy).toHaveBeenCalledWith('PHSurveySent', expect.any(Function)) jest.useRealTimers() }) + + test('should set isPopupVisible to true if isPreviewMode is true', () => { + const { result } = renderHook(() => usePopupVisibility(mockSurvey, mockPostHog, 1000, true)) + expect(result.current.isPopupVisible).toBe(true) + }) + + test('should set isPopupVisible to true after a delay of 500 milliseconds', () => { + jest.useFakeTimers() + const { result } = renderHook(() => usePopupVisibility(mockSurvey, mockPostHog, 500, false)) + expect(result.current.isPopupVisible).toBe(false) + act(() => { + jest.advanceTimersByTime(500) + }) + expect(result.current.isPopupVisible).toBe(true) + jest.useRealTimers() + }) + + test('should not throw an error if posthog is undefined', () => { + const { result } = renderHook(() => usePopupVisibility(mockSurvey, undefined, 0, false)) + expect(result.current.isPopupVisible).toBe(true) + }) + + test('should clean up event listeners on unmount when delay is 0', () => { + const { unmount } = renderHook(() => usePopupVisibility(mockSurvey, mockPostHog, 0, false)) + const removeEventListenerSpy = jest.spyOn(window, 'removeEventListener') + + unmount() + + expect(removeEventListenerSpy).toHaveBeenCalledWith('PHSurveyClosed', expect.any(Function)) + expect(removeEventListenerSpy).toHaveBeenCalledWith('PHSurveySent', expect.any(Function)) + }) + + test('should dispatch PHSurveyShown event when survey is shown', () => { + const dispatchEventSpy = jest.spyOn(window, 'dispatchEvent') + renderHook(() => usePopupVisibility(mockSurvey, mockPostHog, 0, false)) + + expect(dispatchEventSpy).toHaveBeenCalledWith(new Event('PHSurveyShown')) + }) + + test('should handle multiple surveys with overlapping conditions', () => { + jest.useFakeTimers() + const mockSurvey2 = { ...mockSurvey, id: 'testSurvey2', name: 'Test survey 2' } as Survey + const { result: result1 } = renderHook(() => usePopupVisibility(mockSurvey, mockPostHog, 0, false)) + const { result: result2 } = renderHook(() => usePopupVisibility(mockSurvey2, mockPostHog, 500, false)) + + expect(result1.current.isPopupVisible).toBe(true) + expect(result2.current.isPopupVisible).toBe(false) + + act(() => { + jest.advanceTimersByTime(500) + }) + + expect(result2.current.isPopupVisible).toBe(true) + jest.useRealTimers() + }) }) +// describe('') + describe('preview renders', () => { beforeEach(() => { // we have to manually reset the DOM before each test diff --git a/src/extensions/surveys.tsx b/src/extensions/surveys.tsx index 8356fb52f..b4546a2e0 100644 --- a/src/extensions/surveys.tsx +++ b/src/extensions/surveys.tsx @@ -29,23 +29,67 @@ import { const window = _window as Window & typeof globalThis const document = _document as Document -const displayedSurveys = new Set() -let surveyIdBeingDisplayed: string | null = null // TODO this could be a bool instead of a string, we don't need to know the id +// Initialize the set of surveys that are actively displayed +const surveysToActivelyDisplay = new Set() + +export const handleWidgetSelector = (posthog: PostHog, survey: Survey) => { + const selectorOnPage = survey.appearance?.widgetSelector && document.querySelector(survey.appearance.widgetSelector) + if (selectorOnPage) { + if (document.querySelectorAll(`.PostHogWidget${survey.id}`).length === 0) { + handleWidget(posthog, survey) + } else if (document.querySelectorAll(`.PostHogWidget${survey.id}`).length === 1) { + if (!selectorOnPage.getAttribute('PHWidgetSurveyClickListener')) { + const surveyPopup = document + .querySelector(`.PostHogWidget${survey.id}`) + ?.shadowRoot?.querySelector(`.survey-form`) as HTMLFormElement + selectorOnPage.addEventListener('click', () => { + if (surveyPopup) { + surveyPopup.style.display = surveyPopup.style.display === 'none' ? 'block' : 'none' + surveyPopup.addEventListener('PHSurveyClosed', () => (surveyPopup.style.display = 'none')) + } + }) + selectorOnPage.setAttribute('PHWidgetSurveyClickListener', 'true') + } + } + } +} + +const canShowNextSurvey = (): boolean => { + const surveyPopups = document.querySelectorAll(`div[class^=PostHogSurvey]`) + if (surveyPopups.length > 0) { + return surveyPopups[surveyPopups.length - 1].shadowRoot?.childElementCount === 1 + } + return true +} + +const handlePopoverSurvey = (posthog: PostHog, survey: Survey) => { + const surveyWaitPeriodInDays = survey.conditions?.seenSurveyWaitPeriodInDays + const lastSeenSurveyDate = localStorage.getItem(`lastSeenSurveyDate`) + if (surveyWaitPeriodInDays && lastSeenSurveyDate) { + const today = new Date() + const diff = Math.abs(today.getTime() - new Date(lastSeenSurveyDate).getTime()) + const diffDaysFromToday = Math.ceil(diff / (1000 * 3600 * 24)) + if (diffDaysFromToday < surveyWaitPeriodInDays) { + return + } + } + + if (!localStorage.getItem(getSurveySeenKey(survey))) { + surveysToActivelyDisplay.add(survey.id) + const shadow = createShadow(style(survey?.appearance), survey.id) + Preact.render(, shadow) + } +} export const callSurveys = (posthog: PostHog, forceReload: boolean = false) => { posthog?.getActiveMatchingSurveys((surveys) => { const nonAPISurveys = surveys.filter((survey) => survey.type !== 'api') - // Sort the surveys by surveyPopupDelay (ascending order) - nonAPISurveys.sort((a, b) => { - const delayA = a.appearance?.surveyPopupDelay || 0 - const delayB = b.appearance?.surveyPopupDelay || 0 - return delayA - delayB - }) + nonAPISurveys.sort((a, b) => (a.appearance?.surveyPopupDelay || 0) - (b.appearance?.surveyPopupDelay || 0)) nonAPISurveys.forEach((survey) => { - if (displayedSurveys.has(survey.id) || surveyIdBeingDisplayed) { - return // skip if survey is already visible + if (surveysToActivelyDisplay.size > 0) { + return } if (survey.type === SurveyType.Widget) { if ( @@ -55,62 +99,12 @@ export const callSurveys = (posthog: PostHog, forceReload: boolean = false) => { handleWidget(posthog, survey) } if (survey.appearance?.widgetType === 'selector' && survey.appearance?.widgetSelector) { - const selectorOnPage = document.querySelector(survey.appearance.widgetSelector) - if (selectorOnPage) { - if (document.querySelectorAll(`.PostHogWidget${survey.id}`).length === 0) { - handleWidget(posthog, survey) - } else if (document.querySelectorAll(`.PostHogWidget${survey.id}`).length === 1) { - // we have to check if user selector already has a survey listener attached to it because we always have to check if it's on the page or not - if (!selectorOnPage.getAttribute('PHWidgetSurveyClickListener')) { - const surveyPopup = document - .querySelector(`.PostHogWidget${survey.id}`) - ?.shadowRoot?.querySelector(`.survey-form`) as HTMLFormElement - selectorOnPage.addEventListener('click', () => { - if (surveyPopup) { - surveyPopup.style.display = - surveyPopup.style.display === 'none' ? 'block' : 'none' - surveyPopup.addEventListener( - 'PHSurveyClosed', - () => (surveyPopup.style.display = 'none') - ) - } - }) - selectorOnPage.setAttribute('PHWidgetSurveyClickListener', 'true') - } - } - } + handleWidgetSelector(posthog, survey) } } - // with event based surveys, we need to show the next survey without reloading the page. - // A simple check for div elements with the class name pattern of PostHogSurvey_xyz doesn't work here - // because preact leaves behind the div element for any surveys responded/dismissed with a + + ` + const lastSurveyPopup = document.querySelector('.PostHogSurvey_1') as HTMLElement + const shadowRoot = lastSurveyPopup.attachShadow({ mode: 'open' }) + const styleNode = document.createElement('style') + shadowRoot.appendChild(styleNode) + + expect(canShowNextSurvey()).toBe(true) + }) + + test('returns false if the last survey popup has more than one child element', () => { + document.body.innerHTML = ` +
+ +
+ ` + const lastSurveyPopup = document.querySelector('.PostHogSurvey_1') as HTMLElement + const shadowRoot = lastSurveyPopup.attachShadow({ mode: 'open' }) + const styleNode = document.createElement('style') + const divNode = document.createElement('div') + shadowRoot.appendChild(styleNode) + shadowRoot.appendChild(divNode) + + expect(canShowNextSurvey()).toBe(false) + }) +}) + +describe('handlePopoverSurvey', () => { + const mockSurvey: Survey = { + id: 'testSurvey1', + name: 'Test survey 1', + type: SurveyType.Popover, + appearance: {}, + start_date: '2021-01-01T00:00:00.000Z', + questions: [ + { + question: 'How satisfied are you with our newest product?', + description: 'This is a question description', + type: SurveyQuestionType.Rating, + display: 'number', + scale: 10, + lowerBoundLabel: 'Not Satisfied', + upperBoundLabel: 'Very Satisfied', + }, + ], + conditions: {}, + end_date: null, + targeting_flag_key: null, + } as Survey + + const mockPostHog: PostHog = { + getActiveMatchingSurveys: jest.fn().mockImplementation((callback) => callback([mockSurvey])), + get_session_replay_url: jest.fn(), + capture: jest.fn().mockImplementation((eventName) => eventName), + } as unknown as PostHog + + beforeEach(() => { + localStorage.clear() + jest.clearAllMocks() + }) + + test('respects survey wait period', () => { + const survey = { ...mockSurvey, conditions: { seenSurveyWaitPeriodInDays: 7 } } as Survey + localStorage.setItem('lastSeenSurveyDate', new Date().toISOString()) + const spy = jest.spyOn(Preact, 'render') + handlePopoverSurvey(mockPostHog, survey) + expect(spy).not.toHaveBeenCalled() + }) + + test('skips survey if it has already been seen', () => { + localStorage.setItem(getSurveySeenKey(mockSurvey), 'true') + const spy = jest.spyOn(Preact, 'render') + handlePopoverSurvey(mockPostHog, mockSurvey) + expect(spy).not.toHaveBeenCalled() + }) +}) describe('preview renders', () => { beforeEach(() => { diff --git a/src/extensions/surveys.tsx b/src/extensions/surveys.tsx index b85da7dbc..a7bfcdc42 100644 --- a/src/extensions/surveys.tsx +++ b/src/extensions/surveys.tsx @@ -29,7 +29,16 @@ import { const window = _window as Window & typeof globalThis const document = _document as Document -// Initialize the set of surveys that are actively displayed +// TODO: @dmarticus - Keeping track of the surveys that are actively displayed is a bit of a hack +// to support survey popup delay feature that enables us to embed multiple surveys on the same page +// without showing them all at once. This is a temporary solution until we have a better way to +// manage the lifecycle of surveys without using a global state. I'm currently working on a better +// a refactor of this feature (see TODO GITHUB LINK), but in the interest of shipping fast and iterating, +// I decided to go with this approach to unlock a feature that users have been asking for ASAP. I'll call +// out all the places in this file where we use this global state and why we use it, which will hopefully make +// it easier to refactor this feature in the future. + +// We start by creating a set to keep track of the surveys that are actively displayed (as compared to the ones that are just on the page) const surveysToActivelyDisplay = new Set() export const handleWidgetSelector = (posthog: PostHog, survey: Survey) => { @@ -55,7 +64,7 @@ export const handleWidgetSelector = (posthog: PostHog, survey: Survey) => { } } -const canShowNextSurvey = (): boolean => { +export const canShowNextSurvey = (): boolean => { // with event based surveys, we need to show the next survey without reloading the page. // A simple check for div elements with the class name pattern of PostHogSurvey_xyz doesn't work here // because preact leaves behind the div element for any surveys responded/dismissed with a - - ` - const lastSurveyPopup = document.querySelector('.PostHogSurvey_1') as HTMLElement - const shadowRoot = lastSurveyPopup.attachShadow({ mode: 'open' }) - const styleNode = document.createElement('style') - shadowRoot.appendChild(styleNode) - - expect(canShowNextSurvey()).toBe(true) - }) - - test('returns false if the last survey popup has more than one child element', () => { - document.body.innerHTML = ` -
- -
- ` - const lastSurveyPopup = document.querySelector('.PostHogSurvey_1') as HTMLElement - const shadowRoot = lastSurveyPopup.attachShadow({ mode: 'open' }) - const styleNode = document.createElement('style') - const divNode = document.createElement('div') - shadowRoot.appendChild(styleNode) - shadowRoot.appendChild(divNode) - - expect(canShowNextSurvey()).toBe(false) - }) -}) - -describe('handlePopoverSurvey', () => { - const mockSurvey: Survey = { - id: 'testSurvey1', - name: 'Test survey 1', - type: SurveyType.Popover, - appearance: {}, - start_date: '2021-01-01T00:00:00.000Z', - questions: [ - { - question: 'How satisfied are you with our newest product?', - description: 'This is a question description', - type: SurveyQuestionType.Rating, - display: 'number', - scale: 10, - lowerBoundLabel: 'Not Satisfied', - upperBoundLabel: 'Very Satisfied', - }, - ], - conditions: {}, - end_date: null, - targeting_flag_key: null, - } as Survey - - const mockPostHog: PostHog = { - getActiveMatchingSurveys: jest.fn().mockImplementation((callback) => callback([mockSurvey])), - get_session_replay_url: jest.fn(), - capture: jest.fn().mockImplementation((eventName) => eventName), - } as unknown as PostHog - - beforeEach(() => { - localStorage.clear() - jest.clearAllMocks() - }) - - test('respects survey wait period', () => { - const survey = { ...mockSurvey, conditions: { seenSurveyWaitPeriodInDays: 7 } } as Survey - localStorage.setItem('lastSeenSurveyDate', new Date().toISOString()) - const spy = jest.spyOn(Preact, 'render') - handlePopoverSurvey(mockPostHog, survey) - expect(spy).not.toHaveBeenCalled() - }) - - test('skips survey if it has already been seen', () => { - localStorage.setItem(getSurveySeenKey(mockSurvey), 'true') - const spy = jest.spyOn(Preact, 'render') - handlePopoverSurvey(mockPostHog, mockSurvey) - expect(spy).not.toHaveBeenCalled() - }) -}) +// describe('canShowNextSurvey', () => { +// beforeEach(() => { +// // Reset the DOM before each test +// document.body.innerHTML = '' +// }) + +// test('returns true if no survey popups are present', () => { +// expect(canShowNextSurvey()).toBe(true) +// }) + +// test('returns true if the last survey popup has only one child element (a style node)', () => { +// document.body.innerHTML = ` +//
+// +//
+// ` +// const lastSurveyPopup = document.querySelector('.PostHogSurvey_1') as HTMLElement +// const shadowRoot = lastSurveyPopup.attachShadow({ mode: 'open' }) +// const styleNode = document.createElement('style') +// shadowRoot.appendChild(styleNode) + +// expect(canShowNextSurvey()).toBe(true) +// }) + +// test('returns false if the last survey popup has more than one child element', () => { +// document.body.innerHTML = ` +//
+// +//
+// ` +// const lastSurveyPopup = document.querySelector('.PostHogSurvey_1') as HTMLElement +// const shadowRoot = lastSurveyPopup.attachShadow({ mode: 'open' }) +// const styleNode = document.createElement('style') +// const divNode = document.createElement('div') +// shadowRoot.appendChild(styleNode) +// shadowRoot.appendChild(divNode) + +// expect(canShowNextSurvey()).toBe(false) +// }) +// }) + +// describe('handlePopoverSurvey', () => { +// const mockSurvey: Survey = { +// id: 'testSurvey1', +// name: 'Test survey 1', +// type: SurveyType.Popover, +// appearance: {}, +// start_date: '2021-01-01T00:00:00.000Z', +// questions: [ +// { +// question: 'How satisfied are you with our newest product?', +// description: 'This is a question description', +// type: SurveyQuestionType.Rating, +// display: 'number', +// scale: 10, +// lowerBoundLabel: 'Not Satisfied', +// upperBoundLabel: 'Very Satisfied', +// }, +// ], +// conditions: {}, +// end_date: null, +// targeting_flag_key: null, +// } as Survey + +// const mockPostHog: PostHog = { +// getActiveMatchingSurveys: jest.fn().mockImplementation((callback) => callback([mockSurvey])), +// get_session_replay_url: jest.fn(), +// capture: jest.fn().mockImplementation((eventName) => eventName), +// } as unknown as PostHog + +// beforeEach(() => { +// localStorage.clear() +// jest.clearAllMocks() +// }) + +// test('respects survey wait period', () => { +// const survey = { ...mockSurvey, conditions: { seenSurveyWaitPeriodInDays: 7 } } as Survey +// localStorage.setItem('lastSeenSurveyDate', new Date().toISOString()) +// const spy = jest.spyOn(Preact, 'render') +// handlePopoverSurvey(mockPostHog, survey) +// expect(spy).not.toHaveBeenCalled() +// }) + +// test('skips survey if it has already been seen', () => { +// localStorage.setItem(getSurveySeenKey(mockSurvey), 'true') +// const spy = jest.spyOn(Preact, 'render') +// handlePopoverSurvey(mockPostHog, mockSurvey) +// expect(spy).not.toHaveBeenCalled() +// }) +// }) describe('preview renders', () => { beforeEach(() => { diff --git a/src/extensions/surveys.tsx b/src/extensions/surveys.tsx index 0261730dd..95a438c4c 100644 --- a/src/extensions/surveys.tsx +++ b/src/extensions/surveys.tsx @@ -29,107 +29,133 @@ import { const window = _window as Window & typeof globalThis const document = _document as Document -// TODO: @dmarticus - Keeping track of the surveys that are actively displayed is a bit of a hack -// to support survey popup delay feature that enables us to embed multiple surveys on the same page -// without showing them all at once. This is a temporary solution until we have a better way to -// manage the lifecycle of surveys without using a global state. I'm currently working on a better -// a refactor of this feature (see: https://github.com/PostHog/posthog-js/pull/1242), but in the interest of shipping fast and iterating, -// I decided to go with this approach to unlock a feature that users have been asking for ASAP. I'll call -// out all the places in this file where we use this global state and why we use it, which will hopefully make -// it easier to refactor this feature in the future. - -// We start by creating a set to keep track of the surveys that are actively displayed (as compared to the ones that are just on the page) -const surveysToActivelyDisplay = new Set() - -export const handleWidgetSelector = (posthog: PostHog, survey: Survey) => { - const selectorOnPage = survey.appearance?.widgetSelector && document.querySelector(survey.appearance.widgetSelector) - if (selectorOnPage) { - if (document.querySelectorAll(`.PostHogWidget${survey.id}`).length === 0) { - handleWidget(posthog, survey) - } else if (document.querySelectorAll(`.PostHogWidget${survey.id}`).length === 1) { - // we have to check if user selector already has a survey listener attached to it because we always have to check if it's on the page or not - if (!selectorOnPage.getAttribute('PHWidgetSurveyClickListener')) { - const surveyPopup = document - .querySelector(`.PostHogWidget${survey.id}`) - ?.shadowRoot?.querySelector(`.survey-form`) as HTMLFormElement - selectorOnPage.addEventListener('click', () => { - if (surveyPopup) { - surveyPopup.style.display = surveyPopup.style.display === 'none' ? 'block' : 'none' - surveyPopup.addEventListener('PHSurveyClosed', () => (surveyPopup.style.display = 'none')) - } - }) - selectorOnPage.setAttribute('PHWidgetSurveyClickListener', 'true') - } +const createSurveyManager = (posthog: PostHog) => { + // TODO: @dmarticus - Keeping track of the surveys that are actively displayed is a bit of a hack + // to support survey popup delay feature that enables us to embed multiple surveys on the same page + // without showing them all at once. This is a temporary solution until we have a better way to + // manage the lifecycle of surveys without using a global state. I'm currently working on a better + // a refactor of this feature (see: https://github.com/PostHog/posthog-js/pull/1242), but in the interest of shipping fast and iterating, + // I decided to go with this approach to unlock a feature that users have been asking for ASAP. I'll call + // out all the places in this file where we use this global state and why we use it, which will hopefully make + // it easier to refactor this feature in the future. + const surveysInFocus = new Set() + + const canShowNextSurvey = (): boolean => { + // with event based surveys, we need to show the next survey without reloading the page. + // A simple check for div elements with the class name pattern of PostHogSurvey_xyz doesn't work here + // because preact leaves behind the div element for any surveys responded/dismissed with a -// -// ` -// const lastSurveyPopup = document.querySelector('.PostHogSurvey_1') as HTMLElement -// const shadowRoot = lastSurveyPopup.attachShadow({ mode: 'open' }) -// const styleNode = document.createElement('style') -// shadowRoot.appendChild(styleNode) - -// expect(canShowNextSurvey()).toBe(true) -// }) - -// test('returns false if the last survey popup has more than one child element', () => { -// document.body.innerHTML = ` -//
-// -//
-// ` -// const lastSurveyPopup = document.querySelector('.PostHogSurvey_1') as HTMLElement -// const shadowRoot = lastSurveyPopup.attachShadow({ mode: 'open' }) -// const styleNode = document.createElement('style') -// const divNode = document.createElement('div') -// shadowRoot.appendChild(styleNode) -// shadowRoot.appendChild(divNode) - -// expect(canShowNextSurvey()).toBe(false) -// }) -// }) - -// describe('handlePopoverSurvey', () => { -// const mockSurvey: Survey = { -// id: 'testSurvey1', -// name: 'Test survey 1', -// type: SurveyType.Popover, -// appearance: {}, -// start_date: '2021-01-01T00:00:00.000Z', -// questions: [ -// { -// question: 'How satisfied are you with our newest product?', -// description: 'This is a question description', -// type: SurveyQuestionType.Rating, -// display: 'number', -// scale: 10, -// lowerBoundLabel: 'Not Satisfied', -// upperBoundLabel: 'Very Satisfied', -// }, -// ], -// conditions: {}, -// end_date: null, -// targeting_flag_key: null, -// } as Survey - -// const mockPostHog: PostHog = { -// getActiveMatchingSurveys: jest.fn().mockImplementation((callback) => callback([mockSurvey])), -// get_session_replay_url: jest.fn(), -// capture: jest.fn().mockImplementation((eventName) => eventName), -// } as unknown as PostHog - -// beforeEach(() => { -// localStorage.clear() -// jest.clearAllMocks() -// }) - -// test('respects survey wait period', () => { -// const survey = { ...mockSurvey, conditions: { seenSurveyWaitPeriodInDays: 7 } } as Survey -// localStorage.setItem('lastSeenSurveyDate', new Date().toISOString()) -// const spy = jest.spyOn(Preact, 'render') -// handlePopoverSurvey(mockPostHog, survey) -// expect(spy).not.toHaveBeenCalled() -// }) - -// test('skips survey if it has already been seen', () => { -// localStorage.setItem(getSurveySeenKey(mockSurvey), 'true') -// const spy = jest.spyOn(Preact, 'render') -// handlePopoverSurvey(mockPostHog, mockSurvey) -// expect(spy).not.toHaveBeenCalled() -// }) -// }) +describe('createSurveyManager', () => { + let mockPostHog: PostHog + let surveyManager: ReturnType + + beforeEach(() => { + mockPostHog = { + getActiveMatchingSurveys: jest.fn(), + get_session_replay_url: jest.fn(), + capture: jest.fn(), + } as unknown as PostHog + + surveyManager = createSurveyManager(mockPostHog) + }) + + test('should initialize surveysInFocus correctly', () => { + expect(surveyManager).toBeDefined() + expect(typeof surveyManager.addSurveyToFocus).toBe('function') + expect(typeof surveyManager.removeSurveyFromFocus).toBe('function') + expect(typeof surveyManager.callSurveys).toBe('function') + }) + + test('addSurveyToFocus should add survey ID to surveysInFocus', () => { + surveyManager.addSurveyToFocus('survey1') + expect(surveyManager['surveysInFocus'].has('survey1')).toBe(true) + }) + + test('removeSurveyFromFocus should remove survey ID from surveysInFocus', () => { + surveyManager.addSurveyToFocus('survey1') + surveyManager.removeSurveyFromFocus('survey1') + expect(surveyManager['surveysInFocus'].has('survey1')).toBe(false) + }) + + test('canShowNextSurvey should return correct visibility status', () => { + const surveyDiv = document.createElement('div') + surveyDiv.className = 'PostHogSurvey_test' + surveyDiv.attachShadow({ mode: 'open' }) + surveyDiv.shadowRoot!.appendChild(document.createElement('style')) + document.body.appendChild(surveyDiv) + + expect(surveyManager['canShowNextSurvey']()).toBe(true) + + surveyDiv.shadowRoot!.appendChild(document.createElement('div')) + expect(surveyManager['canShowNextSurvey']()).toBe(false) + }) + + test('callSurveys should handle surveys correctly', () => { + const mockSurveys: any[] = [ + // TODO let's clean up these anys + { + id: 'survey1', + name: 'Survey 1', + type: SurveyType.Popover, + appearance: { surveyPopupDelaySeconds: 0 }, + start_date: '2021-01-01T00:00:00.000Z', + questions: [ + { + question: 'How satisfied are you with our newest product?', + description: 'This is a question description', + type: 'rating' as SurveyQuestionType, + display: 'number', + scale: 10, + lowerBoundLabel: 'Not Satisfied', + upperBoundLabel: 'Very Satisfied', + }, + ], + }, + ] + + mockPostHog.getActiveMatchingSurveys = jest.fn((callback) => callback(mockSurveys)) + + surveyManager.callSurveys() + + expect(mockPostHog.getActiveMatchingSurveys).toHaveBeenCalled() + expect(document.querySelector('.PostHogSurvey_survey1')).not.toBeNull() + }) + + // Add more tests to cover handleWidget, handleWidgetSelector, and other edge cases... +}) describe('preview renders', () => { beforeEach(() => { diff --git a/src/extensions/surveys.tsx b/src/extensions/surveys.tsx index 3f2d0ffed..054c4a3a9 100644 --- a/src/extensions/surveys.tsx +++ b/src/extensions/surveys.tsx @@ -29,9 +29,9 @@ import { const window = _window as Window & typeof globalThis const document = _document as Document -const createSurveyManager = (posthog: PostHog) => { +export const createSurveyManager = (posthog: PostHog) => { // TODO: @dmarticus - Keeping track of the surveys that are actively displayed is a bit of a hack - // to support survey popup delay feature that enables us to embed multiple surveys on the same page + // to support survey popup millisecondDelay feature that enables us to embed multiple surveys on the same page // without showing them all at once. This is a temporary solution until we have a better way to // manage the lifecycle of surveys without using a global state. I'm currently working on a better // a refactor of this feature (see: https://github.com/PostHog/posthog-js/pull/1242), but in the interest of shipping fast and iterating, @@ -123,9 +123,11 @@ const createSurveyManager = (posthog: PostHog) => { const callSurveys = (forceReload: boolean = false) => { posthog?.getActiveMatchingSurveys((surveys) => { const nonAPISurveys = surveys.filter((survey) => survey.type !== 'api') - nonAPISurveys.sort((a, b) => (a.appearance?.surveyPopupDelay || 0) - (b.appearance?.surveyPopupDelay || 0)) + const nonAPISurveyQueue = nonAPISurveys.sort( + (a, b) => (a.appearance?.surveyPopupDelaySeconds || 0) - (b.appearance?.surveyPopupDelaySeconds || 0) + ) - nonAPISurveys.forEach((survey) => { + nonAPISurveyQueue.forEach((survey) => { if (surveysInFocus.size > 0) { return } @@ -246,11 +248,11 @@ export function generateSurveys(posthog: PostHog) { export function usePopupVisibility( survey: Survey, posthog: PostHog | undefined, - delay: number, + millisecondDelay: number, isPreviewMode: boolean, removeSurveyFromFocus: (id: string) => void ) { - const [isPopupVisible, setIsPopupVisible] = useState(isPreviewMode || delay === 0) + const [isPopupVisible, setIsPopupVisible] = useState(isPreviewMode || millisecondDelay === 0) const [isSurveySent, setIsSurveySent] = useState(false) useEffect(() => { @@ -281,7 +283,7 @@ export function usePopupVisibility( window.addEventListener('PHSurveyClosed', handleSurveyClosed) window.addEventListener('PHSurveySent', handleSurveySent) - if (delay > 0) { + if (millisecondDelay > 0) { const timeoutId = setTimeout(() => { setIsPopupVisible(true) window.dispatchEvent(new Event('PHSurveyShown')) @@ -293,7 +295,7 @@ export function usePopupVisibility( sessionRecordingUrl: posthog.get_session_replay_url?.(), }) localStorage.setItem(`lastSeenSurveyDate`, new Date().toISOString()) - }, delay) + }, millisecondDelay) return () => { clearTimeout(timeoutId) @@ -338,11 +340,14 @@ export function SurveyPopup({ removeSurveyFromFocus: (id: string) => void }) { const isPreviewMode = Number.isInteger(previewPageIndex) - const delay = survey.appearance?.surveyPopupDelay ? survey.appearance.surveyPopupDelay * 1000 : 0 + // NB: The client-side code passes the millisecondDelay in seconds, but setTimeout expects milliseconds, so we multiply by 1000 + const surveyPopupDelayMilliseconds = survey.appearance?.surveyPopupDelaySeconds + ? survey.appearance.surveyPopupDelaySeconds * 1000 + : 0 const { isPopupVisible, isSurveySent, setIsPopupVisible } = usePopupVisibility( survey, posthog, - delay, + surveyPopupDelayMilliseconds, isPreviewMode, removeSurveyFromFocus ) diff --git a/src/posthog-surveys-types.ts b/src/posthog-surveys-types.ts index 728113e39..3adbf4bba 100644 --- a/src/posthog-surveys-types.ts +++ b/src/posthog-surveys-types.ts @@ -26,7 +26,7 @@ export interface SurveyAppearance { position?: 'left' | 'right' | 'center' placeholder?: string shuffleQuestions?: boolean - surveyPopupDelay?: number + surveyPopupDelaySeconds?: number // widget options widgetType?: 'button' | 'tab' | 'selector' widgetSelector?: string From 18d099cc036b37572e34f6b2736a5bf6b0757066 Mon Sep 17 00:00:00 2001 From: dylan Date: Tue, 18 Jun 2024 15:01:50 -0700 Subject: [PATCH 16/23] added a bunch of tests for new behavior --- src/__tests__/extensions/surveys.test.ts | 150 ++++++++++++++++------- src/extensions/surveys.tsx | 85 +++++++------ 2 files changed, 154 insertions(+), 81 deletions(-) diff --git a/src/__tests__/extensions/surveys.test.ts b/src/__tests__/extensions/surveys.test.ts index a45f1cfcd..40d65b32d 100644 --- a/src/__tests__/extensions/surveys.test.ts +++ b/src/__tests__/extensions/surveys.test.ts @@ -3,7 +3,7 @@ import { renderSurveysPreview, renderFeedbackWidgetPreview, usePopupVisibility, - createSurveyManager, + SurveyManager, } from '../../extensions/surveys' import { createShadow } from '../../extensions/surveys/surveys-utils' import { Survey, SurveyQuestionType, SurveyType } from '../../posthog-surveys-types' @@ -40,8 +40,8 @@ describe('survey display logic', () => { type: 'rating', display: 'number', scale: 10, - lower_bound_label: 'Not Satisfied', - upper_bound_label: 'Very Satisfied', + lowerBoundLabel: 'Not Satisfied', + upperBoundLabel: 'Very Satisfied', }, ], }, @@ -51,9 +51,9 @@ describe('survey display logic', () => { getActiveMatchingSurveys: jest.fn().mockImplementation((callback) => callback(mockSurveys)), get_session_replay_url: jest.fn(), capture: jest.fn().mockImplementation((eventName) => eventName), - } + } as any - test('callSurveys runs on interval irrespective of url change', () => { + test('callSurveysAndEvaluateDisplayLogic runs on interval irrespective of url change', () => { jest.useFakeTimers() jest.spyOn(global, 'setInterval') generateSurveys(mockPostHog) @@ -218,9 +218,10 @@ describe('usePopupVisibility', () => { }) }) -describe('createSurveyManager', () => { +describe('SurveyManager', () => { let mockPostHog: PostHog - let surveyManager: ReturnType + let surveyManager: SurveyManager + let mockSurveys: Survey[] beforeEach(() => { mockPostHog = { @@ -229,25 +230,55 @@ describe('createSurveyManager', () => { capture: jest.fn(), } as unknown as PostHog - surveyManager = createSurveyManager(mockPostHog) + surveyManager = new SurveyManager(mockPostHog) + + mockSurveys = [ + { + id: 'survey1', + name: 'Survey 1', + type: SurveyType.Popover, + appearance: { surveyPopupDelay: 0 }, + questions: [], + conditions: {}, + start_date: '2021-01-01T00:00:00.000Z', + end_date: null, + targeting_flag_key: null, + }, + ] as unknown as Survey[] + }) + + test('callSurveysAndEvaluateDisplayLogic should handle a single popover survey correctly', () => { + mockPostHog.getActiveMatchingSurveys = jest.fn((callback) => callback([mockSurveys[0]])) + const handlePopoverSurveyMock = jest + .spyOn(surveyManager as any, 'handlePopoverSurvey') + .mockImplementation(() => { + console.log('Mock handlePopoverSurvey called') + }) + const canShowNextSurveyMock = jest.spyOn(surveyManager as any, 'canShowNextSurvey').mockReturnValue(true) + + surveyManager.callSurveysAndEvaluateDisplayLogic() + + expect(mockPostHog.getActiveMatchingSurveys).toHaveBeenCalled() + expect(handlePopoverSurveyMock).toHaveBeenCalledWith(mockSurveys[0]) + expect(canShowNextSurveyMock).toHaveBeenCalled() }) test('should initialize surveysInFocus correctly', () => { expect(surveyManager).toBeDefined() - expect(typeof surveyManager.addSurveyToFocus).toBe('function') - expect(typeof surveyManager.removeSurveyFromFocus).toBe('function') - expect(typeof surveyManager.callSurveys).toBe('function') + expect(typeof surveyManager.getTestAPI().addSurveyToFocus).toBe('function') + expect(typeof surveyManager.getTestAPI().removeSurveyFromFocus).toBe('function') + expect(typeof surveyManager.callSurveysAndEvaluateDisplayLogic).toBe('function') }) test('addSurveyToFocus should add survey ID to surveysInFocus', () => { - surveyManager.addSurveyToFocus('survey1') - expect(surveyManager['surveysInFocus'].has('survey1')).toBe(true) + surveyManager.getTestAPI().addSurveyToFocus('survey1') + expect(surveyManager.getTestAPI().surveysInFocus.has('survey1')).toBe(true) }) test('removeSurveyFromFocus should remove survey ID from surveysInFocus', () => { - surveyManager.addSurveyToFocus('survey1') - surveyManager.removeSurveyFromFocus('survey1') - expect(surveyManager['surveysInFocus'].has('survey1')).toBe(false) + surveyManager.getTestAPI().addSurveyToFocus('survey1') + surveyManager.getTestAPI().removeSurveyFromFocus('survey1') + expect(surveyManager.getTestAPI().surveysInFocus.has('survey1')).toBe(false) }) test('canShowNextSurvey should return correct visibility status', () => { @@ -257,44 +288,75 @@ describe('createSurveyManager', () => { surveyDiv.shadowRoot!.appendChild(document.createElement('style')) document.body.appendChild(surveyDiv) - expect(surveyManager['canShowNextSurvey']()).toBe(true) + expect(surveyManager.getTestAPI().canShowNextSurvey()).toBe(true) surveyDiv.shadowRoot!.appendChild(document.createElement('div')) - expect(surveyManager['canShowNextSurvey']()).toBe(false) + expect(surveyManager.getTestAPI().canShowNextSurvey()).toBe(false) }) - test('callSurveys should handle surveys correctly', () => { - const mockSurveys: any[] = [ - // TODO let's clean up these anys - { - id: 'survey1', - name: 'Survey 1', - type: SurveyType.Popover, - appearance: { surveyPopupDelaySeconds: 0 }, - start_date: '2021-01-01T00:00:00.000Z', - questions: [ - { - question: 'How satisfied are you with our newest product?', - description: 'This is a question description', - type: 'rating' as SurveyQuestionType, - display: 'number', - scale: 10, - lowerBoundLabel: 'Not Satisfied', - upperBoundLabel: 'Very Satisfied', - }, - ], - }, - ] + test('callSurveysAndEvaluateDisplayLogic should handle surveys correctly', () => { + mockPostHog.getActiveMatchingSurveys = jest.fn((callback) => callback([mockSurveys[0]])) - mockPostHog.getActiveMatchingSurveys = jest.fn((callback) => callback(mockSurveys)) + const handlePopoverSurveyMock = jest + .spyOn(surveyManager as any, 'handlePopoverSurvey') + .mockImplementation(() => { + console.log('Mock handlePopoverSurvey called') + }) + const handleWidgetMock = jest.spyOn(surveyManager as any, 'handleWidget').mockImplementation(() => {}) + const handleWidgetSelectorMock = jest + .spyOn(surveyManager as any, 'handleWidgetSelector') + .mockImplementation(() => {}) + jest.spyOn(surveyManager as any, 'canShowNextSurvey').mockReturnValue(true) - surveyManager.callSurveys() + surveyManager.callSurveysAndEvaluateDisplayLogic() expect(mockPostHog.getActiveMatchingSurveys).toHaveBeenCalled() - expect(document.querySelector('.PostHogSurvey_survey1')).not.toBeNull() + expect(handlePopoverSurveyMock).toHaveBeenCalledWith(mockSurveys[0]) + expect(handleWidgetMock).not.toHaveBeenCalled() + expect(handleWidgetSelectorMock).not.toHaveBeenCalled() }) - // Add more tests to cover handleWidget, handleWidgetSelector, and other edge cases... + test('handleWidget should render the widget correctly', () => { + const mockSurvey = mockSurveys[1] + const handleWidgetMock = jest.spyOn(surveyManager as any, 'handleWidget').mockImplementation(() => {}) + surveyManager.getTestAPI().handleWidget(mockSurvey) + expect(handleWidgetMock).toHaveBeenCalledWith(mockSurvey) + }) + + test('handleWidgetSelector should set up the widget selector correctly', () => { + const mockSurvey = { + id: 'widgetSurvey2', + name: 'Widget Survey 2', + type: SurveyType.Widget, + appearance: { widgetType: 'selector', widgetSelector: '.widget-selector' }, + start_date: '2021-01-01T00:00:00.000Z', + questions: [ + { + question: 'What would you like to see next?', + type: SurveyQuestionType.Open, + }, + ], + conditions: {}, + end_date: null, + targeting_flag_key: null, + } as any + document.body.innerHTML = '
' + const handleWidgetSelectorMock = jest + .spyOn(surveyManager as any, 'handleWidgetSelector') + .mockImplementation(() => {}) + surveyManager.getTestAPI().handleWidgetSelector(mockSurvey) + expect(handleWidgetSelectorMock).toHaveBeenCalledWith(mockSurvey) + }) + + test('callSurveysAndEvaluateDisplayLogic should not call surveys in focus', () => { + mockPostHog.getActiveMatchingSurveys = jest.fn((callback) => callback(mockSurveys)) + + surveyManager.getTestAPI().addSurveyToFocus('survey1') + surveyManager.callSurveysAndEvaluateDisplayLogic() + + expect(mockPostHog.getActiveMatchingSurveys).toHaveBeenCalledTimes(1) + expect(surveyManager.getTestAPI().surveysInFocus.size).toBe(1) + }) }) describe('preview renders', () => { diff --git a/src/extensions/surveys.tsx b/src/extensions/surveys.tsx index 054c4a3a9..37e8adcf5 100644 --- a/src/extensions/surveys.tsx +++ b/src/extensions/surveys.tsx @@ -29,18 +29,17 @@ import { const window = _window as Window & typeof globalThis const document = _document as Document -export const createSurveyManager = (posthog: PostHog) => { - // TODO: @dmarticus - Keeping track of the surveys that are actively displayed is a bit of a hack - // to support survey popup millisecondDelay feature that enables us to embed multiple surveys on the same page - // without showing them all at once. This is a temporary solution until we have a better way to - // manage the lifecycle of surveys without using a global state. I'm currently working on a better - // a refactor of this feature (see: https://github.com/PostHog/posthog-js/pull/1242), but in the interest of shipping fast and iterating, - // I decided to go with this approach to unlock a feature that users have been asking for ASAP. I'll call - // out all the places in this file where we use this global state and why we use it, which will hopefully make - // it easier to refactor this feature in the future. - const surveysInFocus = new Set() - - const canShowNextSurvey = (): boolean => { +export class SurveyManager { + private posthog: PostHog + private surveysInFocus: Set + + constructor(posthog: PostHog) { + this.posthog = posthog + // TODO: @dmarticus: write a comment about this that's up-to-date. + this.surveysInFocus = new Set() + } + + private canShowNextSurvey = (): boolean => { // with event based surveys, we need to show the next survey without reloading the page. // A simple check for div elements with the class name pattern of PostHogSurvey_xyz doesn't work here // because preact leaves behind the div element for any surveys responded/dismissed with a