Skip to content

Commit 817ba2c

Browse files
committed
ref(cmd-k): Extract analytics into observer hook, simplify
- Move all analytics into useCommandPaletteAnalytics hook that reads state from context and tracks events internally - Generate session_id inside the hook (not in shared state context) - Remove _closeMethod hacky module-level variable - Remove method field from closed event (completed boolean suffices) - UI components are now free of analytics code - Hook returns only recordAction for final navigate/callback actions
1 parent d73a49c commit 817ba2c

File tree

6 files changed

+209
-206
lines changed

6 files changed

+209
-206
lines changed

static/app/actionCreators/modal.tsx

Lines changed: 5 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,3 @@
1-
import uniqueId from 'lodash/uniqueId';
2-
31
import type {
42
CommandPaletteState,
53
CommandPaletteDispatch,
@@ -162,33 +160,22 @@ export async function toggleCommandPalette(
162160
dispatch: CommandPaletteDispatch,
163161
source: 'button' | 'keyboard'
164162
) {
165-
const modalModule = await import('sentry/components/commandPalette/ui/modal');
166-
const {default: Modal, modalCss} = modalModule;
163+
const {default: Modal, modalCss} =
164+
await import('sentry/components/commandPalette/ui/modal');
167165

168166
function closeCommandPaletteModal() {
169167
dispatch({type: 'toggle modal'});
170168
}
171169

172170
if (state.open) {
173-
modalModule._closeMethod = 'keyboard_toggle';
174171
closeCommandPaletteModal();
175172
closeModal();
176173
} else {
177-
const sessionId = uniqueId('cmd-palette-');
178-
trackAnalytics('command_palette.opened', {
179-
organization,
180-
source,
181-
session_id: sessionId,
182-
});
183-
dispatch({type: 'toggle modal', session_id: sessionId});
174+
trackAnalytics('command_palette.opened', {organization, source});
175+
dispatch({type: 'toggle modal'});
184176
openModal(deps => <Modal {...deps} {...options} />, {
185177
modalCss,
186-
onClose: reason => {
187-
if (reason === 'backdrop-click') {
188-
modalModule._closeMethod = 'backdrop_click';
189-
}
190-
closeCommandPaletteModal();
191-
},
178+
onClose: closeCommandPaletteModal,
192179
});
193180
}
194181
}

static/app/components/commandPalette/ui/commandPalette.tsx

Lines changed: 7 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,11 @@
1-
import {Fragment, useCallback, useEffect, useLayoutEffect, useMemo, useRef} from 'react';
1+
import {Fragment, useCallback, useLayoutEffect, useMemo} from 'react';
22
import {preload} from 'react-dom';
33
import {useTheme} from '@emotion/react';
44
import styled from '@emotion/styled';
55
import {ListKeyboardDelegate, useSelectableCollection} from '@react-aria/selection';
66
import {mergeProps} from '@react-aria/utils';
77
import {Item} from '@react-stately/collections';
88
import {useTreeState} from '@react-stately/tree';
9-
import * as Sentry from '@sentry/react';
109
import {AnimatePresence, motion} from 'framer-motion';
1110

1211
import errorIllustration from 'sentry-images/spot/computer-missing.svg';
@@ -29,14 +28,13 @@ import {
2928
useCommandPaletteDispatch,
3029
useCommandPaletteState,
3130
} from 'sentry/components/commandPalette/ui/commandPaletteStateContext';
31+
import {useCommandPaletteAnalytics} from 'sentry/components/commandPalette/useCommandPaletteAnalytics';
3232
import {FeedbackButton} from 'sentry/components/feedbackButton/feedbackButton';
3333
import {IconArrow, IconClose, IconSearch} from 'sentry/icons';
3434
import {IconDefaultsProvider} from 'sentry/icons/useIconDefaults';
3535
import {t} from 'sentry/locale';
36-
import {trackAnalytics} from 'sentry/utils/analytics';
3736
import {fzf} from 'sentry/utils/search/fzf';
3837
import type {Theme} from 'sentry/utils/theme';
39-
import {useOrganization} from 'sentry/utils/useOrganization';
4038

4139
const MotionButton = motion.create(Button);
4240
const MotionIconSearch = motion.create(IconSearch);
@@ -65,27 +63,16 @@ type CommandPaletteActionWithPriority = CommandPaletteActionWithKey & {
6563
};
6664

6765
interface CommandPaletteProps {
68-
onAction: (
69-
action: Exclude<CommandPaletteActionWithKey, {type: 'group'}>,
70-
resultIndex: number,
71-
group: string
72-
) => void;
73-
onQueryTracked?: () => void;
74-
sessionId?: string;
66+
onAction: (action: Exclude<CommandPaletteActionWithKey, {type: 'group'}>) => void;
7567
}
7668

7769
export function CommandPalette(props: CommandPaletteProps) {
7870
const theme = useTheme();
7971

8072
const actions = useCommandPaletteActions();
81-
const organization = useOrganization();
8273
const state = useCommandPaletteState();
8374
const dispatch = useCommandPaletteDispatch();
8475

85-
// Debounced query tracking (500ms)
86-
const queryTimerRef = useRef<ReturnType<typeof setTimeout> | null>(null);
87-
const lastTrackedQueryRef = useRef('');
88-
8976
// Preload the empty state image so it's ready if/when there are no results
9077
// Guard against non-string imports (e.g. SVG objects in test environments)
9178
if (typeof errorIllustration === 'string') {
@@ -107,30 +94,7 @@ export function CommandPalette(props: CommandPaletteProps) {
10794
[state.query, displayedActions]
10895
);
10996

110-
// Debounced query analytics
111-
useEffect(() => {
112-
if (queryTimerRef.current) {
113-
clearTimeout(queryTimerRef.current);
114-
}
115-
if (state.query.length > 0 && state.query !== lastTrackedQueryRef.current) {
116-
queryTimerRef.current = setTimeout(() => {
117-
lastTrackedQueryRef.current = state.query;
118-
trackAnalytics('command_palette.searched', {
119-
organization,
120-
query: state.query,
121-
result_count: filteredActions.length,
122-
session_id: props.sessionId ?? '',
123-
});
124-
props.onQueryTracked?.();
125-
}, 500);
126-
}
127-
return () => {
128-
if (queryTimerRef.current) {
129-
clearTimeout(queryTimerRef.current);
130-
}
131-
};
132-
// eslint-disable-next-line react-hooks/exhaustive-deps
133-
}, [state.query, filteredActions.length]);
97+
const analytics = useCommandPaletteAnalytics(filteredActions.length);
13498

13599
const sections = useMemo(
136100
() => groupActionsBySection(filteredActions),
@@ -210,26 +174,16 @@ export function CommandPalette(props: CommandPaletteProps) {
210174
return;
211175
}
212176

213-
const group = action.groupingKey ?? '';
214-
215177
if (action.type === 'group') {
216-
trackAnalytics('command_palette.action_selected', {
217-
organization,
218-
action: action.display.label,
219-
query: state.query,
220-
action_type: 'group',
221-
group,
222-
result_index: resultIndex,
223-
session_id: props.sessionId ?? '',
224-
});
225178
dispatch({type: 'push action', action});
226179
return;
227180
}
228181

182+
analytics.recordAction(action, resultIndex, action.groupingKey ?? '');
229183
dispatch({type: 'trigger action'});
230-
props.onAction(action, resultIndex, group);
184+
props.onAction(action);
231185
},
232-
[filteredActions, dispatch, props, treeState, organization, state.query]
186+
[filteredActions, dispatch, props, analytics, treeState]
233187
);
234188

235189
return (
@@ -486,26 +440,6 @@ function flattenActions(
486440
}
487441

488442
function CommandPaletteNoResults() {
489-
const organization = useOrganization();
490-
const {query, action, session_id} = useCommandPaletteState();
491-
492-
useEffect(() => {
493-
const actionLabel =
494-
typeof action?.value.action.display.label === 'string'
495-
? action.value.action.display.label
496-
: undefined;
497-
trackAnalytics('command_palette.no_results', {
498-
organization,
499-
query,
500-
action: actionLabel,
501-
session_id,
502-
});
503-
Sentry.logger.info('Command palette returned no results', {
504-
query,
505-
action: actionLabel,
506-
});
507-
}, [organization, query, action, session_id]);
508-
509443
return (
510444
<Flex
511445
direction="column"

static/app/components/commandPalette/ui/commandPaletteStateContext.tsx

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,13 +19,12 @@ export type CommandPaletteState = {
1919
input: React.RefObject<HTMLInputElement | null>;
2020
open: boolean;
2121
query: string;
22-
session_id: string;
2322
};
2423

2524
export type CommandPaletteDispatch = React.Dispatch<CommandPaletteAction>;
2625

2726
export type CommandPaletteAction =
28-
| {type: 'toggle modal'; session_id?: string}
27+
| {type: 'toggle modal'}
2928
| {type: 'reset'}
3029
| {query: string; type: 'set query'}
3130
| {action: CommandPaletteActionWithKey; type: 'push action'}
@@ -46,7 +45,6 @@ function commandPaletteReducer(
4645
return {
4746
...state,
4847
open: !state.open,
49-
session_id: action.session_id ?? state.session_id,
5048
};
5149
case 'reset':
5250
return {
@@ -110,7 +108,6 @@ export function CommandPaletteStateProvider({
110108
query: '',
111109
action: null,
112110
open: false,
113-
session_id: '',
114111
});
115112

116113
return (

static/app/components/commandPalette/ui/modal.tsx

Lines changed: 10 additions & 101 deletions
Original file line numberDiff line numberDiff line change
@@ -1,136 +1,45 @@
1-
import {useCallback, useEffect, useRef} from 'react';
1+
import {useCallback} from 'react';
22
import {css} from '@emotion/react';
33

44
import type {ModalRenderProps} from 'sentry/actionCreators/modal';
55
import {closeModal} from 'sentry/actionCreators/modal';
66
import type {CommandPaletteActionWithKey} from 'sentry/components/commandPalette/types';
77
import {CommandPalette} from 'sentry/components/commandPalette/ui/commandPalette';
8-
import {
9-
getActionPath,
10-
useCommandPaletteState,
11-
} from 'sentry/components/commandPalette/ui/commandPaletteStateContext';
8+
import {useCommandPaletteState} from 'sentry/components/commandPalette/ui/commandPaletteStateContext';
129
import {useDsnLookupActions} from 'sentry/components/commandPalette/useDsnLookupActions';
13-
import {trackAnalytics} from 'sentry/utils/analytics';
1410
import type {Theme} from 'sentry/utils/theme';
1511
import {unreachable} from 'sentry/utils/unreachable';
1612
import {normalizeUrl} from 'sentry/utils/url/normalizeUrl';
1713
import {useNavigate} from 'sentry/utils/useNavigate';
18-
import {useOrganization} from 'sentry/utils/useOrganization';
19-
20-
/**
21-
* Stores the close method so the unmount effect can include it in analytics.
22-
* Set externally by toggleCommandPalette's onClose callback.
23-
*/
24-
export let _closeMethod = 'escape';
2514

2615
export default function CommandPaletteModal({Body}: ModalRenderProps) {
2716
const navigate = useNavigate();
28-
const organization = useOrganization();
29-
30-
const state = useCommandPaletteState();
31-
const {query, session_id} = state;
17+
const {query} = useCommandPaletteState();
3218

3319
useDsnLookupActions(query);
3420

35-
const openedAtRef = useRef(Date.now());
36-
const actionsSelectedRef = useRef(0);
37-
const queriesTypedRef = useRef(0);
38-
const completedRef = useRef(false);
39-
const stateRef = useRef(state);
40-
stateRef.current = state;
41-
42-
// Reset close method on mount
43-
_closeMethod = 'escape';
44-
45-
// Fire closed + session summary on unmount
46-
useEffect(() => {
47-
const openedAt = openedAtRef.current;
48-
// These refs accumulate values during the modal's lifetime and are intentionally
49-
// read at cleanup time to capture the final session metrics.
50-
const actions = actionsSelectedRef;
51-
const queries = queriesTypedRef;
52-
const done = completedRef;
53-
54-
return () => {
55-
const s = stateRef.current;
56-
let depth = 0;
57-
let node = s.action;
58-
while (node !== null) {
59-
depth++;
60-
node = node.previous;
61-
}
62-
63-
trackAnalytics('command_palette.closed', {
64-
organization,
65-
method: _closeMethod as 'escape',
66-
query: s.query,
67-
had_interaction: s.query.length > 0 || s.action !== null || actions.current > 0,
68-
session_id: s.session_id,
69-
});
70-
71-
trackAnalytics('command_palette.session', {
72-
organization,
73-
session_id: s.session_id,
74-
duration_ms: Date.now() - openedAt,
75-
actions_selected: actions.current,
76-
queries_typed: queries.current,
77-
completed: done.current,
78-
max_drill_depth: depth,
79-
});
80-
};
81-
// eslint-disable-next-line react-hooks/exhaustive-deps
82-
}, []);
83-
8421
const handleSelect = useCallback(
85-
(
86-
action: Exclude<CommandPaletteActionWithKey, {type: 'group'}>,
87-
resultIndex: number,
88-
group: string
89-
) => {
22+
(action: Exclude<CommandPaletteActionWithKey, {type: 'group'}>) => {
9023
const actionType = action.type;
9124
switch (actionType) {
9225
case 'navigate':
93-
case 'callback': {
94-
const path = getActionPath(state);
95-
const label =
96-
path.length > 0 ? `${path}${action.display.label}` : action.display.label;
97-
trackAnalytics('command_palette.action_selected', {
98-
organization,
99-
action: label,
100-
query,
101-
action_type: actionType,
102-
group,
103-
result_index: resultIndex,
104-
session_id,
105-
});
106-
_closeMethod = 'action_selected';
107-
actionsSelectedRef.current++;
108-
completedRef.current = true;
109-
if (actionType === 'navigate') {
110-
navigate(normalizeUrl(action.to));
111-
} else {
112-
action.onAction();
113-
}
26+
navigate(normalizeUrl(action.to));
27+
break;
28+
case 'callback':
29+
action.onAction();
11430
break;
115-
}
11631
default:
11732
unreachable(actionType);
11833
break;
11934
}
12035
closeModal();
12136
},
122-
[navigate, organization, state, query, session_id]
37+
[navigate]
12338
);
12439

12540
return (
12641
<Body>
127-
<CommandPalette
128-
onAction={handleSelect}
129-
sessionId={session_id}
130-
onQueryTracked={() => {
131-
queriesTypedRef.current++;
132-
}}
133-
/>
42+
<CommandPalette onAction={handleSelect} />
13443
</Body>
13544
);
13645
}

0 commit comments

Comments
 (0)