chore(compass-telemetry): add right-click telemetry events for Compass COMPASS-9660 #7168
Conversation
| @@ -44,8 +44,7 @@ const tsxRules = { | |||
| 'react-hooks/exhaustive-deps': [ | |||
| 'warn', | |||
| { | |||
| additionalHooks: | |||
| '(useTrackOnChange|useContextMenuItems|useContextMenuGroups)', | |||
| additionalHooks: '(useTrackOnChange|useContextMenuGroups)', | |||
There was a problem hiding this comment.
we seem to be converging on useContextMenuGroups so I'm removing the old useContextMenuItems syntax
There was a problem hiding this comment.
Pull Request Overview
This PR adds telemetry tracking to context menu interactions in Compass by introducing event tracking for context menu openings and item clicks. The changes restructure the context menu system to support telemetry labels on menu groups and implement the tracking logic.
- Refactors context menu API from item-based to group-based structure with telemetry labels
- Adds telemetry event tracking for context menu opens and item clicks
- Updates all context menu usage throughout the codebase to use the new group-based API
Reviewed Changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| packages/compass/src/app/components/home.tsx | Adds telemetry tracking callbacks to the main Compass component |
| packages/compass-telemetry/src/telemetry-events.ts | Defines new telemetry event types for context menu interactions |
| packages/compass-context-menu/src/use-context-menu.tsx | Updates API from registerItems to registerItemGroups with telemetry support |
| packages/compass-context-menu/src/types.ts | Adds ContextMenuItemGroup type with telemetry labels |
| packages/compass-context-menu/src/context-menu-provider.tsx | Implements onContextMenuOpen callback support |
| packages/compass-components/src/components/context-menu.tsx | Adds telemetry click tracking and removes deprecated useContextMenuItems |
| packages/compass-sidebar/src/components/multiple-connections/connections-navigation.tsx | Migrates from useContextMenuItems to useContextMenuGroups |
| packages/compass-crud/src/components/use-document-item-context-menu.tsx | Updates context menu structure with telemetry labels |
| packages/compass-crud/src/components/crud-toolbar.tsx | Migrates to group-based context menu API |
| packages/compass-connections-navigation/src/context-menus.ts | Adds telemetry label parameter to context menu helper |
| packages/compass-connections-navigation/src/connections-navigation-tree.tsx | Provides telemetry labels for tree item context menus |
| packages/compass-components/src/components/workspace-tabs/tab.tsx | Migrates tab context menu to new API |
| packages/compass-components/src/components/document-list/element.tsx | Updates element field context menu with telemetry |
| packages/compass-components/src/components/compass-components-provider.tsx | Adds telemetry callback props to the provider |
| configs/eslint-config-compass/index.js | Removes deprecated useContextMenuItems from ESLint config |
Comments suppressed due to low confidence (1)
packages/compass-crud/src/components/use-document-item-context-menu.tsx:57
- The ternary operator condition
isEditableis missing proper structure. The conditional item should be wrapped in brackets or properly structured asisEditable ? { ... } : undefined.
isEditable
| ], | ||
| }, | ||
| { | ||
| telemetryLabel: 'Document Item Delete', |
There was a problem hiding this comment.
intentionally moving this to its own group since we have this pattern of separating deletions into its own thing
gagik
changed the title
| onContextMenuOpen={(itemGroups) => { | ||
| if (itemGroups.length > 0) { | ||
| track('Context Menu Opened', { | ||
| item_groups: itemGroups.map((group) => group.telemetryLabel), | ||
| }); | ||
| } | ||
| }} |
There was a problem hiding this comment.
This is slightly annoying, but this only adds tracking on the compass desktop side, you'd want the same code on the compass-web side too (until we have time to deal with https://jira.mongodb.org/browse/COMPASS-7830)
There was a problem hiding this comment.
There was a problem hiding this comment.
You don't need to do this in DE, this is something you have to do in the entrypoint in compass-web package 🙂
| @@ -85,7 +94,7 @@ export function ContextMenuProvider({ | |||
| capture: true, | |||
| }); | |||
| }; | |||
| }, [disabled, handleClosingEvent, parentContext]); | |||
| }, [disabled, handleClosingEvent, onContextMenuOpen, parentContext]); | |||
There was a problem hiding this comment.
Is this an expected effect change here? Seems very easy to cause this effect to run on almost every render for no good reason. Your current callback implementation for example recreates functions every render and while in desktop this high level render is more or less stable, in compass-web we are quite some levels down, so React will be re-rendering compass-web quite often
| @@ -100,7 +92,8 @@ export function ContextMenuProvider({ | |||
| capture: true, | |||
| }); | |||
| }; | |||
| }, [disabled, handleClosingEvent, handleContextMenuOpen, parentContext]); | |||
| // eslint-disable-next-line react-hooks/exhaustive-deps | |||
| }, [disabled, handleClosingEvent, parentContext]); | |||
There was a problem hiding this comment.
@gribnoysup is removing it from the dependencies a good enough solution? I was also thinking of wrapping it in a useCallback with [] but that doesn't seem any better. alternatively I could do a `useCallback on application-level
There was a problem hiding this comment.
In other parts of the app we use a useRef that updates in render to keep the latest stable reference to the prop (recent example). I personally do prefer this to just adding an exception (because an exception doesn't differenciate and if you change dependencies later you might miss actually important dependencies) and it means you are actually calling the latest version of the function, but an exception is also fine in this particular case, I'd just suggest to add a comment explaining why you are adding an exception here.
gagik
force-pushed
the
gagik/rc-telemetry
branch
from
71b2cd4 to
9bcb7c0
Compare
gagik
force-pushed
the
gagik/rc-telemetry
branch
from
9bcb7c0 to
9014d4e
Compare
lerouxb
changed the title
Adds telemetry events to right click menu actions.
Pending CI tests.