Enhancement/11578 AdSense Alert CTA should open in a new tab

assets/js/components/Banner/CTAButton.js

@@ -23,6 +23,7 @@ import PropTypes from 'prop-types';
  * Internal dependencies
  */
 import { SpinnerButton } from 'googlesitekit-components';
+import ExternalIcon from '@/svg/icons/external.svg';
 
 export default function CTAButton( {
 	label,
@@ -31,11 +32,18 @@ export default function CTAButton( {
 	inProgress,
 	onClick,
 	href,
+	external = false,
+	hideExternalIndicator = false,
 } ) {
 	if ( ! label || ( ! onClick && ! href ) ) {
 		return null;
 	}
 
+	let trailingIconToUse;
+	if ( external && ! hideExternalIndicator ) {
+		trailingIconToUse = <ExternalIcon width={ 14 } height={ 14 } />;
+	}
+
 	return (
 		<SpinnerButton
 			className="googlesitekit-banner__cta"
@@ -44,6 +52,8 @@ export default function CTAButton( {
 			isSaving={ inProgress }
 			onClick={ onClick }
 			href={ href }
+			target={ external ? '_blank' : undefined }
+			trailingIcon={ trailingIconToUse }
 		>
 			{ label }
 		</SpinnerButton>

assets/js/components/Banner/index.stories.js

@@ -83,7 +83,7 @@ function Template() {
 									an ad blocker the option to allow ads on
 									your site. Site Kit will place an ad
 									blocking recovery tag on your site.{ ' ' }
-									<Link>Learn more</Link>
+									<Link external>Learn more</Link>
 								</p>
 								<p>
 									Publishers see up to 1 in 5 users choose to
@@ -225,6 +225,7 @@ function Template() {
 							ctaButton={ {
 								label: 'Go to AdSense',
 								onClick: () => {},
+								external: true,
 							} }
 							dismissButton={ {
 								label: 'Maybe later',

assets/js/components/Notice/CTAButton.js

@@ -31,8 +31,8 @@ export default function CTAButton( {
 	inProgress,
 	onClick,
 	href,
-	external,
-	hideExternalIndicator,
+	external = false,
+	hideExternalIndicator = false,
 } ) {
 	let trailingIconToUse;
 	if ( external && ! hideExternalIndicator ) {
@@ -45,7 +45,7 @@ export default function CTAButton( {
 			isSaving={ inProgress }
 			onClick={ onClick }
 			href={ href }
-			target={ external ? '_blank' : '_self' }
+			target={ external ? '_blank' : undefined }
 			trailingIcon={ trailingIconToUse }
 		>
 			{ label }

assets/js/components/NotificationFromServer.js

@@ -104,9 +104,17 @@ function NotificationFromServer( {
 			ctaButton={ {
 				label: ctaLabel,
 				href: ctaURL,
-				target: ctaTarget,
 				onClick: onCTAClick,
 				dismissOptions,
+				// In the case of notifications, other `target="_blank"` CTA URLs
+				// are external, and adding a new, more explicit/clear `external`
+				// prop would be a refactor of the service, JS, and PHP code all
+				// for mildly more "intuitive" prop names/API.
+				//
+				// In this case it's not worth the refactor (as of writing this
+				// logic), so we allow for any "open in a new window" URLs
+				// to also be "external".
+				external: ctaTarget === '_blank',
 			} }
 			dismissButton={
 				dismissible

assets/sass/components/banner/_googlesitekit-banner.scss

@@ -131,6 +131,7 @@
 
 	// Setting the margin top on each button so that when they stack on mobile the spacing is consistent.
 	.googlesitekit-notice__action > .mdc-button {
+		gap: 0 6px;
 		margin-right: 10px;
 		margin-top: 20px;
 	}