Skip to content

Implement grid view analytics#514

Merged
sarangj merged 8 commits into
qafrom
final_gridalytics
Feb 12, 2026
Merged

Implement grid view analytics#514
sarangj merged 8 commits into
qafrom
final_gridalytics

Conversation

@sarangj

@sarangj sarangj commented Jan 26, 2026

Copy link
Copy Markdown
Contributor

Ticket:

This PR does the following:

Note, this is branched off #497, I'll wait for that to go in before publishing this PR.

Adds two events for grid view to ga4:

  1. A simple event when a user explicitly selects a search view option (ie, clicking list or grid)
  2. General search analytics (including the view type) on searches

Open questions

For the search analytics, I implemented this by standardizing the search analytics data in the search manager object, and then using that data as dependencies for an effect (so that if the search params change, a new event is sent).

  1. Is the search manager a good place for that method? Seemed like a simple spot to put it, and I kept the search manager decoupled from the ga4 module
  2. Should the different useEffect calls be refactored into their own hook?
  3. Is there a better way to add the dependency than JSON encoding the analytics data? I found that if I just passed the data in directly, because how js object equality works, we'd end up firing duplicate events. If I passed each field separately as a dep, the array filterNames would still suffer the same issue.

How has this been tested? How should a reviewer test this?

Tested on vercel by inspecting window.dataLayer in the console.

I'll add a couple unit tests as well.

Accessibility concerns or updates

Checklist:

  • I have added relevant accessibility documentation for this pull request.
  • All new and existing tests passed.
  • I have updated the CHANGELOG.md.

@vercel

vercel Bot commented Jan 26, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
digital-collections Ready Ready Preview, Comment Feb 11, 2026 7:58pm

Request Review

Comment thread app/src/utils/ga4Utils.ts Outdated
Comment thread app/src/utils/searchManager/searchManager.tsx Outdated
Comment thread app/src/utils/searchManager/searchManager.tsx Outdated
@avertrees

Copy link
Copy Markdown
Collaborator

mmm I'm not seeing the data in window.dataLayer change when I change click the List button

@avertrees

Copy link
Copy Markdown
Collaborator

mmm I'm not seeing the data in window.dataLayer change when I change click the List button

nvm I was looking at the wrong location

Comment thread app/src/hooks/useSearchAnalytics.ts Outdated
@EdwinGuzman

Copy link
Copy Markdown
Member

Reviewed this with @isastettler because she's also working on GA4 event tracking in the Scout app.

Is the search manager a good place for that method? Seemed like a simple spot to put it, and I kept the search manager decoupled from the ga4 module
Should the different useEffect calls be refactored into their own hook?

Looks like this was already made and it makes sense.

Is there a better way to add the dependency than JSON encoding the analytics data? I found that if I just passed the data in directly, because how js object equality works, we'd end up firing duplicate events. If I passed each field separately as a dep, the array filterNames would still suffer the same issue.

This may not be the most ideal way to track object updates, but it works well and is a common pattern. It's doing string comparison over the shallow object diff. I'm surprised that filterNames had the same issue (as individual values in the dep array) since that's an array of strings based on a new instance.

We verified the data is set in window.dataLayer. Just don't want to approve on a repo we don't work on, but code looks good!

Send an event when a user explicitly clicks on the list or grid view
button selection.
Add a getter to the search manager that collates analytics data on
searches, then pass that search data to ga4 in a useEffect in the
various search interfaces.
We are eventually going to pass title cased params to ga4, but since the
code is generally using lowercase, let's pass in lowercase
Move this into the ga4 utils and make search type possibly undefined to
clarify the contract.
Define a hook to extract the search analytics code, and remove the
Analytics stuff from the SearchManager to keep it cleaner
if the viewmode is the only thing that changed, we don't want to send
this event.
JS / TS arrays are truthy even when empty - so check the array length.
As the code was written, we were sending up "" when there were no
filters selected.

@7emansell 7emansell left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree that the useSearchAnalytics makes more sense as a hook and shouldn't be on the searchManager, and seeing expected event data in the console 👍

Base automatically changed from grid-view to qa February 12, 2026 17:53
@sarangj sarangj merged commit 2bb8c3e into qa Feb 12, 2026
3 checks passed
@sarangj sarangj deleted the final_gridalytics branch February 12, 2026 18:18
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.

5 participants