Skip to content

feat: ga event#3436

Merged
cs1707 merged 3 commits intodevelopfrom
feat/ga-event
Jan 23, 2026
Merged

feat: ga event#3436
cs1707 merged 3 commits intodevelopfrom
feat/ga-event

Conversation

@cs1707
Copy link
Copy Markdown
Collaborator

@cs1707 cs1707 commented Jan 22, 2026

  • fix: desktop ga event
  • feat: add swap to token event

@cs1707
Copy link
Copy Markdown
Collaborator Author

cs1707 commented Jan 23, 2026

@claude review this PR

@claude
Copy link
Copy Markdown
Contributor

claude bot commented Jan 23, 2026

Claude finished @cs1707's task in 1m 6s —— View job


PR Review: GA Event Tracking

  • Gather context and read changed files
  • Analyze code changes
  • Provide review feedback

Summary

This PR adds Google Analytics (GA4) and Matomo event tracking to the desktop navigation and swap token selection flows. The changes are well-structured and follow existing patterns in the codebase.


Changes Overview

File Changes
src/ui/component/DesktopNav/index.tsx Added navigation tracking (Portfolio, Perps, Lending, Prediction)
src/ui/component/TokenSelect/index.tsx Added callback props for token modal events
src/ui/component/TokenSelector/index.tsx Added onSelectRecentToken callback
src/ui/views/DesktopProfile/index.tsx Removed duplicate Portfolio tracking (moved to DesktopNav)
src/ui/views/Swap/Component/Token.tsx Added swap token selection events

Review Feedback

Overall: Approve - The code is well-written and follows existing patterns.

Positive observations:

  1. Good consolidation of the Portfolio tracking from DesktopProfile into DesktopNav - this is a cleaner approach that handles all navigation events in one place
  2. The new onOpenTokenModal and onSelectRecentToken callbacks in TokenSelect follow good React patterns for component composition
  3. Consistent use of both matomoRequestEvent and ga4.fireEvent for dual analytics tracking

Minor suggestions (non-blocking):

  1. DesktopNav event firing on mount (src/ui/component/DesktopNav/index.tsx:118-130)

    The useEffect fires whenever activeNav?.eventKey changes, which will happen on:

    • Initial component mount
    • Navigation between tabs

    This is likely the intended behavior, but be aware it will fire on every page load/refresh. If you only want to track explicit user navigation clicks, you might consider moving the tracking to the onClick handler instead.

  2. Event naming consistency (src/ui/views/Swap/Component/Token.tsx:239-262)

    The event category is TokenSelect while the action is Swap_To_Token. Consider whether Swap or TokenSelect should be the category for consistency with other swap-related events.

  3. Potential duplicate event firing (src/ui/component/TokenSelector/index.tsx:602-604)

    When a recent token is clicked, both onConfirm(token) and onSelectRecentToken?.(token) are called. If any parent component fires events in both callbacks, this could lead to duplicate tracking. The current implementation in Token.tsx handles this correctly by only tracking in onSelectRecentToken, but future maintainers should be aware of this.


LGTM - Ready to merge once CI passes.

@cs1707 cs1707 merged commit 4029a14 into develop Jan 23, 2026
9 checks passed
@cs1707 cs1707 deleted the feat/ga-event branch January 23, 2026 08:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant