From 44660203bdd4beedad8eae61612dbaa2237b64d9 Mon Sep 17 00:00:00 2001
From: Thomas Randolph <trandolph@gitlab.com>
Date: Mon, 15 Feb 2021 10:33:52 -0700
Subject: [PATCH 1/3] Add Diffs app skeleton loader components

---
 .../components/loaders/diff_files_loading.vue | 40 +++++++++++++++++++
 .../components/loaders/file_tree_loading.vue  | 24 +++++++++++
 .../loaders/mr_version_controls_loading.vue   | 24 +++++++++++
 .../stylesheets/pages/merge_requests.scss     |  4 ++
 4 files changed, 92 insertions(+)
 create mode 100644 app/assets/javascripts/diffs/components/loaders/diff_files_loading.vue
 create mode 100644 app/assets/javascripts/diffs/components/loaders/file_tree_loading.vue
 create mode 100644 app/assets/javascripts/diffs/components/loaders/mr_version_controls_loading.vue

diff --git a/app/assets/javascripts/diffs/components/loaders/diff_files_loading.vue b/app/assets/javascripts/diffs/components/loaders/diff_files_loading.vue
new file mode 100644
index 00000000000000..55d0c89218aef5
--- /dev/null
+++ b/app/assets/javascripts/diffs/components/loaders/diff_files_loading.vue
@@ -0,0 +1,40 @@
+<script>
+import { GlLoadingIcon, GlSkeletonLoader } from '@gitlab/ui';
+
+export default {
+  components: {
+    GlLoadingIcon,
+    GlSkeletonLoader,
+  },
+};
+</script>
+
+<template>
+  <div class="gl-display-grid gl-px-5 gl-w-80p diff-files-loader">
+    <gl-loading-icon />
+    <gl-skeleton-loader>
+      <rect x="5" y="5" width="30%" height="5" />
+
+      <rect x="10" y="15" width="1" height="100%" />
+      <rect x="20" y="15" width="1" height="100%" />
+
+      <rect x="25" y="25" width="50%" height="3" />
+      <rect x="25" y="33" width="60%" height="3" />
+      <rect x="25" y="41" width="50%" height="3" />
+      <rect x="25" y="49" width="60%" height="3" />
+      <rect x="25" y="57" width="40%" height="3" />
+    </gl-skeleton-loader>
+    <gl-skeleton-loader>
+      <rect x="5" y="5" width="30%" height="5" />
+
+      <rect x="10" y="15" width="1" height="100%" />
+      <rect x="20" y="15" width="1" height="100%" />
+
+      <rect x="25" y="25" width="50%" height="3" />
+      <rect x="25" y="33" width="60%" height="3" />
+      <rect x="25" y="41" width="50%" height="3" />
+      <rect x="25" y="49" width="60%" height="3" />
+      <rect x="25" y="57" width="40%" height="3" />
+    </gl-skeleton-loader>
+  </div>
+</template>
diff --git a/app/assets/javascripts/diffs/components/loaders/file_tree_loading.vue b/app/assets/javascripts/diffs/components/loaders/file_tree_loading.vue
new file mode 100644
index 00000000000000..d5d57160e66ad7
--- /dev/null
+++ b/app/assets/javascripts/diffs/components/loaders/file_tree_loading.vue
@@ -0,0 +1,24 @@
+<script>
+import { GlLoadingIcon, GlSkeletonLoader } from '@gitlab/ui';
+
+export default {
+  components: {
+    GlLoadingIcon,
+    GlSkeletonLoader,
+  },
+};
+</script>
+
+<template>
+  <div class="gl-display-grid gl-px-5 file-tree-loader">
+    <gl-loading-icon />
+    <gl-skeleton-loader :width="100" :height="200">
+      <rect x="1" y="0" rx="2" ry="2" width="90%" height="10" />
+      <rect x="1" y="15" width="75%" height="2.5" />
+      <rect x="1" y="22.5" width="90%" height="2.5" />
+      <rect x="1" y="30" width="75%" height="2.5" />
+      <rect x="1" y="37.5" width="90%" height="2.5" />
+      <rect x="1" y="45" width="50%" height="2.5" />
+    </gl-skeleton-loader>
+  </div>
+</template>
diff --git a/app/assets/javascripts/diffs/components/loaders/mr_version_controls_loading.vue b/app/assets/javascripts/diffs/components/loaders/mr_version_controls_loading.vue
new file mode 100644
index 00000000000000..8d0aeb01921c34
--- /dev/null
+++ b/app/assets/javascripts/diffs/components/loaders/mr_version_controls_loading.vue
@@ -0,0 +1,24 @@
+<script>
+import { GlLoadingIcon, GlSkeletonLoader } from '@gitlab/ui';
+
+export default {
+  components: {
+    GlLoadingIcon,
+    GlSkeletonLoader,
+  },
+};
+</script>
+
+<template>
+  <div class="gl-display-grid gl-my-5 gl-px-5 version-controls-loader">
+    <gl-loading-icon />
+    <gl-skeleton-loader :height="32" :width="240">
+      <rect width="32" height="32" rx="4" fill="black" fill-opacity="0.16" />
+      <rect x="40" y="4" width="400" height="24" fill="black" fill-opacity="0.16" />
+    </gl-skeleton-loader>
+    <gl-skeleton-loader :height="32" :width="240">
+      <rect x="176" width="64" height="32" rx="4" fill="black" fill-opacity="0.16" />
+      <rect y="12" width="160" height="8" fill="black" fill-opacity="0.16" />
+    </gl-skeleton-loader>
+  </div>
+</template>
diff --git a/app/assets/stylesheets/pages/merge_requests.scss b/app/assets/stylesheets/pages/merge_requests.scss
index a62df1258afc39..f6f898de689d77 100644
--- a/app/assets/stylesheets/pages/merge_requests.scss
+++ b/app/assets/stylesheets/pages/merge_requests.scss
@@ -439,6 +439,10 @@ $mr-widget-min-height: 69px;
   }
 }
 
+.version-controls-loader {
+  grid-template-columns: 1fr 1fr;
+}
+
 .nothing-here-block {
   img {
     width: 230px;
-- 
GitLab


From d680554a544ac8ecb811155b22a1cd199b618e11 Mon Sep 17 00:00:00 2001
From: Thomas Randolph <trandolph@gitlab.com>
Date: Mon, 15 Feb 2021 10:34:18 -0700
Subject: [PATCH 2/3] Use skeleton loader components in the Diffs app

---
 .../javascripts/diffs/components/app.vue      | 44 ++++++++----
 .../stylesheets/pages/merge_requests.scss     | 67 +++++++++++++++++++
 2 files changed, 99 insertions(+), 12 deletions(-)

diff --git a/app/assets/javascripts/diffs/components/app.vue b/app/assets/javascripts/diffs/components/app.vue
index bb3111486275a4..81dec3844e088b 100644
--- a/app/assets/javascripts/diffs/components/app.vue
+++ b/app/assets/javascripts/diffs/components/app.vue
@@ -1,4 +1,5 @@
 <script>
+import { mapState, mapGetters, mapActions } from 'vuex';
 import { GlLoadingIcon, GlPagination, GlSprintf } from '@gitlab/ui';
 import { GlBreakpointInstance as bp } from '@gitlab/ui/dist/utils';
 import Mousetrap from 'mousetrap';
@@ -30,15 +31,20 @@ import eventHub from '../event_hub';
 import { reviewStatuses } from '../utils/file_reviews';
 import { diffsApp } from '../utils/performance';
 import { fileByFile } from '../utils/preferences';
-import CollapsedFilesWarning from './collapsed_files_warning.vue';
 import CommitWidget from './commit_widget.vue';
 import CompareVersions from './compare_versions.vue';
 import DiffFile from './diff_file.vue';
-import HiddenFilesWarning from './hidden_files_warning.vue';
-import MergeConflictWarning from './merge_conflict_warning.vue';
 import NoChanges from './no_changes.vue';
 import TreeList from './tree_list.vue';
 
+import DiffFilesLoader from './loaders/diff_files_loading.vue';
+import FileTreeLoader from './loaders/file_tree_loading.vue';
+import VersionControlsLoader from './loaders/mr_version_controls_loading.vue';
+
+import HiddenFilesWarning from './hidden_files_warning.vue';
+import MergeConflictWarning from './merge_conflict_warning.vue';
+import CollapsedFilesWarning from './collapsed_files_warning.vue';
+
 export default {
   name: 'DiffsApp',
   components: {
@@ -48,9 +54,11 @@ export default {
     HiddenFilesWarning,
     MergeConflictWarning,
     CollapsedFilesWarning,
+    DiffFilesLoader,
+    FileTreeLoader,
+    VersionControlsLoader,
     CommitWidget,
     TreeList,
-    GlLoadingIcon,
     PanelResizer,
     GlPagination,
     GlSprintf,
@@ -363,10 +371,11 @@ export default {
       return !this.diffFiles.length;
     },
     fetchData(toggleTree = true) {
+      if (toggleTree) this.setTreeDisplay();
+
       this.fetchDiffFilesMeta()
         .then(({ real_size }) => {
           this.diffFilesLength = parseInt(real_size, 10);
-          if (toggleTree) this.setTreeDisplay();
 
           this.startDiffRendering();
         })
@@ -462,10 +471,11 @@ export default {
 </script>
 
 <template>
-  <div v-show="shouldShow">
-    <div v-if="isLoading || !isTreeLoaded" class="loading"><gl-loading-icon size="lg" /></div>
-    <div v-else id="diffs" :class="{ active: shouldShow }" class="diffs tab-pane">
+  <div>
+    <div id="diffs" :class="{ active: shouldShow }" class="diffs tab-pane">
+      <version-controls-loader v-if="isLoading || !isTreeLoaded" />
       <compare-versions
+        v-else
         :is-limited-container="isLimitedContainer"
         :diff-files-count-text="numTotalFiles"
       />
@@ -490,10 +500,20 @@ export default {
 
       <div
         :data-can-create-note="getNoteableData.current_user.can_create_note"
-        class="files d-flex gl-mt-2"
+        class="files gl-mt-2"
+        :class="{ 'with-tree': showTreeList }"
+        :style="{
+          gridTemplateColumns: showTreeList
+            ? `minmax( ${$options.minTreeWidth}px, ${treeWidth}px ) 1fr`
+            : '0 1fr',
+        }"
       >
+        <file-tree-loader
+          v-if="showTreeList && (isLoading || !isTreeLoaded)"
+          class="px-3 pr-md-0"
+        />
         <div
-          v-if="renderFileTree"
+          v-else-if="renderFileTree"
           :style="{ width: `${treeWidth}px` }"
           class="diff-tree-list js-diff-tree-list px-3 pr-md-0"
         >
@@ -514,7 +534,7 @@ export default {
           }"
         >
           <commit-widget v-if="commit" :commit="commit" :collapsible="false" />
-          <div v-if="isBatchLoading" class="loading"><gl-loading-icon size="lg" /></div>
+          <diff-files-loader v-if="isBatchLoading" />
           <template v-else-if="renderDiffFiles">
             <diff-file
               v-for="(file, index) in diffs"
@@ -544,7 +564,7 @@ export default {
                 <template #total>{{ diffFiles.length }}</template>
               </gl-sprintf>
             </div>
-            <gl-loading-icon v-else-if="retrievingBatches" size="lg" />
+            <diff-files-loader v-else-if="retrievingBatches" />
           </template>
           <no-changes v-else :changes-empty-state-illustration="changesEmptyStateIllustration" />
         </div>
diff --git a/app/assets/stylesheets/pages/merge_requests.scss b/app/assets/stylesheets/pages/merge_requests.scss
index f6f898de689d77..ca1eae2f5426a4 100644
--- a/app/assets/stylesheets/pages/merge_requests.scss
+++ b/app/assets/stylesheets/pages/merge_requests.scss
@@ -439,10 +439,77 @@ $mr-widget-min-height: 69px;
   }
 }
 
+/************************************
+ *
+ * App Layout during loading states
+ *
+ ***********************************/
+
 .version-controls-loader {
   grid-template-columns: 1fr 1fr;
+
+  svg {
+    max-height: 32px;
+
+    &:last-of-type {
+      position: absolute;
+      right: 78px;
+    }
+  }
+
+  .gl-spinner-container {
+    grid-column: 1 / 3;
+  }
 }
 
+.version-controls-loader,
+.file-tree-loader,
+.diff-files-loader {
+  .gl-skeleton-loader {
+    display: none;
+  }
+
+  @include media-breakpoint-up(sm) {
+    .gl-spinner-container {
+      display: none;
+    }
+
+    .gl-skeleton-loader {
+      display: unset;
+    }
+  }
+}
+
+.diffs {
+  .files {
+    display: flex;
+
+    &.with-tree {
+      @include media-breakpoint-up(md) {
+        display: grid;
+      }
+
+      .diff-files-loader .gl-spinner-container {
+        @include gl-mt-5;
+      }
+    }
+  }
+
+  .diff-files-holder {
+    grid-column: 1;
+
+    @include media-breakpoint-up(md) {
+      grid-column: 2;
+    }
+  }
+}
+
+/************************************
+ *
+ * In lieu of another section starting: End of previous section
+ *
+ ***********************************/
+
 .nothing-here-block {
   img {
     width: 230px;
-- 
GitLab


From 7e97fcc6c2ae9db58a55a9a6012df323d27e3032 Mon Sep 17 00:00:00 2001
From: Thomas Randolph <trandolph@gitlab.com>
Date: Tue, 2 Feb 2021 05:06:12 -0700
Subject: [PATCH 3/3] Add changelog for switching to skeleton loaders

---
 changelogs/unreleased/tor-feature-mr-skeleton-loader.yml | 6 ++++++
 1 file changed, 6 insertions(+)
 create mode 100644 changelogs/unreleased/tor-feature-mr-skeleton-loader.yml

diff --git a/changelogs/unreleased/tor-feature-mr-skeleton-loader.yml b/changelogs/unreleased/tor-feature-mr-skeleton-loader.yml
new file mode 100644
index 00000000000000..160129f373fc46
--- /dev/null
+++ b/changelogs/unreleased/tor-feature-mr-skeleton-loader.yml
@@ -0,0 +1,6 @@
+---
+title: Swap to skeleton loaders from a single circular loader on MR Diffs (Changes)
+  tab
+merge_request: 53133
+author:
+type: added
-- 
GitLab