-
Notifications
You must be signed in to change notification settings - Fork 293
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add the dropdown and toggle controls to "existing GA4 property" setup variants #5276
Comments
IB ✔️ |
Add dropdown & toggle to GA4 Activation Banner
@nfmohit I am struggling to test this ticket. Have spent quite some time trying to get the dropdown on the GA4 banner notification. No rush but could we jump on a call at some point today or tomorrow to go through the QAB. Thank you. |
QA Update:
|
QA Update:
|
Hey @wpdarren, apologies for not spotting this earlier - I guess I applied the fix in a bit of a hurry. I've realised it looks a bit glitchy when showing the "existing property" variant because it first shows the "no existing property" variant for a moment before switching. existing-property.mp4I have a proposed fix here which shows a progress bar before showing either variant: fix-existing-property.mp4fix-no-existing-property.mp4From a QA perspective does this look OK to you (maybe the progress bar should be full width?), and would you like to see this fix applied to this issue or possibly create a separate issue for it? Nb, here is a draft followup PR for the sake of reference. #5796 |
@techanvil I've just set up a test and yes, I can see what you mean. I think the progress bar is a good solution so that there's no confusion with the flicker. I personally, feel that the fix should be part of this ticket. With regard to the progress bar I feel it should be the full width since the previous notification is full width. A few other quick observations:
Would you like me to send this back to Execution and when you've completed the PR on this, I'll give it a thorough test. |
Thanks @wpdarren - good observations, we should be pre-selecting a GA4 property (or "Set up a new property") and with an option selected the dropdown should no longer have the red error outline. I'll bring this back into Execution and address these points and the progress bar width too. Cheers. |
@wpdarren this should be ready for another pass on |
QA Update:
|
@aaemnnosttv if the behaviour I detailed above is correct then this ticket can be sent to approved. I wasn't sure if the toggle should be disabled or enabled. The figma designs and stories suggest that it should be disabled. |
@wpdarren the toggle will only be disabled if the associated measurement ID for the selected property matches the existing tag. I tested this locally and it seems to work as expected. Thanks for clarifying! Moving to Approval + approved 👍 |
Feature Description
Add the dropdown and toggle controls to the following Setup Component variants:
From the Design Doc:
Acceptance criteria
Implementation Brief
assets/js/modules/analytics-4/components/dashboard/ActivationBanner/SetupBanner.js
or possibly subcomponents.analytics-4
PropertySelect
component to display the property dropdown.label
prop to specify the dropdown label text: Google Analytics 4 Propertyanalytics-4
useExistingTagEffect
hook to keep snippet status in sync.analytics-4
SetupUseSnippetSwitch
component to display the switch and informative notice.Test Coverage
QA Brief
googlesitekit_analytics-4_settings
option in WordPress. This can be done in a number of ways, for example:wp option delete googlesitekit_analytics-4_settings
.DELETE FROM wp_options WHERE option_name = 'googlesitekit_analytics-4_settings';
delete_option( 'googlesitekit_analytics-4_settings' );
in the console.Set up now
in the displayed banner.header.php
file:<head>
tag:<body>
tag starts:Modules -> Analytics 4 -> SetupBanner
:Changelog entry
The text was updated successfully, but these errors were encountered: