Merge pull request #11077 from nextcloud/stable14-11064-fix-markup-an…

…d-style-of-mentions-in-comments [stable14] Fix markup and style of mentions in comments
nextcloud · Sep 20, 2018 · 0c85cb1 · 0c85cb1
2 parents a0f2bd8 + 51b978d
commit 0c85cb1
Show file tree

Hide file tree

Showing 3 changed files with 55 additions and 32 deletions.
diff --git a/apps/comments/css/comments.scss b/apps/comments/css/comments.scss
@@ -133,7 +133,7 @@
 	 adding this brings them closer to the element**/
 	margin-top: 5px;
 }
-#commentsTabView .comments li .message .avatar-name-wrapper,
+
 .atwho-view-ul * .avatar-name-wrapper,
 #commentsTabView .comment .authorRow {
 	position: relative;
@@ -144,24 +144,34 @@
 
 #commentsTabView .comment:not(.newCommentRow) .message .avatar-name-wrapper:not(.currentUser),
 #commentsTabView .comment:not(.newCommentRow) .message .avatar-name-wrapper:not(.currentUser) .avatar,
+#commentsTabView .comment:not(.newCommentRow) .message .avatar-name-wrapper:not(.currentUser) .avatar img,
 #commentsTabView .comment .authorRow .avatar:not(.currentUser),
 #commentsTabView .comment .authorRow .author:not(.currentUser) {
 	cursor: pointer;
 }
 
 .atwho-view-ul .avatar-name-wrapper,
-.atwho-view-ul .avatar-name-wrapper .avatar {
+.atwho-view-ul .avatar-name-wrapper .avatar,
+.atwho-view-ul .avatar-name-wrapper .avatar img {
 	cursor: pointer;
 }
 
 #commentsTabView .comments li .message .atwho-inserted,
 #commentsTabView .newCommentForm .atwho-inserted {
 	.avatar-name-wrapper {
+		/* Make the wrapper the positioning context of its child contacts
+		 * menu. */
+		position: relative;
+
 		display: inline;
 		vertical-align: top;
 		background-color: var(--color-background-dark);
 		border-radius: 50vh;
 		padding: 1px 7px 1px 1px;
+
+		/* Ensure that the avatar and the user name will be kept together. */
+		white-space: nowrap;
+
 		.avatar {
 			img {
 				vertical-align: top;
@@ -174,6 +184,15 @@
 			margin-left: 0;
 			margin-right: 2px;
 		}
+		strong {
+			/* Ensure that the user name is shown in bold, as different browsers
+			 * use different font weights for strong elements. */
+			font-weight: bold;
+		}
+	}
+	.avatar-name-wrapper.currentUser {
+		background-color: var(--color-primary);
+		color: var(--color-primary-text);
 	}
 }
 
@@ -234,4 +253,4 @@
 
 .app-files .action-comment {
 	padding: 16px 14px;
-}
+}
diff --git a/apps/comments/js/commentstabview.js b/apps/comments/js/commentstabview.js
@@ -196,24 +196,26 @@
 					sorter: function (q, items) { return items; }
 				},
 				displayTpl: function (item) {
-					return '<li>'
-						+ '<span class="avatar-name-wrapper">'
-						+ '<div class="avatar" '
-						+ ' data-username="' + escapeHTML(item.id) + '"'	// for avatars
-						+ ' data-user="' + escapeHTML(item.id) + '"'		// for contactsmenu
-						+ ' data-user-display-name="' + escapeHTML(item.label) + '"></div>'
-						+ ' <strong>' + escapeHTML(item.label) + '</strong>'
-						+ '</span></li>';
+					return '<li>' +
+						'<span class="avatar-name-wrapper">' +
+							'<span class="avatar" ' +
+									'data-username="' + escapeHTML(item.id) + '" ' + // for avatars
+									'data-user="' + escapeHTML(item.id) + '" ' + // for contactsmenu
+									'data-user-display-name="' + escapeHTML(item.label) + '">' +
+							'</span>' +
+							'<strong>' + escapeHTML(item.label) + '</strong>' +
+						'</span></li>';
 				},
 				insertTpl: function (item) {
-					return ''
-						+ '<span class="avatar-name-wrapper">'
-						+ '<div class="avatar" '
-						+ ' data-username="' + escapeHTML(item.id) + '"'	// for avatars
-						+ ' data-user="' + escapeHTML(item.id) + '"'		// for contactsmenu
-						+ ' data-user-display-name="' + escapeHTML(item.label) + '"></div>'
-						+ ' <strong>' + escapeHTML(item.label) + '</strong>'
-						+ '</span>';
+					return '' +
+						'<span class="avatar-name-wrapper">' +
+							'<span class="avatar" ' +
+									'data-username="' + escapeHTML(item.id) + '" ' + // for avatars
+									'data-user="' + escapeHTML(item.id) + '" ' + // for contactsmenu
+									'data-user-display-name="' + escapeHTML(item.label) + '">' +
+							'</span>' +
+							'<strong>' + escapeHTML(item.label) + '</strong>' +
+						'</span>';
 				},
 				searchKey: "label"
 			});
@@ -224,7 +226,7 @@
 					// passing the whole comments form would re-apply and request
 					// avatars from the server
 					$(je.target).find(
-						'div[data-username="' + $el.find('[data-username]').data('username') + '"]'
+						'span[data-username="' + $el.find('[data-username]').data('username') + '"]'
 					).parent(),
 					editionMode
 				);
@@ -486,20 +488,22 @@
 		},
 
 		_composeHTMLMention: function(uid, displayName) {
-			var avatar = '<div class="avatar" '
-				+ 'data-username="' + _.escape(uid) + '"'
-				+ ' data-user="' + _.escape(uid) + '"'
-				+ ' data-user-display-name="'
-				+ _.escape(displayName) + '"></div>';
+			var avatar = '' +
+				'<span class="avatar" ' +
+						'data-username="' + _.escape(uid) + '" ' +
+						'data-user="' + _.escape(uid) + '" ' +
+						'data-user-display-name="' + _.escape(displayName) + '">' +
+				'</span>';
 
 			var isCurrentUser = (uid === OC.getCurrentUser().uid);
 
-			return ''
-				+ '<span class="atwho-inserted" contenteditable="false">'
-				+ '<span class="avatar-name-wrapper' + (isCurrentUser ? ' currentUser' : '') + '">'
-				+ avatar + ' <strong>'+ _.escape(displayName)+'</strong>'
-				+ '</span>'
-				+ '</span>';
+			return '' +
+				'<span class="atwho-inserted" contenteditable="false">' +
+					'<span class="avatar-name-wrapper' + (isCurrentUser ? ' currentUser' : '') + '">' +
+						avatar +
+						'<strong>' + _.escape(displayName) + '</strong>' +
+					'</span>' +
+				'</span>';
 		},
 
 		nextPage: function() {

diff --git a/apps/comments/tests/js/commentstabviewSpec.js b/apps/comments/tests/js/commentstabviewSpec.js
@@ -309,7 +309,7 @@ describe('OCA.Comments.CommentsTabView tests', function() {
 
 			expect(createStub.calledOnce).toEqual(false);
 			expect($newCommentForm.find('.message').html()).toContain('Mention to <span');
-			expect($newCommentForm.find('.message').html()).toContain('<div class="avatar"');
+			expect($newCommentForm.find('.message').html()).toContain('<span class="avatar"');
 			expect($newCommentForm.find('.message').html()).toContain('<strong>User Name</strong>');
 			expect($newCommentForm.find('.message').text()).not.toContain('@');
 			// In this case the default behaviour is prevented by the