Closed aaemnnosttv closed 1 year ago
@nfmohit IB sounds like an elegant fix to me 👍 My only concern is that it might become un-disabled immediately and then replaced by the progress bar 🤔 Let's try it and see!
IB ✅
@wpdarren @mohitwp this should fix the issue flagged here https://github.com/google/site-kit-wp/issues/6774#issuecomment-1498805535
Verified:
"Connect Analytics 4"
CTA.@mohitwp it would be good to get a second pair of eyes on this, would you be able to look over it since you discovered the initial issue while release testing. I have assigned you, but just send it to approval if you think it looks good.
Bug Description
In #6330 we decoupled the web data stream selection from the property select which used to handle both. In doing so, it also fixed a minor bug where the disabled property select (when GA4 wasn't enabled yet) so that it wouldn't load any options until enabled. This had a side effect of breaking the tooltip that we show when clicking the CTA to "Connect Analytics 4" as this enables GA4 at the same time. The tooltip sees its target is present (the select) but it is immediately removed due to the loading state (it's replaced by the progress bar). This causes the mutation observer to error because the element is no longer in the DOM.
Steps to reproduce
Screenshots
The moment the error is raised
Do not alter or remove anything below. The following sections will be managed by moderators only.
Acceptance criteria
Implementation Brief
assets/js/modules/analytics/components/settings/GA4SettingsControls.js
:<JoyrideTooltip />
component used to renderSet up your Google Analytics 4 property here
.target
prop of the above component to be.googlesitekit-analytics-4__select-property:not( .mdc-select--disabled )
so that it doesn't attempt to render the tooltip when the<PropertySelect>
is disabled.Test Coverage
QA Brief
Steps to reproduce
mentioned above and ensure that the reported error doesn't occur.Changelog entry