From b68b330f140ad14ffb24f17f00b2890b05e07f03 Mon Sep 17 00:00:00 2001 From: Claas Augner Date: Fri, 7 Jun 2024 18:35:11 +0200 Subject: [PATCH 1/8] feat(telemetry): measure survey interactions --- client/src/telemetry/constants.ts | 1 + client/src/ui/molecules/document-survey/index.tsx | 8 +++++++- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/client/src/telemetry/constants.ts b/client/src/telemetry/constants.ts index ff9ff0c5e634..4351ff6244d1 100644 --- a/client/src/telemetry/constants.ts +++ b/client/src/telemetry/constants.ts @@ -80,3 +80,4 @@ export const BASELINE = Object.freeze({ export const CLIENT_SIDE_NAVIGATION = "client_side_nav"; export const LANGUAGE = "language"; export const THEME_SWITCHER = "theme_switcher"; +export const SURVEY = "survey"; diff --git a/client/src/ui/molecules/document-survey/index.tsx b/client/src/ui/molecules/document-survey/index.tsx index 7f1fa43b1dbf..e3473cbf9def 100644 --- a/client/src/ui/molecules/document-survey/index.tsx +++ b/client/src/ui/molecules/document-survey/index.tsx @@ -7,6 +7,8 @@ import { useIsServer } from "../../../hooks"; import { Icon } from "../../atoms/icon"; import { useLocation } from "react-router"; import { DEV_MODE, WRITER_MODE } from "../../../env"; +import { useGleanClick } from "../../../telemetry/glean-context"; +import { SURVEY } from "../../../telemetry/constants"; const FORCE_SURVEY_PREFIX = "#FORCE_SURVEY="; @@ -49,6 +51,7 @@ export function DocumentSurvey({ doc }: { doc: Doc }) { } function SurveyDisplay({ survey, force }: { survey: Survey; force: boolean }) { + const gleanClick = useGleanClick(); const details = React.useRef(null); const [originalState] = React.useState(() => getSurveyState(survey.bucket)); @@ -63,6 +66,7 @@ function SurveyDisplay({ survey, force }: { survey: Survey; force: boolean }) { ...state, dismissed_at: Date.now(), }); + gleanClick(`${SURVEY}: dismissed ${survey.bucket}`); } function submitted() { @@ -70,6 +74,7 @@ function SurveyDisplay({ survey, force }: { survey: Survey; force: boolean }) { ...state, submitted_at: Date.now(), }); + gleanClick(`${SURVEY}: submitted ${survey.bucket}`); } React.useEffect(() => { @@ -98,8 +103,9 @@ function SurveyDisplay({ survey, force }: { survey: Survey; force: boolean }) { ...state, seen_at: Date.now(), }); + gleanClick(`${SURVEY}: seen ${survey.bucket}`); } - }, [state]); + }, [state, gleanClick, survey.bucket]); React.useEffect(() => { // For this to work, the Survey needs this JavaScript action: From 6af0ecf6916fe81efc330ea49222a55bc7469056 Mon Sep 17 00:00:00 2001 From: Claas Augner Date: Fri, 7 Jun 2024 20:50:04 +0200 Subject: [PATCH 2/8] fix(telemetry): avoid measuring seen survey twice --- client/src/ui/molecules/document-survey/index.tsx | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/client/src/ui/molecules/document-survey/index.tsx b/client/src/ui/molecules/document-survey/index.tsx index e3473cbf9def..2a47b9735085 100644 --- a/client/src/ui/molecules/document-survey/index.tsx +++ b/client/src/ui/molecules/document-survey/index.tsx @@ -56,6 +56,7 @@ function SurveyDisplay({ survey, force }: { survey: Survey; force: boolean }) { const [originalState] = React.useState(() => getSurveyState(survey.bucket)); const [state, setState] = React.useState(() => originalState); + const seen = React.useRef(!!originalState.seen_at); React.useEffect(() => { writeSurveyState(survey.bucket, state); @@ -103,7 +104,10 @@ function SurveyDisplay({ survey, force }: { survey: Survey; force: boolean }) { ...state, seen_at: Date.now(), }); - gleanClick(`${SURVEY}: seen ${survey.bucket}`); + if (!seen.current) { + seen.current = true; + gleanClick(`${SURVEY}: seen ${survey.bucket}`); + } } }, [state, gleanClick, survey.bucket]); From 9f322367d86b816fe0297d72d873ec38c3fd8832 Mon Sep 17 00:00:00 2001 From: Claas Augner Date: Fri, 7 Jun 2024 20:56:37 +0200 Subject: [PATCH 3/8] chore(telemetry): measure when survey is opened --- client/src/ui/molecules/document-survey/index.tsx | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/client/src/ui/molecules/document-survey/index.tsx b/client/src/ui/molecules/document-survey/index.tsx index 2a47b9735085..26362d9d411a 100644 --- a/client/src/ui/molecules/document-survey/index.tsx +++ b/client/src/ui/molecules/document-survey/index.tsx @@ -90,13 +90,14 @@ function SurveyDisplay({ survey, force }: { survey: Survey; force: boolean }) { ...state, opened_at: Date.now(), }); + gleanClick(`${SURVEY}: opened ${survey.bucket}`); } }; current.addEventListener("toggle", listener); return () => current.removeEventListener("toggle", listener); - }, [details, state, survey]); + }, [details, state, survey, gleanClick]); React.useEffect(() => { if (!state.seen_at) { From eafc3f917281725399f3b02fc18fc9d9485324b1 Mon Sep 17 00:00:00 2001 From: Claas Augner Date: Fri, 7 Jun 2024 21:26:55 +0200 Subject: [PATCH 4/8] fixup! fix(telemetry): avoid measuring seen survey twice --- .../ui/molecules/document-survey/index.tsx | 34 ++++++++++++------- 1 file changed, 22 insertions(+), 12 deletions(-) diff --git a/client/src/ui/molecules/document-survey/index.tsx b/client/src/ui/molecules/document-survey/index.tsx index 26362d9d411a..ec5e34627831 100644 --- a/client/src/ui/molecules/document-survey/index.tsx +++ b/client/src/ui/molecules/document-survey/index.tsx @@ -56,12 +56,29 @@ function SurveyDisplay({ survey, force }: { survey: Survey; force: boolean }) { const [originalState] = React.useState(() => getSurveyState(survey.bucket)); const [state, setState] = React.useState(() => originalState); - const seen = React.useRef(!!originalState.seen_at); React.useEffect(() => { writeSurveyState(survey.bucket, state); }, [state, survey.bucket]); + const seen = React.useCallback(() => { + if (state.seen_at) { + return; + } + + const timeoutId = setTimeout( + () => + setState((state) => ({ + ...state, + seen_at: Date.now(), + })), + 0 + ); + gleanClick(`${SURVEY}: seen ${survey.bucket}`); + + return () => clearTimeout(timeoutId); + }, [gleanClick, state.seen_at, survey.bucket]); + function dismiss() { setState({ ...state, @@ -100,17 +117,10 @@ function SurveyDisplay({ survey, force }: { survey: Survey; force: boolean }) { }, [details, state, survey, gleanClick]); React.useEffect(() => { - if (!state.seen_at) { - setState({ - ...state, - seen_at: Date.now(), - }); - if (!seen.current) { - seen.current = true; - gleanClick(`${SURVEY}: seen ${survey.bucket}`); - } - } - }, [state, gleanClick, survey.bucket]); + const timeoutId = setTimeout(() => seen(), 0); + + return () => clearTimeout(timeoutId); + }, [seen]); React.useEffect(() => { // For this to work, the Survey needs this JavaScript action: From b124bd38979c3d8fe758b55dc7d84a284bc5aa6d Mon Sep 17 00:00:00 2001 From: Claas Augner Date: Fri, 7 Jun 2024 21:36:44 +0200 Subject: [PATCH 5/8] fixup! fixup! fix(telemetry): avoid measuring seen survey twice --- .../ui/molecules/document-survey/index.tsx | 27 +++++++------------ 1 file changed, 10 insertions(+), 17 deletions(-) diff --git a/client/src/ui/molecules/document-survey/index.tsx b/client/src/ui/molecules/document-survey/index.tsx index ec5e34627831..ae41cf864e83 100644 --- a/client/src/ui/molecules/document-survey/index.tsx +++ b/client/src/ui/molecules/document-survey/index.tsx @@ -62,22 +62,13 @@ function SurveyDisplay({ survey, force }: { survey: Survey; force: boolean }) { }, [state, survey.bucket]); const seen = React.useCallback(() => { - if (state.seen_at) { - return; - } + setState((state) => ({ + ...state, + seen_at: Date.now(), + })); - const timeoutId = setTimeout( - () => - setState((state) => ({ - ...state, - seen_at: Date.now(), - })), - 0 - ); gleanClick(`${SURVEY}: seen ${survey.bucket}`); - - return () => clearTimeout(timeoutId); - }, [gleanClick, state.seen_at, survey.bucket]); + }, [gleanClick, survey.bucket]); function dismiss() { setState({ @@ -117,10 +108,12 @@ function SurveyDisplay({ survey, force }: { survey: Survey; force: boolean }) { }, [details, state, survey, gleanClick]); React.useEffect(() => { - const timeoutId = setTimeout(() => seen(), 0); + if (!state.seen_at) { + const timeoutId = setTimeout(seen, 0); - return () => clearTimeout(timeoutId); - }, [seen]); + return () => clearTimeout(timeoutId); + } + }, [seen, state.seen_at]); React.useEffect(() => { // For this to work, the Survey needs this JavaScript action: From ac855e573285f69213934788db5c54086c3aee75 Mon Sep 17 00:00:00 2001 From: Claas Augner Date: Fri, 7 Jun 2024 21:47:55 +0200 Subject: [PATCH 6/8] fix(telemetry): move measurement into state callback --- .../ui/molecules/document-survey/index.tsx | 37 ++++++++++--------- 1 file changed, 20 insertions(+), 17 deletions(-) diff --git a/client/src/ui/molecules/document-survey/index.tsx b/client/src/ui/molecules/document-survey/index.tsx index ae41cf864e83..77496767eaa6 100644 --- a/client/src/ui/molecules/document-survey/index.tsx +++ b/client/src/ui/molecules/document-survey/index.tsx @@ -61,21 +61,30 @@ function SurveyDisplay({ survey, force }: { survey: Survey; force: boolean }) { writeSurveyState(survey.bucket, state); }, [state, survey.bucket]); + const measure = React.useCallback( + (action: string) => gleanClick(`${SURVEY}: ${action} ${survey.bucket}`), + [gleanClick, survey.bucket] + ); + const seen = React.useCallback(() => { - setState((state) => ({ - ...state, - seen_at: Date.now(), - })); + setState((state) => { + if (!state.seen_at) { + measure("seen"); + } - gleanClick(`${SURVEY}: seen ${survey.bucket}`); - }, [gleanClick, survey.bucket]); + return { + ...state, + seen_at: Date.now(), + }; + }); + }, [measure]); function dismiss() { setState({ ...state, dismissed_at: Date.now(), }); - gleanClick(`${SURVEY}: dismissed ${survey.bucket}`); + measure("dismissed"); } function submitted() { @@ -83,7 +92,7 @@ function SurveyDisplay({ survey, force }: { survey: Survey; force: boolean }) { ...state, submitted_at: Date.now(), }); - gleanClick(`${SURVEY}: submitted ${survey.bucket}`); + measure("submitted"); } React.useEffect(() => { @@ -98,22 +107,16 @@ function SurveyDisplay({ survey, force }: { survey: Survey; force: boolean }) { ...state, opened_at: Date.now(), }); - gleanClick(`${SURVEY}: opened ${survey.bucket}`); + measure("opened"); } }; current.addEventListener("toggle", listener); return () => current.removeEventListener("toggle", listener); - }, [details, state, survey, gleanClick]); - - React.useEffect(() => { - if (!state.seen_at) { - const timeoutId = setTimeout(seen, 0); + }, [details, state, survey, measure]); - return () => clearTimeout(timeoutId); - } - }, [seen, state.seen_at]); + React.useEffect(seen, [seen]); React.useEffect(() => { // For this to work, the Survey needs this JavaScript action: From 2160f26657a220aa32aff2eeed0023140860bc2b Mon Sep 17 00:00:00 2001 From: Claas Augner Date: Fri, 7 Jun 2024 23:00:27 +0200 Subject: [PATCH 7/8] fix(survey): avoid overwriting seen_at --- client/src/ui/molecules/document-survey/index.tsx | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/client/src/ui/molecules/document-survey/index.tsx b/client/src/ui/molecules/document-survey/index.tsx index 77496767eaa6..fc906f63b0d1 100644 --- a/client/src/ui/molecules/document-survey/index.tsx +++ b/client/src/ui/molecules/document-survey/index.tsx @@ -68,10 +68,12 @@ function SurveyDisplay({ survey, force }: { survey: Survey; force: boolean }) { const seen = React.useCallback(() => { setState((state) => { - if (!state.seen_at) { - measure("seen"); + if (state.seen_at) { + return state; } + measure("seen"); + return { ...state, seen_at: Date.now(), From a3d827ae5d0931334b940b5583b06a90ff00d0f4 Mon Sep 17 00:00:00 2001 From: Claas Augner Date: Fri, 7 Jun 2024 23:01:18 +0200 Subject: [PATCH 8/8] refactor(surveys): move measurement before setState for consistency --- client/src/ui/molecules/document-survey/index.tsx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/client/src/ui/molecules/document-survey/index.tsx b/client/src/ui/molecules/document-survey/index.tsx index fc906f63b0d1..8f1ff6fd844f 100644 --- a/client/src/ui/molecules/document-survey/index.tsx +++ b/client/src/ui/molecules/document-survey/index.tsx @@ -82,19 +82,19 @@ function SurveyDisplay({ survey, force }: { survey: Survey; force: boolean }) { }, [measure]); function dismiss() { + measure("dismissed"); setState({ ...state, dismissed_at: Date.now(), }); - measure("dismissed"); } function submitted() { + measure("submitted"); setState({ ...state, submitted_at: Date.now(), }); - measure("submitted"); } React.useEffect(() => { @@ -105,11 +105,11 @@ function SurveyDisplay({ survey, force }: { survey: Survey; force: boolean }) { const listener = () => { if (current.open && !state.opened_at) { + measure("opened"); setState({ ...state, opened_at: Date.now(), }); - measure("opened"); } };