Connect Analytics CTA widget shifts up when user clicks on 'Connect CTA' button.

mohitwp commented 1 year ago

Bug Description

Steps to reproduce

Pick or tailor KMW metrics.
Disconnect Analytics.
See Connect GA CTA on dashboard.
Either "Scroll the page" further down OR reload the page with a Dashboard notification by appending the following link in the URL to your site: /wp-admin/admin.php?page=googlesitekit-dashboard&notification=authentication_success&slug=adsense
Click on the Connect Analytics button within the Connect GA Key Metric CTA tile.
Observe that the page jumps to the beginning of the Key Metrics widget or the to the top of the page if there is a notification before it then redirects to the Analytics Setup page.

Screenshots

https://github.com/google/site-kit-wp/assets/94359491/9542c6a6-78c9-45ea-ade9-73cd2178889f

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

Acceptance criteria

Clicking on the "Connect Analytics" link within the Key Metrics tile CTA should not cause movement on the page before loading the Analytics settings setup page.
The link should behave exactly like the "Complete Setup" CTA button in the GA4 All Traffic dashboard widget when GA4 is disconnected.

Implementation Brief

In assets/js/components/KeyMetrics/ConnectModuleCTATile.js:

Locate the Link component used for the "Connect %s" (Connect Analytics) link. This component should have an href prop with a value of #, which causes the page to jump to the top upon click.
Remove the href prop with the value of #. This will prevent the default behavior of jumping to the top of the page.

Test Coverage

No new tests are required for this change.

QA Brief

Follow the steps to reproduce, and verify that clicking the CTA does not cause the page to scroll as per the AC.

Changelog entry

Improve behavior of Key Metrics "Connect Analytics" CTA.

jimmymadon commented 1 year ago

@mohitwp I have retested this on the latest develop branch but cannot reproduce this.

mohitwp commented 1 year ago

@jimmymadon I'm not able to reproduce this issue on latest develop branch. Closing this issue now.

https://github.com/google/site-kit-wp/assets/94359491/ea478bee-2359-44e4-bec4-2f3bed88f710

mohitwp commented 1 year ago

@jimmymadon

I'm able to reproduce this issue on dev. It is happening in two cases-

1) If any dashboard notification is appearing on main dashboard. 2) If a portion of KMW banner is hidden then on clicking on 'Connect Analytics' button banner position will shift to display the whole banner.

See attached screencast and let me know if you require a huddle.

https://github.com/google/site-kit-wp/assets/94359491/9542c6a6-78c9-45ea-ade9-73cd2178889f

techanvil commented 1 year ago

AC :white_check_mark:

tofumatt commented 1 year ago

IB ✅

mohitwp commented 1 year ago

QA Update ✅

Tested on dev environment.
Verified that on on the "Connect Analytics" link within the Key Metrics tile CTA should not cause movement on the page before loading the Analytics settings setup page.
Verified that behave exactly like the "Complete Setup" CTA button in the GA4 All Traffic dashboard widget when GA4 is disconnected.

https://github.com/google/site-kit-wp/assets/94359491/2cab5e3a-ff25-421a-81a1-9851c69181b9

google / site-kit-wp