From 42e2e5efbcd2e993761664e6fe0f5f0a57f4618e Mon Sep 17 00:00:00 2001
From: Ed Sanders <esanders@wikimedia.org>
Date: Thu, 23 Jul 2020 15:23:29 +0100
Subject: [PATCH] Make NotificationItemWidget an <a> tag to fix focus issues

Bug: T258705
Change-Id: Ib44e04dd08e2818eb99ffb0cba73c775256d66d5
---
 .../mw.echo.ui.NotificationItemWidget.less    | 16 ++++++++++++++++
 .../ui/mw.echo.ui.NotificationItemWidget.js   | 19 ++++++-------------
 2 files changed, 22 insertions(+), 13 deletions(-)

diff --git a/modules/styles/mw.echo.ui.NotificationItemWidget.less b/modules/styles/mw.echo.ui.NotificationItemWidget.less
index b5f1838e2..01baa9118 100644
--- a/modules/styles/mw.echo.ui.NotificationItemWidget.less
+++ b/modules/styles/mw.echo.ui.NotificationItemWidget.less
@@ -3,6 +3,7 @@
 @import '../echo.mixins.less';
 
 .mw-echo-ui-notificationItemWidget {
+	display: block;
 	background-color: @notification-item-background-read;
 	position: relative;
 	white-space: normal;
@@ -11,10 +12,25 @@
 	border: 1px solid #c8ccd1;
 	border-bottom: 0;
 
+	&:not( [ href ] ) {
+		// Items without a primary URL are not clickable
+		cursor: default;
+	}
+
+	label {
+		// Reset browser default of cursor:default;
+		cursor: inherit;
+	}
+
 	&:hover {
 		background-color: #ececec;
 	}
 
+	&:focus {
+		box-shadow: inset 0 0 0 2px @color-primary;
+		outline: 0;
+	}
+
 	&:last-child {
 		border-bottom: 1px solid #c8ccd1;
 	}
diff --git a/modules/ui/mw.echo.ui.NotificationItemWidget.js b/modules/ui/mw.echo.ui.NotificationItemWidget.js
index a31f3d46c..30b6dde60 100644
--- a/modules/ui/mw.echo.ui.NotificationItemWidget.js
+++ b/modules/ui/mw.echo.ui.NotificationItemWidget.js
@@ -200,25 +200,18 @@
 			.toggleClass( 'mw-echo-ui-notificationItemWidget-initiallyUnseen', !this.model.isSeen() && !this.bundle )
 			.toggleClass( 'mw-echo-ui-notificationItemWidget-bundled', this.bundle );
 
-		// Wrap the entire item with primary url
 		if ( this.model.getPrimaryUrl() ) {
-			this.$element.contents()
-				.wrapAll(
-					// HACK: Wrap the entire item with a link that takes
-					// the user to the primary url. This is not perfect,
-					// but it makes the behavior native to the browser rather
-					// than us listening to click events and opening new
-					// windows.
-					$( '<a>' )
-						.addClass( 'mw-echo-ui-notificationItemWidget-linkWrapper' )
-						.attr( 'href', this.model.getPrimaryUrl() )
-						.on( 'click', this.onPrimaryLinkClick.bind( this ) )
-				);
+			this.$element
+				.attr( 'href', this.model.getPrimaryUrl() )
+				.on( 'click', this.onPrimaryLinkClick.bind( this ) );
 		}
 	};
 
 	OO.inheritClass( mw.echo.ui.NotificationItemWidget, OO.ui.Widget );
 
+	// Make the whole item a link to get native link behaviour
+	mw.echo.ui.NotificationItemWidget.static.tagName = 'a';
+
 	/**
 	 * Respond to primary link click.
 	 * Override this in the descendents.