Revert "Merge branch '118442-restyle-changes-header-and-file-tree' into 'master'"

This reverts commit 5b9bdbb0, reversing
changes made to 65a22395.

app/assets/javascripts/diffs/components/app.vue

@@ -374,7 +374,7 @@ export default {
 
       <div
         :data-can-create-note="getNoteableData.current_user.can_create_note"
-        class="files d-flex"
+        class="files d-flex prepend-top-default"
       >
         <div
           v-show="showTreeList"

app/assets/javascripts/diffs/components/compare_versions.vue

 <script>
+/* eslint-disable @gitlab/vue-i18n/no-bare-strings */
 import { mapActions, mapGetters, mapState } from 'vuex';
 import { GlTooltipDirective, GlLink, GlButton } from '@gitlab/ui';
 import { __ } from '~/locale';
@@ -62,6 +63,9 @@ export default {
     showDropdowns() {
       return !this.commit && this.mergeRequestDiffs.length;
     },
+    fileTreeIcon() {
+      return this.showTreeList ? 'collapse-left' : 'expand-left';
+    },
     toggleFileBrowserTitle() {
       return this.showTreeList ? __('Hide file browser') : __('Show file browser');
     },
@@ -87,7 +91,7 @@ export default {
 </script>
 
 <template>
-  <div class="mr-version-controls border-top">
+  <div class="mr-version-controls border-top border-bottom">
     <div
       class="mr-version-menus-container content-block"
       :class="{
@@ -104,17 +108,17 @@ export default {
         :title="toggleFileBrowserTitle"
         @click="toggleShowTreeList"
       >
-        <icon name="file-tree" />
+        <icon :name="fileTreeIcon" />
       </button>
       <div v-if="showDropdowns" class="d-flex align-items-center compare-versions-container">
-        {{ __('Compare') }}
+        Changes between
         <compare-versions-dropdown
           :other-versions="mergeRequestDiffs"
           :merge-request-version="mergeRequestDiff"
           :show-commit-count="true"
           class="mr-version-dropdown"
         />
-        {{ __('and') }}
+        and
         <compare-versions-dropdown
           :other-versions="comparableDiffs"
           :base-version-path="baseVersionPath"

app/assets/javascripts/diffs/components/diff_file_header.vue

@@ -123,20 +123,6 @@ export default {
       }
       return s__('MRDiff|Show full file');
     },
-    changedFile() {
-      const {
-        new_path: changed,
-        deleted_file: deleted,
-        new_file: tempFile,
-        ...diffFile
-      } = this.diffFile;
-      return {
-        ...diffFile,
-        changed: Boolean(changed),
-        deleted,
-        tempFile,
-      };
-    },
   },
   mounted() {
     polyfillSticky(this.$refs.header);
@@ -235,7 +221,7 @@ export default {
 
     <div
       v-if="!diffFile.submodule && addMergeRequestButtons"
-      class="file-actions d-none d-sm-flex align-items-center"
+      class="file-actions d-none d-sm-block"
     >
       <diff-stats :added-lines="diffFile.added_lines" :removed-lines="diffFile.removed_lines" />
       <div class="btn-group" role="group">

app/assets/javascripts/diffs/components/diff_stats.vue

 <script>
+import Icon from '~/vue_shared/components/icon.vue';
 import { n__ } from '~/locale';
 
 export default {
+  components: { Icon },
   props: {
     addedLines: {
       type: Number,
@@ -19,7 +21,7 @@ export default {
   },
   computed: {
     filesText() {
-      return n__('file', 'files', this.diffFilesLength);
+      return n__('File', 'Files', this.diffFilesLength);
     },
     isCompareVersionsHeader() {
       return Boolean(this.diffFilesLength);
@@ -37,21 +39,14 @@ export default {
     }"
   >
     <div v-if="diffFilesLength !== null" class="diff-stats-group">
-      <span class="text-secondary bold">{{ diffFilesLength }} {{ filesText }}</span>
+      <icon name="doc-code" class="diff-stats-icon text-secondary" />
+      <strong>{{ diffFilesLength }} {{ filesText }}</strong>
     </div>
-    <div
-      class="diff-stats-group cgreen d-flex align-items-center"
-      :class="{ bold: isCompareVersionsHeader }"
-    >
-      <span>+</span>
-      <span class="js-file-addition-line">{{ addedLines }}</span>
+    <div class="diff-stats-group cgreen">
+      <icon name="file-addition" class="diff-stats-icon" /> <strong>{{ addedLines }}</strong>
     </div>
-    <div
-      class="diff-stats-group cred d-flex align-items-center"
-      :class="{ bold: isCompareVersionsHeader }"
-    >
-      <span>-</span>
-      <span class="js-file-deletion-line">{{ removedLines }}</span>
+    <div class="diff-stats-group cred">
+      <icon name="file-deletion" class="diff-stats-icon" /> <strong>{{ removedLines }}</strong>
     </div>
   </div>
 </template>

app/assets/javascripts/diffs/components/tree_list.vue

@@ -4,6 +4,7 @@ import { GlTooltipDirective } from '@gitlab/ui';
 import { s__, sprintf } from '~/locale';
 import Icon from '~/vue_shared/components/icon.vue';
 import FileRow from '~/vue_shared/components/file_row.vue';
+import FileRowStats from './file_row_stats.vue';
 
 export default {
   directives: {
@@ -47,6 +48,9 @@ export default {
         return acc;
       }, []);
     },
+    fileRowExtraComponent() {
+      return this.hideFileStats ? null : FileRowStats;
+    },
   },
   methods: {
     ...mapActions('diffs', ['toggleTreeOpen', 'scrollToFile']),
@@ -54,8 +58,8 @@ export default {
       this.search = '';
     },
   },
-  searchPlaceholder: sprintf(s__('MergeRequest|Search files (%{modifier_key}P)'), {
-    modifier_key: /Mac/i.test(navigator.userAgent) ? '⌘' : 'Ctrl+',
+  searchPlaceholder: sprintf(s__('MergeRequest|Filter files or search with %{modifier_key}+p'), {
+    modifier_key: /Mac/i.test(navigator.userAgent) ? 'cmd' : 'ctrl',
   }),
 };
 </script>
@@ -93,6 +97,7 @@ export default {
           :file="file"
           :level="0"
           :hide-extra-on-tree="true"
+          :extra-component="fileRowExtraComponent"
           :show-changed-icon="true"
           @toggleTreeOpen="toggleTreeOpen"
           @clickFile="scrollToFile"

app/assets/javascripts/vue_shared/components/changed_file_icon.vue

@@ -36,17 +36,12 @@ export default {
       required: false,
       default: true,
     },
-    showChangedStatus: {
-      type: Boolean,
-      required: false,
-      default: false,
-    },
   },
   computed: {
     changedIcon() {
       // False positive i18n lint: https://gitlab.com/gitlab-org/frontend/eslint-plugin-i18n/issues/26
       // eslint-disable-next-line @gitlab/i18n/no-non-i18n-strings
-      const suffix = this.showStagedIcon ? '-solid' : '';
+      const suffix = !this.file.changed && this.file.staged && this.showStagedIcon ? '-solid' : '';
 
       return `${getCommitIconMap(this.file).icon}${suffix}`;
     },
@@ -91,8 +86,8 @@ export default {
   <span
     v-gl-tooltip.right
     :title="tooltipTitle"
-    :class="[{ 'ml-auto': isCentered }, changedIconClass]"
-    class="file-changed-icon d-flex align-items-center "
+    :class="{ 'ml-auto': isCentered }"
+    class="file-changed-icon d-inline-block"
   >
     <icon v-if="showIcon" :name="changedIcon" :size="size" :class="changedIconClass" />
   </span>

app/assets/javascripts/vue_shared/components/file_row.vue

 <script>
+import Icon from '~/vue_shared/components/icon.vue';
 import FileHeader from '~/vue_shared/components/file_row_header.vue';
 import FileIcon from '~/vue_shared/components/file_icon.vue';
 import ChangedFileIcon from '~/vue_shared/components/changed_file_icon.vue';
@@ -8,6 +9,7 @@ export default {
   components: {
     FileHeader,
     FileIcon,
+    Icon,
     ChangedFileIcon,
   },
   props: {
@@ -24,7 +26,6 @@ export default {
       required: false,
       default: null,
     },
-
     hideExtraOnTree: {
       type: Boolean,
       required: false,
@@ -142,17 +143,17 @@ export default {
       @mouseleave="toggleDropdown(false)"
     >
       <div class="file-row-name-container">
-        <span ref="textOutput" :style="levelIndentation" class="file-row-name str-truncated d-flex">
+        <span ref="textOutput" :style="levelIndentation" class="file-row-name str-truncated">
           <file-icon
             v-if="!showChangedIcon || file.type === 'tree'"
-            class="file-row-icon text-secondary mr-1"
+            class="file-row-icon"
             :file-name="file.name"
             :loading="file.loading"
             :folder="isTree"
             :opened="file.opened"
             :size="16"
           />
-          <file-icon v-else :file-name="file.name" :size="16" css-classes="top mr-1" />
+          <changed-file-icon v-else :file="file" :size="16" class="append-right-5" />
           {{ file.name }}
         </span>
         <component
@@ -162,7 +163,6 @@ export default {
           :dropdown-open="dropdownOpen"
           @toggle="toggleDropdown($event)"
         />
-        <changed-file-icon :file="file" :size="16" class="append-right-5" />
       </div>
     </div>
     <template v-if="file.opened || file.isHeader">
@@ -172,6 +172,7 @@ export default {
         :file="childFile"
         :level="childFilesLevel"
         :hide-extra-on-tree="hideExtraOnTree"
+        :extra-component="extraComponent"
         :show-changed-icon="showChangedIcon"
         @toggleTreeOpen="toggleTreeOpen"
         @clickFile="clickedFile"

app/assets/stylesheets/pages/diff.scss

@@ -14,9 +14,9 @@
     cursor: pointer;
 
     @media (min-width: map-get($grid-breakpoints, md)) {
-      // The `+11` is to ensure the file header border shows when scrolled -
+      // The `-1` below is to prevent two borders from clashing up against eachother -
       // the bottom of the compare-versions header and the top of the file header
-      $mr-file-header-top: $mr-version-controls-height + $header-height + $mr-tabs-height + 11;
+      $mr-file-header-top: $mr-version-controls-height + $header-height + $mr-tabs-height - 1;
 
       position: -webkit-sticky;
       position: sticky;
@@ -552,7 +552,7 @@ table.code {
 
 .diff-stats {
   align-items: center;
-  padding: 0 1rem;
+  padding: 0 0.25rem;
 
   .diff-stats-group {
     padding: 0 0.25rem;
@@ -564,7 +564,7 @@ table.code {
 
   &.is-compare-versions-header {
     .diff-stats-group {
-      padding: 0 0.25rem;
+      padding: 0 0.5rem;
     }
   }
 }
@@ -1059,8 +1059,8 @@ table.code {
 .diff-tree-list {
   position: -webkit-sticky;
   position: sticky;
-  $top-pos: $header-height + $mr-tabs-height + $mr-version-controls-height + 11px;
-  top: $header-height + $mr-tabs-height + $mr-version-controls-height + 11px;
+  $top-pos: $header-height + $mr-tabs-height + $mr-version-controls-height + 10px;
+  top: $header-height + $mr-tabs-height + $mr-version-controls-height + 10px;
   max-height: calc(100vh - #{$top-pos});
   z-index: 202;
 
@@ -1097,7 +1097,10 @@ table.code {
 
 .tree-list-scroll {
   max-height: 100%;
+  padding-top: $grid-size;
   padding-bottom: $grid-size;
+  border-top: 1px solid $border-color;
+  border-bottom: 1px solid $border-color;
   overflow-y: scroll;
   overflow-x: auto;
 }

app/assets/stylesheets/pages/merge_requests.scss

@@ -708,7 +708,7 @@
 .mr-version-controls {
   position: relative;
   z-index: 203;
-  background: $white-light;
+  background: $gray-light;
   color: $gl-text-color;
   margin-top: -1px;
 
@@ -732,7 +732,7 @@
   }
 
   .content-block {
-    padding: $gl-padding;
+    padding: $gl-padding-top $gl-padding;
     border-bottom: 0;
   }
 

changelogs/unreleased/118442-restyle-changes-header-and-file-tree.yml

----
-title: Restyle changes header & file tree
-merge_request: 22364
-author:
-type: changed

locale/gitlab.pot

@@ -8097,6 +8097,11 @@ msgstr ""
 msgid "Fetching licenses failed. You are not permitted to perform this action."
 msgstr ""
 
+msgid "File"
+msgid_plural "Files"
+msgstr[0] ""
+msgstr[1] ""
+
 msgid "File Hooks"
 msgstr ""
 
@@ -11680,10 +11685,10 @@ msgstr ""
 msgid "MergeRequest|Error loading full diff. Please try again."
 msgstr ""
 
-msgid "MergeRequest|No files found"
+msgid "MergeRequest|Filter files or search with %{modifier_key}+p"
 msgstr ""
 
-msgid "MergeRequest|Search files (%{modifier_key}P)"
+msgid "MergeRequest|No files found"
 msgstr ""
 
 msgid "Merged"
@@ -21778,9 +21783,6 @@ msgstr ""
 msgid "among other things"
 msgstr ""
 
-msgid "and"
-msgstr ""
-
 msgid "any-approver for the merge request already exists"
 msgstr ""
 
@@ -22220,11 +22222,6 @@ msgstr ""
 msgid "failed to dismiss associated finding(id=%{finding_id}): %{message}"
 msgstr ""
 
-msgid "file"
-msgid_plural "files"
-msgstr[0] ""
-msgstr[1] ""
-
 msgid "finding is not found or is already attached to a vulnerability"
 msgstr ""
 

spec/features/merge_request/user_sees_versions_spec.rb

@@ -50,7 +50,7 @@ describe 'Merge request > User sees versions', :js do
         expect(page).to have_content 'latest version'
       end
 
-      expect(page).to have_content '8 files'
+      expect(page).to have_content '8 Files'
     end
 
     it_behaves_like 'allows commenting',
@@ -84,7 +84,7 @@ describe 'Merge request > User sees versions', :js do
     end
 
     it 'shows comments that were last relevant at that version' do
-      expect(page).to have_content '5 files'
+      expect(page).to have_content '5 Files'
 
       position = Gitlab::Diff::Position.new(
         old_path: ".gitmodules",
@@ -128,10 +128,12 @@ describe 'Merge request > User sees versions', :js do
         diff_id: merge_request_diff3.id,
         start_sha: '6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9'
       )
-      expect(page).to have_content '4 files'
+      expect(page).to have_content '4 Files'
 
-      additions_content = page.find('.diff-stats.is-compare-versions-header .diff-stats-group .js-file-addition-line').text
-      deletions_content = page.find('.diff-stats.is-compare-versions-header .diff-stats-group .js-file-deletion-line').text
+      additions_content = page.find('.diff-stats.is-compare-versions-header .diff-stats-group svg.ic-file-addition')
+                              .ancestor('.diff-stats-group').text
+      deletions_content = page.find('.diff-stats.is-compare-versions-header .diff-stats-group svg.ic-file-deletion')
+                              .ancestor('.diff-stats-group').text
 
       expect(additions_content).to eq '15'
       expect(deletions_content).to eq '6'
@@ -154,10 +156,12 @@ describe 'Merge request > User sees versions', :js do
     end
 
     it 'show diff between new and old version' do
-      additions_content = page.find('.diff-stats.is-compare-versions-header .diff-stats-group .js-file-addition-line').text
-      deletions_content = page.find('.diff-stats.is-compare-versions-header .diff-stats-group .js-file-deletion-line').text
+      additions_content = page.find('.diff-stats.is-compare-versions-header .diff-stats-group svg.ic-file-addition')
+                              .ancestor('.diff-stats-group').text
+      deletions_content = page.find('.diff-stats.is-compare-versions-header .diff-stats-group svg.ic-file-deletion')
+                              .ancestor('.diff-stats-group').text
 
-      expect(page).to have_content '4 files'
+      expect(page).to have_content '4 Files'
       expect(additions_content).to eq '15'
       expect(deletions_content).to eq '6'
     end
@@ -167,7 +171,7 @@ describe 'Merge request > User sees versions', :js do
       page.within '.mr-version-dropdown' do
         expect(page).to have_content 'latest version'
       end
-      expect(page).to have_content '8 files'
+      expect(page).to have_content '8 Files'
     end
 
     it_behaves_like 'allows commenting',
@@ -193,7 +197,7 @@ describe 'Merge request > User sees versions', :js do
         find('.btn-default').click
         click_link 'version 1'
       end
-      expect(page).to have_content '0 files'
+      expect(page).to have_content '0 Files'
     end
   end
 
@@ -219,7 +223,7 @@ describe 'Merge request > User sees versions', :js do
         expect(page).to have_content 'version 1'
       end
 
-      expect(page).to have_content '0 files'
+      expect(page).to have_content '0 Files'
     end
   end
 

spec/frontend/diffs/components/compare_versions_spec.js

@@ -49,7 +49,8 @@ describe('CompareVersions', () => {
 
       expect(treeListBtn.exists()).toBe(true);
       expect(treeListBtn.attributes('title')).toBe('Hide file browser');
-      expect(treeListBtn.find(Icon).props('name')).toBe('file-tree');
+      expect(treeListBtn.findAll(Icon).length).not.toBe(0);
+      expect(treeListBtn.find(Icon).props('name')).toBe('collapse-left');
     });
 
     it('should render comparison dropdowns with correct values', () => {

spec/frontend/diffs/components/diff_stats_spec.js

 import { shallowMount } from '@vue/test-utils';
+import Icon from '~/vue_shared/components/icon.vue';
 import DiffStats from '~/diffs/components/diff_stats.vue';
 
 describe('diff_stats', () => {
@@ -23,11 +24,18 @@ describe('diff_stats', () => {
       },
     });
 
-    const findFileLine = name => wrapper.find(name);
-    const additions = findFileLine('.js-file-addition-line');
-    const deletions = findFileLine('.js-file-deletion-line');
+    const findIcon = name =>
+      wrapper
+        .findAll(Icon)
+        .filter(c => c.attributes('name') === name)
+        .at(0).element.parentNode;
 
-    expect(additions.text()).toBe('100');
-    expect(deletions.text()).toBe('200');
+    const additions = findIcon('file-addition');
+    const deletions = findIcon('file-deletion');
+    const filesChanged = findIcon('doc-code');
+
+    expect(additions.textContent).toContain('100');
+    expect(deletions.textContent).toContain('200');
+    expect(filesChanged.textContent).toContain('300');
   });
 });

spec/frontend/vue_shared/components/changed_file_icon_spec.js

@@ -56,10 +56,10 @@ describe('Changed file icon', () => {
 
   describe.each`
     file                      | iconName                 | tooltipText                           | desc
-    ${changedFile()}          | ${'file-modified-solid'} | ${'Unstaged modification'}            | ${'with file changed'}
+    ${changedFile()}          | ${'file-modified'}       | ${'Unstaged modification'}            | ${'with file changed'}
     ${stagedFile()}           | ${'file-modified-solid'} | ${'Staged modification'}              | ${'with file staged'}
-    ${changedAndStagedFile()} | ${'file-modified-solid'} | ${'Unstaged and staged modification'} | ${'with file changed and staged'}
-    ${newFile()}              | ${'file-addition-solid'} | ${'Unstaged addition'}                | ${'with file new'}
+    ${changedAndStagedFile()} | ${'file-modified'}       | ${'Unstaged and staged modification'} | ${'with file changed and staged'}
+    ${newFile()}              | ${'file-addition'}       | ${'Unstaged addition'}                | ${'with file new'}
   `('$desc', ({ file, iconName, tooltipText }) => {
     beforeEach(() => {
       factory({ file });

spec/javascripts/ide/components/repo_tab_spec.js

@@ -93,13 +93,13 @@ describe('RepoTab', () => {
 
     Vue.nextTick()
       .then(() => {
-        expect(vm.$el.querySelector('.file-modified-solid')).toBeNull();
+        expect(vm.$el.querySelector('.file-modified')).toBeNull();
 
         vm.$el.dispatchEvent(new Event('mouseout'));
       })
       .then(Vue.nextTick)
       .then(() => {
-        expect(vm.$el.querySelector('.file-modified-solid')).not.toBeNull();
+        expect(vm.$el.querySelector('.file-modified')).not.toBeNull();
 
         done();
       })