Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Learn More Link on Link Analytics and AdSense CTA opens in same window #4286

Closed
wpdarren opened this issue Oct 27, 2021 · 2 comments · Fixed by #4425
Closed

Learn More Link on Link Analytics and AdSense CTA opens in same window #4286

wpdarren opened this issue Oct 27, 2021 · 2 comments · Fixed by #4425
Labels
Good First Issue Good first issue for new engineers Module: Analytics Google Analytics module related issues P2 Low priority Rollover Issues which role over to the next sprint Type: Bug Something isn't working

Comments

@wpdarren
Copy link
Collaborator

wpdarren commented Oct 27, 2021 •
edited by aaemnnosttv
Loading

Bug Description

On the Link Analytics and AdSense CTA, there's a Learn more link and it currently opens the page in the same window rather than opening a new one like our other CTAs.

image.png

Steps to reproduce

  1. Connect AdSense but do not link Analytics.
  2. Go to Site Kit Dashboard and scroll down to Earnings widget.
  3. See the CTA Link Analytics and AdSense
  4. Click on the Learn more link and see behaviour.

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

Acceptance criteria

  • The Learn more link on the CTA to link AdSense and Analytics should open in a separate window/tab, like other "Learn more" links.

Implementation Brief

  • Add the ctaLinkExternal prop to the CTA component rendered in AdSenseLinkCTA.

    site-kit-wp/assets/js/modules/analytics/components/common/AdSenseLinkCTA.js

    Lines 40 to 48 in 1be7723

    <CTA
    title={ __( 'Link Analytics and AdSense', 'google-site-kit' ) }
    description={ __(
    'Get reports for your top earning pages by linking your Analytics and AdSense accounts',
    'google-site-kit'
    ) }
    ctaLink={ supportURL }
    ctaLabel={ __( 'Learn more', 'google-site-kit' ) }
    />

Test Coverage

  • No new tests needed.
  • Existing tests should pass.

QA Brief

  1. Connect AdSense but do not link Analytics.
  2. Go to Site Kit Dashboard and scroll down to Earnings widget.
  3. Click on the Learn more link and observe the link now it opens in a separate window.

Changelog entry

  • Update the "Learn More" links of Analytics and AdSense CTAs to open in a new window.
@wpdarren wpdarren added the Type: Bug Something isn't working label Oct 27, 2021
@aaemnnosttv aaemnnosttv added Module: Analytics Google Analytics module related issues P2 Low priority labels Nov 2, 2021
@felixarntz felixarntz assigned felixarntz and unassigned felixarntz Nov 11, 2021
@techanvil techanvil assigned techanvil and unassigned techanvil Nov 12, 2021
@eugene-manuilov eugene-manuilov self-assigned this Nov 15, 2021
@eugene-manuilov
Copy link
Collaborator

IB ✔️

@eugene-manuilov eugene-manuilov removed their assignment Nov 15, 2021
@eugene-manuilov eugene-manuilov added the Good First Issue Good first issue for new engineers label Nov 15, 2021
@mitogh mitogh self-assigned this Nov 29, 2021
mitogh added a commit that referenced this issue Nov 29, 2021
@mitogh
Add property ctaLinkExternal to open the link in a separate window
97cece2 
The following work sets the property `ctaLinkExternal` to `true`,
according to the details from #4286 in order to allow the link
to be opened in a new window instead of the current one.
mitogh added a commit that referenced this issue Nov 29, 2021
@mitogh
Update propTypes for the CTA component
a943336 
Register the missing property `ctaLinkExternal` in order to allow
callers to identify available properties, it sets a default value of
`false` as well in order to be backwards compatible as if the value
is not set `false` would be used instead for existing components where
`CTA` component is used.

This work was based out of the ticket #4286.
@mitogh mitogh mentioned this issue Nov 29, 2021
Merged
7 tasks
@mitogh mitogh removed their assignment Nov 29, 2021
@mitogh mitogh linked a pull request Nov 30, 2021 that will close this issue
Learn More links on the Adsense modules opens in a new window. #4425
Merged
7 tasks
@mitogh mitogh removed their assignment Nov 30, 2021
@eugene-manuilov eugene-manuilov self-assigned this Dec 1, 2021
@eugene-manuilov eugene-manuilov removed a link to a pull request Dec 1, 2021
Learn More links on the Adsense modules opens in a new window. #4425
Merged
7 tasks
@eugene-manuilov eugene-manuilov removed their assignment Dec 1, 2021
@mitogh mitogh linked a pull request Dec 1, 2021 that will close this issue
Learn More links on the Adsense modules opens in a new window. #4425
Merged
7 tasks
@FlicHollis FlicHollis added the Rollover Issues which role over to the next sprint label Dec 6, 2021
@wpdarren
Copy link
Collaborator Author

wpdarren commented Dec 8, 2021 •
edited
Loading

QA Update: ✅

Verified:

  • when you click on the Learn more link, it opens in a separate window.
  • tested this on current and unified dashboard.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Good First Issue Good first issue for new engineers Module: Analytics Google Analytics module related issues P2 Low priority Rollover Issues which role over to the next sprint Type: Bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Learn More links on the Adsense modules opens in a new window. google/site-kit-wp
8 participants
@eugene-manuilov @aaemnnosttv @felixarntz @mitogh @techanvil @wpdarren @FlicHollis @mohitwp