Diff improvements

Author: silverwind

templates/repo/diff/box.tmpl

@@ -17,9 +17,15 @@
 			<div class="gt-df gt-ac gt-fw">
 				<a class="diff-toggle-file-tree-button muted gt-df gt-ac">
 					{{/* the icon meaning is reversed here, "octicon-sidebar-collapse" means show the file tree */}}
-					{{svg "octicon-sidebar-collapse" 20 "icon hide"}}
-					{{svg "octicon-sidebar-expand" 20 "icon"}}
+					{{svg "octicon-sidebar-collapse" 20 "icon gt-hidden"}}
+					{{svg "octicon-sidebar-expand" 20 "icon gt-hidden"}}
 				</a>
+				<script>
+					const treeVisible = localStorage.getItem('diff_file_tree_visible') === 'true';
+					const btn = document.querySelector('.diff-toggle-file-tree-button');
+					const icon = `.octicon-sidebar-${treeVisible ? 'expand' : 'collapse'}`;
+					btn.querySelector(icon).classList.remove('gt-hidden');
+				</script>
 				<div class="diff-detail-stats gt-df gt-ac gt-ml-3">
 					{{svg "octicon-diff" 16 "gt-mr-2"}}{{.locale.Tr "repo.diff.stats_desc" .Diff.NumFiles .Diff.TotalAddition .Diff.TotalDeletion | Str2html}}
 				</div>
@@ -68,6 +74,9 @@
 		<div id="diff-file-list"></div>
 		<div id="diff-container">
 				<div id="diff-file-tree" class="gt-hidden"></div>
+				<script>
+					if (treeVisible) document.getElementById('diff-file-tree').classList.remove('gt-hidden');
+				</script>
 				<div id="diff-file-boxes" class="sixteen wide column">
 					{{range $i, $file := .Diff.Files}}
 						{{/*notice: the index of Diff.Files should not be used for element ID, because the index will be restarted from 0 when doing load-more for PRs with a lot of files*/}}

web_src/css/repository.css

@@ -1852,6 +1852,24 @@
   padding-bottom: 5px;
 }
 
+.diff-file-box {
+  border: 1px solid transparent;
+  border-radius: 0.285rem; /* Just like ui.top.attached.header */
+}
+
+.diff-file-box .ui.attached.header,
+.diff-file-box .ui.attached.table {
+  margin: 0; /* remove fomantic negative margins */;
+  width: initial; /* remove fomantic over 100% width */;
+  max-width: initial; /* remove fomantic over 100% width */;
+}
+
+.diff-file-box:target {
+  border-color: var(--color-primary) !important;
+  border-radius: var(--border-radius) !important;
+  box-shadow: 0 0 0 3px var(--color-primary-alpha-30) !important;
+}
+
 .repository .diff-stats {
   clear: both;
   margin-bottom: 5px;

web_src/css/review.css

@@ -308,11 +308,3 @@ a.blob-excerpt:hover {
   width: 72px;
   height: 10px;
 }
-
-.diff-file-box {
-  border-radius: 0.285rem; /* Just like ui.top.attached.header */
-}
-
-.diff-file-box:target {
-  box-shadow: 0 0 0 3px var(--color-accent);
-}

web_src/js/components/DiffFileTree.vue

@@ -16,6 +16,7 @@
 <script>
 import DiffFileTreeItem from './DiffFileTreeItem.vue';
 import {doLoadMoreFiles} from '../features/repo-diff.js';
+import {hideElem, showElem} from '../utils/dom.js';
 
 const {pageData} = window.config;
 const LOCAL_STORAGE_KEY = 'diff_file_tree_visible';
@@ -93,7 +94,7 @@ export default {
   },
   mounted() {
     // ensure correct buttons when we are mounted to the dom
-    this.adjustToggleButton(this.fileTreeIsVisible);
+    this.updateState(this.fileTreeIsVisible);
     // replace the pageData.diffFileInfo.files with our watched data so we get updates
     pageData.diffFileInfo.files = this.files;
 
@@ -109,15 +110,20 @@ export default {
     updateVisibility(visible) {
       this.fileTreeIsVisible = visible;
       localStorage.setItem(LOCAL_STORAGE_KEY, this.fileTreeIsVisible);
-      this.adjustToggleButton(this.fileTreeIsVisible);
+      this.updateState(this.fileTreeIsVisible);
     },
-    adjustToggleButton(visible) {
+    updateState(visible) {
       const [toShow, toHide] = document.querySelectorAll('.diff-toggle-file-tree-button .icon');
-      toShow.classList.toggle('gt-hidden', visible);  // hide the toShow icon if the tree is visible
-      toHide.classList.toggle('gt-hidden', !visible); // similarly
-
-      const diffTree = document.getElementById('diff-file-tree');
-      diffTree.classList.toggle('gt-hidden', !visible);
+      const tree = document.getElementById('diff-file-tree');
+      if (visible) {
+        showElem(tree);
+        hideElem(toShow);
+        showElem(toHide);
+      } else {
+        hideElem(tree);
+        hideElem(toHide);
+        showElem(toShow);
+      }
     },
     loadMoreData() {
       this.isLoadingNewData = true;