[Blockly] Adding Statsig logging proxy metric for keyboard users #70006
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds analytics tracking to identify potential keyboard navigation users in Blockly by logging when they press the '/' key to open the shortcuts menu.
- Adds a new Statsig event constant for tracking slash key presses
- Implements keyboard event listener on the blockly-div element to capture '/' key presses
- Includes proper cleanup logic to prevent duplicate listeners during re-initialization
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| apps/src/metrics/AnalyticsConstants.js | Adds new event constant for tracking slash key presses |
| apps/src/blockly/addons/cdoKeyboardNavigation.ts | Implements slash key listener with cleanup handlers and integrates into keyboard navigation initialization |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| analyticsReporter.sendEvent( | ||
| EVENTS.BLOCKLY_SLASH_KEY_PRESSED, | ||
| {}, | ||
| PLATFORMS.STATSIG | ||
| ); |
There was a problem hiding this comment.
The analytics event is sent every time '/' is pressed, which could result in duplicate events if a user presses the key multiple times or holds it down. Consider debouncing or throttling this event, or tracking only the first occurrence per session to avoid inflating metrics.
There was a problem hiding this comment.
I think we want to know truly how often users are pressing this key.
|
|
||
| // Create and store new listener | ||
| slashKeyListener = handleSlashKeyPress; | ||
| blocklyDiv.addEventListener('keydown', slashKeyListener); |
There was a problem hiding this comment.
The event listener is attached every time initializeKeyboardNavigation is called without checking if one already exists at the DOM level. While the code removes the listener reference before adding a new one, consider using addEventListener with {once: true} for the first occurrence tracking, or add a flag to track if this has already been logged in the current session.
There was a problem hiding this comment.
We already take care of this by double checking and removing if one exists.
Warning!!
We have entered Pixel Lock for Hour of AI! All merges to the
stagingbranch from Dec 2 through Dec 12 must go through live change review and be deemed critical for supporting the Hour of AI. External contributions will not be accepted at this time.For non-critical changes, please change your base to
staging-nextand delete this warning. We will mergestaging-nextintostagingon Dec 15, 2025.This PR adds a metric that listens for users who press '/' to open the shortcuts menu. This might be a possible indicator that we have a keyboard navigation user.
I was able to confirm that the metric logs when focus is in the blockly div, and doesn't log outside that focus. Note- this time Claude was very helpful and I only needed to make minor tweaks, but another set of eyes would be helpful to make sure I'm not biased to the tool's solution.
Links
Testing story
Deployment strategy
Follow-up work
Privacy
Security
Caching
PR Creation Checklist: