department-of-veterans-affairs / va.gov-cms

Editor-centered management for Veteran-centered content.
https://prod.cms.va.gov
GNU General Public License v2.0
96 stars 69 forks source link

[VS Component Analytics] [DEFECT] The va-search-input component seemingly does not make use of disable-analytics prop #18846

Closed FranECross closed 1 week ago

FranECross commented 1 month ago

Description

The Sitewide team is working to remove any duplicate Google Analytics events from our products as we pay per event. We have a ticket here to resolve this duplicate event issue for Find Forms specifically but we can't complete it until this issue is fixed.

Issue Blocking our ticket noted above

The va-search-input component seemingly does not make use of disable-analytics prop. When it is added, the basic analytics event for this component still fires: <img width="943"

src="https://api.zenhub.com/attachedFiles/eyJfcmFpbHMiOnsibWVzc2FnZSI6IkJBaHBBN0l5Qmc9PSIsImV4cCI6bnVsbCwicHVyIjoiYmxvYl9pZCJ9fQ==--d266deb7653a29e6b7f48152b69f1378ed5f99ad/image.png" alt="image.png" />

You can see this when using the search bar on https://www.va.gov/find-forms, typing a search term, and searching (I use the Chrome extension Adswerve to see the events).

What I expected to happen

I expected the va-search-input component to allow disabling the analytics. I looked in the source code for this component: https://github.com/department-of-veterans-affairs/component-library/blob/main/packages/web-components/src/components/va-search-input/va-search-input.tsx and the disableAnalytics prop is not present as it is for va-link.

Reproducing

You can add disable-analytics to the search input in the Find Forms app here: https://github.com/department-of-veterans-affairs/vets-website/blob/b364d52879588928a756ebfcd7ebe7eaf7788aef/src/applications/find-forms/containers/SearchForm.jsx#L97 and run the code locally using yarn watch --env api=https://staging-api.va.gov. Note that the disable-analytics="true" prop shows up in the DOM but the int-search-input-click still fires when the search happens.

Engineering notes / background

Analytics considerations

Quality / testing notes

Acceptance criteria

Consider

  • Design / Accessibility reviews
  • Collab cycle requirements
  • Device sizes (mobile first)
  • Documentation updates / Change management < - Content model documentation
  • Testing notes
randimays commented 1 month ago

Pointed at a 2; implementation seems simple but I'm not sure of the code / merge / deploy process on DST; this allows for some ambiguity.

randimays commented 1 month ago

Pulling in as stretch for sprint 9

randimays commented 1 month ago

@FranECross Our portion of the work for this ticket is complete; the PR has been merged. The code will go out with the next DST release next week. Perhaps we should leave this in prod verification until the release is ready?

FranECross commented 1 month ago

@FranECross Our portion of the work for this ticket is complete; the PR has been merged. The code will go out with the next DST release next week. Perhaps we should leave this in prod verification until the release is ready?

Sounds great, thanks!

randimays commented 3 weeks ago

component-library release was made on Friday. We're waiting on this PR in vets-website to be merged and deployed to production. Once that is complete, we'll need to pull the new code and test locally; we don't have any va-search-input components in production with a disable-analytics prop on them yet.

randimays commented 3 weeks ago

EOD update: this PR is still waiting to be merged. Once it has been merged into vets-website, the fix can be validated locally:

In vets-website:

If the steps above work as expected, we can close this ticket 👍🏻

randimays commented 1 week ago

This is the analytics event that is fired on a va-search-input that does not have analytics disabled (screenshot from prod).

Screenshot 2024-09-03 at 4 06 01 PM

I disabled analytics on the Find Forms search input locally and did not see this event fire:

Screenshot 2024-09-03 at 4 06 07 PM

Our code changes worked as expected. Closing as complete!