jenkins-bot has submitted this change and it was merged.
Change subject: Make items on notifications flyout behave like links
......................................................................
Make items on notifications flyout behave like links
We want the notifications in the flyout to behave just like links,
including standard middle-click and Ctrl-click behavior. The simplest
way to do that would be to actually make them links - but the area can
contain a few other links, so we can't do that and have to resort to
ugly hacks.
Or do we?
Turns out that while browsers won't accept HTML containing nested <a>
tags[1], such a structure is valid XHTML, and it's possible to create
such structure in HTML mode using DOM manipulation. It works like one
would expect: the entire thing is clickable, but inner <a> tags' hrefs
override outer ones.
Firefox even had a request to make that work[2] which was happily
fulfilled.
Tested the basic case [see below] on Firefox 22, Opera 12, Opera 15
(which uses the Blink engine like Chrome), IE 8 and IE 6 and it works
the same on all of them. Tested the XHTML variant [see below] on all
of the above except for the IEs which don't grok XHTML and it exhibits
the same behavior.
[1] Simple test: $('<div>1<a>2<a>3</a>4</a>5</div>').html() is
"1<a>2</a><a>3</a>45", not actually "1<a>2<a>3</a>4</a>5" like one
might expect.
[2] https://bugzilla.mozilla.org/show_bug.cgi?id=331959
----
The test cases used are below. When trying out the XHTML one make sure
that the browser uses application/xhtml+xml MIME type; saving the file
with .xhtml extension should be enough.
XHTML:
<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Strict//EN"
"http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<body>
<div>1<a href="http://google.com/">2<a
href="http://example.com/">3</a>4</a>5</div>
</body>
</html>
HTML:
<!DOCTYPE html>
<html>
<body>
<script type="text/javascript">
var div = document.createElement('div');
var a1 = document.createElement('a');
a1.href = "http://google.com/";
var a2 = document.createElement('a');
a2.href = "http://example.com/";
div.appendChild( document.createTextNode('1') );
div.appendChild( a1 );
a1.appendChild( document.createTextNode('2') );
a1.appendChild( a2 );
a2.appendChild( document.createTextNode('3') );
a1.appendChild( document.createTextNode('4') );
div.appendChild( document.createTextNode('5') );
document.body.appendChild(div);
</script>
</body>
</html>
----
Bug: 52319
Change-Id: I311eca70f025ce92129c828cd88f96686b7cff72
---
M modules/overlay/ext.echo.overlay.css
M modules/overlay/ext.echo.overlay.js
2 files changed, 30 insertions(+), 15 deletions(-)
Approvals:
Bsitu: Looks good to me, but someone else must approve
Kaldari: Looks good to me, approved
EBernhardson (WMF): Looks good to me, but someone else must approve
jenkins-bot: Verified
diff --git a/modules/overlay/ext.echo.overlay.css
b/modules/overlay/ext.echo.overlay.css
index bd51cc4..83b32b8 100644
--- a/modules/overlay/ext.echo.overlay.css
+++ b/modules/overlay/ext.echo.overlay.css
@@ -29,26 +29,33 @@
overflow: auto;
}
#p-personal .mw-echo-overlay li.mw-echo-notification {
- background-color: #F1F1F1;
display: block;
float: none;
+ margin: 0;
+ padding: 0;
+}
+#p-personal .mw-echo-overlay li.mw-echo-notification
.mw-echo-notification-wrapper {
+ display: block;
+ background-color: #F1F1F1;
border-bottom: 1px solid #DDDDDD;
padding: 15px 40px 10px 10px;
- margin: 0;
white-space: normal;
font-size: 13px;
line-height: 16px;
+ /* Suppress standard links styles */
+ color: inherit;
+ text-decoration: inherit;
}
-#p-personal .mw-echo-overlay li.mw-echo-notification:hover {
+#p-personal .mw-echo-overlay li.mw-echo-notification:hover
.mw-echo-notification-wrapper {
background-color: #F9F9F9;
}
-#p-personal .mw-echo-overlay li.mw-echo-notification.mw-echo-unread {
+#p-personal .mw-echo-overlay li.mw-echo-notification.mw-echo-unread
.mw-echo-notification-wrapper {
background-color: white;
}
-#p-personal .mw-echo-overlay li.mw-echo-notification.mw-echo-unread:hover {
+#p-personal .mw-echo-overlay li.mw-echo-notification.mw-echo-unread:hover
.mw-echo-notification-wrapper {
background-color: #F9F9F9;
}
-#p-personal .mw-echo-overlay li.mw-echo-notification:last-child {
+#p-personal .mw-echo-overlay li.mw-echo-notification:last-child
.mw-echo-notification-wrapper {
border-bottom: none;
}
#p-personal .mw-echo-title a {
diff --git a/modules/overlay/ext.echo.overlay.js
b/modules/overlay/ext.echo.overlay.js
index f03f5ca..508a426 100644
--- a/modules/overlay/ext.echo.overlay.js
+++ b/modules/overlay/ext.echo.overlay.js
@@ -56,7 +56,8 @@
}
$ul.css( 'max-height', notificationLimit * 95 +
'px' );
$.each( notifications.index, function ( index,
id ) {
- var data = notifications.list[id],
+ var $wrapper,
+ data = notifications.list[id],
$li = $( '<li></li>' )
.data( 'details', data )
.data( 'id', id )
@@ -81,17 +82,24 @@
}
);
// If there is a primary link, make the
entire notification clickable.
+ // Yes, it is possible to nest <a> tags
via DOM manipulation,
+ // and it works like one would expect.
if ( $li.find(
'.mw-echo-notification-primary-link' ).length ) {
- $li.css( 'cursor', 'pointer' );
- $li.click( function() {
- if (
mw.echo.clickThroughEnabled ) {
- // Log the
clickthrough
-
mw.echo.logInteraction( 'notification-link-click', 'flyout', +data.id,
data.type );
- }
- window.location.href =
$li.find( '.mw-echo-notification-primary-link' ).attr( 'href' );
- } );
+ $wrapper = $( '<a>' )
+ .addClass(
'mw-echo-notification-wrapper' )
+ .attr( 'href',
$li.find( '.mw-echo-notification-primary-link' ).attr( 'href' ) )
+ .click( function() {
+ if (
mw.echo.clickThroughEnabled ) {
+ // Log
the clickthrough
+
mw.echo.logInteraction( 'notification-link-click', 'flyout', +data.id,
data.type );
+ }
+ } );
+ } else {
+ $wrapper = $('<div>').addClass(
'mw-echo-notification-wrapper' );
}
+ $li.wrapInner( $wrapper );
+
mw.echo.setupNotificationLogging( $li,
'flyout' );
if ( !data.read ) {
--
To view, visit https://gerrit.wikimedia.org/r/77824
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I311eca70f025ce92129c828cd88f96686b7cff72
Gerrit-PatchSet: 5
Gerrit-Project: mediawiki/extensions/Echo
Gerrit-Branch: master
Gerrit-Owner: Matmarex <[email protected]>
Gerrit-Reviewer: Alex Monk <[email protected]>
Gerrit-Reviewer: Anomie <[email protected]>
Gerrit-Reviewer: Bsitu <[email protected]>
Gerrit-Reviewer: Cscott <[email protected]>
Gerrit-Reviewer: EBernhardson (WMF) <[email protected]>
Gerrit-Reviewer: Kaldari <[email protected]>
Gerrit-Reviewer: Krinkle <[email protected]>
Gerrit-Reviewer: Matmarex <[email protected]>
Gerrit-Reviewer: jenkins-bot
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits