Improve PR Review Box UI (#22986)

This PR follows: * #22950 ### Before The Review Box has many problems: * It doesn't work for small screens. * It has an anonying animation which makes the UI laggy. * It uses "custom dropdown menu" which is very difficult to fine tune. * `$().toggle('visible')` is not a correct call * jQuery just accepts any invalid `duration` argument: `$().toggle('anyting')` * The button is not a button. <details> ![image](https://user-images.githubusercontent.com/2114189/219948865-6da3f39c-6fde-4c86-9e42-da5020f3d0c3.png) </details> ### After These problems are fixed, and eliminate many `!important` games. <details> ![image](https://user-images.githubusercontent.com/2114189/219952744-8862fe1a-7ef1-49e4-bf92-2d0c1f104ee4.png) ![image](https://user-images.githubusercontent.com/2114189/219952771-be169a76-45fd-47a8-8f9c-b447d064f4ca.png) ![image](https://user-images.githubusercontent.com/2114189/219952784-3f52e9b7-64ce-4ad1-9553-64c33fb83042.png) </details> And most dropdown icons still looks good: <details> ![image](https://user-images.githubusercontent.com/2114189/219952942-52866a00-e0f9-4af7-8fb5-eb1a8cad1ff3.png) ![image](https://user-images.githubusercontent.com/2114189/219948909-b3bfb844-f84e-4b79-ab1f-382ec66dec31.png) </details> Co-authored-by: delvh <leon@kske.dev>
go-gitea · Feb 21, 2023 · 1fcf96a · 1fcf96a
1 parent 9ebf642
commit 1fcf96a
Show file tree

Hide file tree

Showing 9 changed files with 35 additions and 52 deletions.
diff --git a/templates/repo/diff/box.tmpl b/templates/repo/diff/box.tmpl
@@ -24,10 +24,10 @@
 					{{svg "octicon-diff" 16 "gt-mr-2"}}{{.locale.Tr "repo.diff.stats_desc" .Diff.NumFiles .Diff.TotalAddition .Diff.TotalDeletion | Str2html}}
 				</div>
 			</div>
-			<div class="diff-detail-actions gt-df gt-ac">
+			<div class="diff-detail-actions gt-df gt-ac gt-w-100">
 				{{if and .PageIsPullFiles $.SignedUserID (not .IsArchived)}}
 					<progress id="viewed-files-summary" class="gt-mr-2" value="{{.Diff.NumViewedFiles}}" max="{{.Diff.NumFiles}}"></progress>
-					<label for="viewed-files-summary" id="viewed-files-summary-label" class="gt-mr-3" data-text-changed-template="{{.locale.Tr "repo.pulls.viewed_files_label"}}">
+					<label for="viewed-files-summary" id="viewed-files-summary-label" class="gt-mr-3 gt-f1" data-text-changed-template="{{.locale.Tr "repo.pulls.viewed_files_label"}}">
 						{{.locale.Tr "repo.pulls.viewed_files_label" .Diff.NumViewedFiles .Diff.NumFiles}}
 					</label>
 				{{end}}

diff --git a/templates/repo/diff/new_review.tmpl b/templates/repo/diff/new_review.tmpl
@@ -1,11 +1,11 @@
-<div class="ui top right pointing dropdown custom" id="review-box">
-	<div class="ui tiny green button btn-review gt-ml-2 gt-mr-0">
+<div id="review-box">
+	<button class="ui tiny green button gt-ml-2 gt-mr-0 js-btn-review">
 		{{.locale.Tr "repo.diff.review"}}
 		<span class="ui small label review-comments-counter" data-pending-comment-number="{{.PendingCodeCommentNumber}}">{{.PendingCodeCommentNumber}}</span>
 		{{svg "octicon-triangle-down" 14 "dropdown icon"}}
-	</div>
-	<div class="menu review-box">
-		<div class="ui clearing segment">
+	</button>
+	<div class="review-box-panel gt-hidden">
+		<div class="ui segment">
 			<form class="ui form" action="{{.Link}}/reviews/submit" method="post">
 			{{.CsrfTokenHtml}}
 				<input type="hidden" name="commit_id" value="{{.AfterCommitID}}"/>

diff --git a/templates/repo/home.tmpl b/templates/repo/home.tmpl
@@ -77,7 +77,7 @@
 					<a href="{{.Repository.Link}}/find/{{.BranchNameSubURL}}" class="ui compact basic button">{{.locale.Tr "repo.find_file.go_to_file"}}</a>
 				{{end}}
 				{{if or .CanAddFile .CanUploadFile}}
-					<button class="ui basic small compact dropdown jump icon button gt-mr-2"{{if not .Repository.CanEnableEditor}} disabled{{end}}>
+					<button class="ui basic compact dropdown jump icon button gt-mr-2"{{if not .Repository.CanEnableEditor}} disabled{{end}}>
 						<span class="text">{{.locale.Tr "repo.editor.add_file"}}</span>
 						<div class="menu">
 							{{if .CanAddFile}}

diff --git a/web_src/js/features/common-global.js b/web_src/js/features/common-global.js
@@ -60,6 +60,7 @@ export function initGlobalEnterQuickSubmit() {
 export function initGlobalButtonClickOnEnter() {
   $(document).on('keypress', '.ui.button', (e) => {
     if (e.keyCode === 13 || e.keyCode === 32) { // enter key or space bar
+      if (e.target.nodeName === 'BUTTON') return; // button already handles space&enter correctly
       $(e.target).trigger('click');
       e.preventDefault();
     }

diff --git a/web_src/js/features/repo-issue.js b/web_src/js/features/repo-issue.js
@@ -470,7 +470,7 @@ export function initRepoPullRequestReview() {
     assignMenuAttributes(form.find('.menu'));
   });
 
-  const $reviewBox = $('.review-box');
+  const $reviewBox = $('.review-box-panel');
   if ($reviewBox.length === 1) {
     (async () => {
       // the editor's height is too large in some cases, and the panel cannot be scrolled with page now because there is `.repository .diff-detail-box.sticky { position: sticky; }`
@@ -487,12 +487,12 @@ export function initRepoPullRequestReview() {
     return;
   }
 
-  $('.btn-review').on('click', function (e) {
+  $('.js-btn-review').on('click', function (e) {
     e.preventDefault();
-    $(this).closest('.dropdown').find('.menu').toggle('visible'); // eslint-disable-line
-  }).closest('.dropdown').find('.close').on('click', function (e) {
+    toggleElem($(this).parent().find('.review-box-panel'));
+  }).parent().find('.review-box-panel .close').on('click', function (e) {
     e.preventDefault();
-    $(this).closest('.menu').toggle('visible'); // eslint-disable-line
+    hideElem($(this).closest('.review-box-panel'));
   });
 
   $(document).on('click', 'a.add-code-comment', async function (e) {

diff --git a/web_src/less/_base.less b/web_src/less/_base.less
@@ -2476,24 +2476,13 @@ table th[data-sortt-desc] {
   }
 }
 
-/* fix up SVG dropdown triangles because fomantic thinks they are icon fonts */
-/* see https://github.com/go-gitea/gitea/issues/14014 */
-.ui.dropdown > .dropdown.icon,
-.btn-review > .dropdown.icon {
-  height: auto !important;
-  margin-left: .5rem !important;
-  margin-top: -1px !important;
-  margin-bottom: -1px !important;
-  margin-right: -.5rem !important;
+.ui.dropdown .svg.dropdown.icon,
+.svg.dropdown.icon {
+  margin-top: 0 !important; // reset the "ui.selection.dropdown > .dropdown.icon {margin-top}", for the Issue Dependencies dropdown
+  margin-right: -.5rem !important; // fix up SVG dropdown triangles because Fomantic thinks they are icon fonts
+  height: auto; // reset the ".ui.dropdown > .dropdown.icon {height}", otherwise the icon would be too small
 }
-.ui.button.dropdown > .dropdown.icon,
-.btn-review > .dropdown.icon {
-  float: right !important;
 
-  @media (max-width: 480px) {
-    display: none;
-  }
-}
 .ui.selection.dropdown > .search.icon,
 .ui.selection.dropdown > .delete.icon,
 .ui.selection.dropdown > .dropdown.icon {

diff --git a/web_src/less/_editor.less b/web_src/less/_editor.less
@@ -13,7 +13,7 @@
 }
 
 .editor-toolbar {
-  opacity: 1 !important;
+  max-width: calc(100vw - 80px);
   border-color: var(--color-secondary);
 }
 

diff --git a/web_src/less/_repository.less b/web_src/less/_repository.less
@@ -2864,11 +2864,6 @@
   margin-top: 10px;
 }
 
-.repo-button-row .dropdown > .dropdown.icon {
-  margin-left: .25rem !important;
-  margin-right: 0 !important;
-}
-
 .wiki .repo-button-row {
   margin-bottom: 0;
 }

diff --git a/web_src/less/_review.less b/web_src/less/_review.less
@@ -220,47 +220,45 @@ a.blob-excerpt:hover {
 
 // See the comment of createCommentEasyMDE() for the review editor
 // EasyMDE's options can not handle minHeight & maxHeight together correctly, we have to set minHeight in JS code
-#review-box .CodeMirror-scroll {
+.review-box-panel .CodeMirror-scroll {
   min-height: 80px;
   max-height: calc(100vh - 360px);
 }
 
 @media @mediaSm {
-  #review-box > .menu {
-    > .ui.segment {
-      width: 94vw;
-    }
-
-    .editor-toolbar {
-      overflow-x: auto;
-    }
-  }
-
-  #review-box .CodeMirror-scroll {
+  .review-box-panel .CodeMirror-scroll {
     max-width: calc(100vw - 70px);
   }
 }
 
 @media @mediaMd {
-  #review-box .CodeMirror-scroll {
+  .review-box-panel .CodeMirror-scroll {
     max-width: 700px;
   }
 }
 
 @media @mediaLg {
-  #review-box .CodeMirror-scroll {
+  .review-box-panel .CodeMirror-scroll {
     max-width: 800px;
   }
 }
 
 @media @mediaXl {
-  #review-box .CodeMirror-scroll {
+  .review-box-panel .CodeMirror-scroll {
     max-width: 900px;
   }
 }
 
-.review-box > .segment {
-  border: none !important;
+#review-box {
+  position: relative;
+}
+
+.review-box-panel {
+  position: absolute;
+  min-width: max-content;
+  top: 45px;
+  right: -5px;
+  z-index: 2;
 }
 
 #review-box .review-comments-counter {