Multiple improvements for comment edit diff (#21990) (#22007)

Backport #21990

- Use explicit avatar size so when JS copies the HTML, the size gets
copied with it
- Replace icon font use with SVG
- Improve styling and diff rendering
- Sort lists in `svg.js`

Fixes: #21924

Co-authored-by: Lunny Xiao <[email protected]>
Co-authored-by: techknowlogick <[email protected]>

Author: 3 people

routers/web/repo/issue_content_history.go

@@ -6,16 +6,17 @@ package repo
 
 import (
 	"bytes"
-	"fmt"
 	"html"
 	"net/http"
 	"strings"
 
+	"code.gitea.io/gitea/models/avatars"
 	issues_model "code.gitea.io/gitea/models/issues"
 	"code.gitea.io/gitea/models/unit"
 	"code.gitea.io/gitea/modules/context"
 	"code.gitea.io/gitea/modules/log"
 	"code.gitea.io/gitea/modules/setting"
+	"code.gitea.io/gitea/modules/templates"
 	"code.gitea.io/gitea/modules/timeutil"
 
 	"github.com/sergi/go-diff/diffmatchpatch"
@@ -64,16 +65,20 @@ func GetContentHistoryList(ctx *context.Context) {
 		} else {
 			actionText = ctx.Locale.Tr("repo.issues.content_history.edited")
 		}
-		timeSinceText := timeutil.TimeSinceUnix(item.EditedUnix, ctx.Locale)
 
 		username := item.UserName
 		if setting.UI.DefaultShowFullName && strings.TrimSpace(item.UserFullName) != "" {
 			username = strings.TrimSpace(item.UserFullName)
 		}
 
+		src := html.EscapeString(item.UserAvatarLink)
+		class := avatars.DefaultAvatarClass + " mr-3"
+		name := html.EscapeString(username)
+		avatarHTML := string(templates.AvatarHTML(src, 28, class, username))
+		timeSinceText := string(timeutil.TimeSinceUnix(item.EditedUnix, ctx.Locale))
+
 		results = append(results, map[string]interface{}{
-			"name": fmt.Sprintf("<img class='ui avatar image' src='%s'><strong>%s</strong> %s %s",
-				html.EscapeString(item.UserAvatarLink), html.EscapeString(username), actionText, timeSinceText),
+			"name":  avatarHTML + "<strong>" + name + "</strong> " + actionText + " " + timeSinceText,
 			"value": item.HistoryID,
 		})
 	}

web_src/js/features/repo-issue-content.js

@@ -13,20 +13,17 @@ function showContentHistoryDetail(issueBaseUrl, commentId, historyId, itemTitleH
 
   $dialog = $(`
 <div class="ui modal content-history-detail-dialog">
-  <i class="close icon inside"></i>
-  <div class="header">
-    ${itemTitleHtml}
-    <div class="ui dropdown right dialog-header-options" style="display: none; margin-right: 50px;">
-      ${i18nTextOptions} <i class="dropdown icon"></i>
+  ${svg('octicon-x', 16, 'close icon inside')}
+  <div class="header df ac sb">
+    <div>${itemTitleHtml}</div>
+    <div class="ui dropdown dialog-header-options df ac mr-5 hide">
+      ${i18nTextOptions}${svg('octicon-triangle-down', 14, 'dropdown icon')}
       <div class="menu">
         <div class="item red text" data-option-item="delete">${i18nTextDeleteFromHistory}</div>
       </div>
     </div>
   </div>
-  <!-- ".modal .content" style was polluted in "_base.less": "&.modal > .content"  -->
-  <div class="scrolling content" style="text-align: left; min-height: 30vh;">
-      <div class="ui loader active"></div>
-  </div>
+  <div class="comment-diff-data tl p-3 is-loading"></div>
 </div>`);
   $dialog.appendTo($('body'));
   $dialog.find('.dialog-header-options').dropdown({
@@ -62,10 +59,10 @@ function showContentHistoryDetail(issueBaseUrl, commentId, historyId, itemTitleH
           _csrf: csrfToken,
         },
       }).done((resp) => {
-        $dialog.find('.content').html(resp.diffHtml);
+        $dialog.find('.comment-diff-data').removeClass('is-loading').html(resp.diffHtml);
         // there is only one option "item[data-option-item=delete]", so the dropdown can be entirely shown/hidden.
         if (resp.canSoftDelete) {
-          $dialog.find('.dialog-header-options').show();
+          $dialog.find('.dialog-header-options').removeClass('hide');
         }
       });
     },
@@ -79,7 +76,7 @@ function showContentHistoryMenu(issueBaseUrl, $item, commentId) {
   const $headerLeft = $item.find('.comment-header-left');
   const menuHtml = `
   <div class="ui pointing dropdown top left content-history-menu" data-comment-id="${commentId}">
-    <a>&bull; ${i18nTextEdited} ${svg('octicon-triangle-down', 17)}</a>
+    &bull; <a>${i18nTextEdited}${svg('octicon-triangle-down', 14, 'dropdown icon ml-1 mt-1')}</a>
     <div class="menu">
     </div>
   </div>`;

web_src/js/svg.js

@@ -1,11 +1,12 @@
 import octiconChevronDown from '../../public/img/svg/octicon-chevron-down.svg';
 import octiconChevronRight from '../../public/img/svg/octicon-chevron-right.svg';
-import octiconCopy from '../../public/img/svg/octicon-copy.svg';
 import octiconClock from '../../public/img/svg/octicon-clock.svg';
+import octiconCopy from '../../public/img/svg/octicon-copy.svg';
 import octiconDiffAdded from '../../public/img/svg/octicon-diff-added.svg';
 import octiconDiffModified from '../../public/img/svg/octicon-diff-modified.svg';
 import octiconDiffRemoved from '../../public/img/svg/octicon-diff-removed.svg';
 import octiconDiffRenamed from '../../public/img/svg/octicon-diff-renamed.svg';
+import octiconFile from '../../public/img/svg/octicon-file.svg';
 import octiconFileDirectoryFill from '../../public/img/svg/octicon-file-directory-fill.svg';
 import octiconGitMerge from '../../public/img/svg/octicon-git-merge.svg';
 import octiconGitPullRequest from '../../public/img/svg/octicon-git-pull-request.svg';
@@ -20,17 +21,23 @@ import octiconProject from '../../public/img/svg/octicon-project.svg';
 import octiconRepo from '../../public/img/svg/octicon-repo.svg';
 import octiconRepoForked from '../../public/img/svg/octicon-repo-forked.svg';
 import octiconRepoTemplate from '../../public/img/svg/octicon-repo-template.svg';
-import octiconTriangleDown from '../../public/img/svg/octicon-triangle-down.svg';
-import octiconFile from '../../public/img/svg/octicon-file.svg';
-import octiconSidebarExpand from '../../public/img/svg/octicon-sidebar-expand.svg';
 import octiconSidebarCollapse from '../../public/img/svg/octicon-sidebar-collapse.svg';
+import octiconSidebarExpand from '../../public/img/svg/octicon-sidebar-expand.svg';
+import octiconTriangleDown from '../../public/img/svg/octicon-triangle-down.svg';
+import octiconX from '../../public/img/svg/octicon-x.svg';
 
 
 export const svgs = {
   'octicon-chevron-down': octiconChevronDown,
   'octicon-chevron-right': octiconChevronRight,
-  'octicon-copy': octiconCopy,
   'octicon-clock': octiconClock,
+  'octicon-copy': octiconCopy,
+  'octicon-diff-added': octiconDiffAdded,
+  'octicon-diff-modified': octiconDiffModified,
+  'octicon-diff-removed': octiconDiffRemoved,
+  'octicon-diff-renamed': octiconDiffRenamed,
+  'octicon-file': octiconFile,
+  'octicon-file-directory-fill': octiconFileDirectoryFill,
   'octicon-git-merge': octiconGitMerge,
   'octicon-git-pull-request': octiconGitPullRequest,
   'octicon-issue-closed': octiconIssueClosed,
@@ -44,15 +51,10 @@ export const svgs = {
   'octicon-repo': octiconRepo,
   'octicon-repo-forked': octiconRepoForked,
   'octicon-repo-template': octiconRepoTemplate,
-  'octicon-triangle-down': octiconTriangleDown,
-  'octicon-file': octiconFile,
-  'octicon-file-directory-fill': octiconFileDirectoryFill,
-  'octicon-sidebar-expand': octiconSidebarExpand,
   'octicon-sidebar-collapse': octiconSidebarCollapse,
-  'octicon-diff-added': octiconDiffAdded,
-  'octicon-diff-modified': octiconDiffModified,
-  'octicon-diff-removed': octiconDiffRemoved,
-  'octicon-diff-renamed': octiconDiffRenamed,
+  'octicon-sidebar-expand': octiconSidebarExpand,
+  'octicon-triangle-down': octiconTriangleDown,
+  'octicon-x': octiconX,
 };
 
 

web_src/less/_repository.less

@@ -2907,6 +2907,24 @@ tbody.commit-list {
   text-align: left;
 }
 
+.comment-diff-data {
+  background: var(--color-code-bg);
+  max-height: calc(100vh - 10.5rem);
+  overflow-y: auto;
+}
+
+.comment-diff-data pre {
+  line-height: 18px;
+  white-space: pre-wrap;
+  word-break: break-all;
+  overflow-wrap: break-word;
+}
+
+.content-history-detail-dialog .header .avatar {
+  position: relative;
+  top: -2px;
+}
+
 #topic_edit {
   margin-top: 5px;
 }