site-kit-wp icon indicating copy to clipboard operation
site-kit-wp copied to clipboard

Published 20 hours ago • verified publisher icon google

Integrate `<ConversionTrackingToggle />` Component Into Ads and GA4 `<SettingsEdit />` Components

Open 10upsimon opened this issue 1 year ago • 4 comments

Feature Description

With the Conversion Tracking UI toggle component and associated settings/datastore now complete (see #8616), the toggle is ready to be integrated into both the settings edit area of the Ads module, as well as the GA4 module.

This issue focusses on the edit view of the settings area of said modules, i.e the <SettingsEdit /> component of each module.

The enabled/disabled status for Conversion Tracking should be displayed in UI toggle form based on the state of the toggle (i.e if conversion tracking enabled (true) or disabled (false) via the applicable datastore selector). Toggling the element on or off will result in the applicable Conversion Tracking setting being updated, and UI reflected as such.

Please reference the "Conversion Event Tracking Toggle & Settings" area of the design doc for more detailed information.

Do not alter or remove anything below. The following sections will be managed by moderators only.

Acceptance criteria

  • ConversionTrackingToggle component implemented in 8616 should be added to the SettingsEdit component of Ads and Analytics modules

Implementation Brief

  • [ ] Update assets/js/modules/ads/components/settings/SettingsForm.js:
    • Include ConversionTrackingToggle component
    • It should be placed as on figma design
    • If needed styling can be applied in assets/sass/components/ads/_googlesitekit-ads-settings.scss
  • [ ] Update assets/js/modules/analytics-4/components/settings/SettingsForm.js
    • Do the same as for Ads module settings form
    • Follow the figma design

Test Coverage

  • Update failing VRT

QA Brief

Changelog entry

10upsimon avatar Apr 25 '24 09:04 10upsimon

AC ✅

Moving to IB

10upsimon avatar May 06 '24 11:05 10upsimon

  • Toggle will handle saving the settings to the CORE_SITE store independently, so the focus of this issue is placing the rendered component in right place as per design and adjusting the wrapper spacing . Styling can be applied in assets/sass/components/ads/_googlesitekit-ads-settings.scss

@zutigrm this line is not correct anymore. We don't need to mention anything about saving because there are no action items for it in this task, it will be handled in another one. Let's remove it to not confuse anyone who will work on it.

eugene-manuilov avatar May 08 '24 16:05 eugene-manuilov

@eugene-manuilov Thanks, IB updated

zutigrm avatar May 08 '24 16:05 zutigrm

Thanks, @zutigrm. IB :heavy_check_mark:

eugene-manuilov avatar May 09 '24 11:05 eugene-manuilov

QA Update ❌

  • Tested on dev environment.
  • Verified Ads module edit settings view for non PAX manner and PAX manner.
  • Verified that toggling the value and saving persists the value on the next page load/reload.

@10upsimon

Issue

If the user does not click the "Save" button after changing the toggle value, the toggle value is still getting updated under the settings view. Upon reloading the page, the toggle value reverts to its original status.

Expected Behavior: If the user does not click the "Save" button, the toggle value should not change under the edit view.

Steps to Reproduce:

  1. Disable/Enable the toggle button.
  2. Click on the "Cancel" button.
  3. Notice that the toggle value updates as per the selection in step 1.
  4. Reload the page.
  5. Observe that the toggle value reverts to the value before step 1 because the "Save" button was not clicked.

https://github.com/google/site-kit-wp/assets/94359491/796c551f-4e6e-4171-be2c-4618dd4d2c59

Pass Cases

Ads module Non Pax manner

image

Ads module Pax Manner

image

https://github.com/google/site-kit-wp/assets/94359491/f7c56803-0946-402f-b1c5-03c00270e16c

mohitwp avatar Jun 05 '24 13:06 mohitwp

@mohitwp This has been fixed in #8821 , you can confirm the fix there once it lands in QA

zutigrm avatar Jun 11 '24 09:06 zutigrm

QA Update

Blocked due to #8821.

mohitwp avatar Jun 13 '24 12:06 mohitwp

QA Update ✅

  • Tested on dev environment.
  • Verified Ads module edit settings view for non PAX manner and PAX manner.
  • Verified that toggling the value and saving persists the value on the next page load/reload.
  • Issue reported above is resolve under https://github.com/google/site-kit-wp/issues/8821.

Ads module Non Pax manner

image

Ads module Pax Manner

image

https://github.com/google/site-kit-wp/assets/94359491/f7c56803-0946-402f-b1c5-03c00270e16c

mohitwp avatar Jun 14 '24 06:06 mohitwp