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

Restyle changes header & file tree

Closes #118442

See merge request gitlab-org/gitlab!23665

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 prepend-top-default"
+        class="files d-flex"
       >
         <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';
@@ -63,9 +62,6 @@ 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');
     },
@@ -91,7 +87,7 @@ export default {
 </script>
 
 <template>
-  <div class="mr-version-controls border-top border-bottom">
+  <div class="mr-version-controls border-top">
     <div
       class="mr-version-menus-container content-block"
       :class="{
@@ -108,17 +104,17 @@ export default {
         :title="toggleFileBrowserTitle"
         @click="toggleShowTreeList"
       >
-        <icon :name="fileTreeIcon" />
+        <icon name="file-tree" />
       </button>
       <div v-if="showDropdowns" class="d-flex align-items-center compare-versions-container">
-        Changes between
+        {{ __('Compare') }}
         <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

@@ -224,7 +224,7 @@ export default {
 
     <div
       v-if="!diffFile.submodule && addMergeRequestButtons"
-      class="file-actions d-none d-sm-block"
+      class="file-actions d-none d-sm-flex align-items-center flex-wrap"
     >
       <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

@@ -22,7 +22,7 @@ export default {
   },
   computed: {
     filesText() {
-      return n__('File', 'Files', this.diffFilesLength);
+      return n__('file', 'files', this.diffFilesLength);
     },
     isCompareVersionsHeader() {
       return Boolean(this.diffFilesLength);
@@ -44,13 +44,21 @@ export default {
   >
     <div v-if="hasDiffFiles" class="diff-stats-group">
       <icon name="doc-code" class="diff-stats-icon text-secondary" />
-      <strong>{{ diffFilesLength }} {{ filesText }}</strong>
+      <span class="text-secondary bold">{{ diffFilesLength }} {{ filesText }}</span>
     </div>
-    <div class="diff-stats-group cgreen">
-      <icon name="file-addition" class="diff-stats-icon" /> <strong>{{ addedLines }}</strong>
+    <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>
-    <div class="diff-stats-group cred">
-      <icon name="file-deletion" class="diff-stats-icon" /> <strong>{{ removedLines }}</strong>
+    <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>
   </div>
 </template>

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

@@ -58,8 +58,8 @@ export default {
       this.search = '';
     },
   },
-  searchPlaceholder: sprintf(s__('MergeRequest|Filter files or search with %{modifier_key}+p'), {
-    modifier_key: /Mac/i.test(navigator.userAgent) ? 'cmd' : 'ctrl',
+  searchPlaceholder: sprintf(s__('MergeRequest|Search files (%{modifier_key}P)'), {
+    modifier_key: /Mac/i.test(navigator.userAgent) ? '⌘' : 'Ctrl+',
   }),
 };
 </script>

app/assets/stylesheets/pages/diff.scss

@@ -14,9 +14,9 @@
     cursor: pointer;
 
     @media (min-width: map-get($grid-breakpoints, md)) {
-      // The `-1` below is to prevent two borders from clashing up against eachother -
+      // The `+11` is to ensure the file header border shows when scrolled -
       // 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 - 1;
+      $mr-file-header-top: $mr-version-controls-height + $header-height + $mr-tabs-height + 11;
 
       position: -webkit-sticky;
       position: sticky;
@@ -547,7 +547,7 @@ table.code {
 
 .diff-stats {
   align-items: center;
-  padding: 0 0.25rem;
+  padding: 0 1rem;
 
   .diff-stats-group {
     padding: 0 0.25rem;
@@ -559,7 +559,7 @@ table.code {
 
   &.is-compare-versions-header {
     .diff-stats-group {
-      padding: 0 0.5rem;
+      padding: 0 0.25rem;
     }
   }
 }
@@ -1054,8 +1054,8 @@ table.code {
 .diff-tree-list {
   position: -webkit-sticky;
   position: sticky;
-  $top-pos: $header-height + $mr-tabs-height + $mr-version-controls-height + 10px;
-  top: $header-height + $mr-tabs-height + $mr-version-controls-height + 10px;
+  $top-pos: $header-height + $mr-tabs-height + $mr-version-controls-height + 11px;
+  top: $top-pos;
   max-height: calc(100vh - #{$top-pos});
   z-index: 202;
 
@@ -1092,10 +1092,7 @@ 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: $gray-light;
+  background: $white-light;
   color: $gl-text-color;
   margin-top: -1px;
 
@@ -732,7 +732,7 @@
   }
 
   .content-block {
-    padding: $gl-padding-top $gl-padding;
+    padding: $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

@@ -8292,11 +8292,6 @@ 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 ""
 
@@ -11931,10 +11926,10 @@ msgstr ""
 msgid "MergeRequest|Error loading full diff. Please try again."
 msgstr ""
 
-msgid "MergeRequest|Filter files or search with %{modifier_key}+p"
+msgid "MergeRequest|No files found"
 msgstr ""
 
-msgid "MergeRequest|No files found"
+msgid "MergeRequest|Search files (%{modifier_key}P)"
 msgstr ""
 
 msgid "Merged"
@@ -22185,6 +22180,9 @@ msgstr ""
 msgid "among other things"
 msgstr ""
 
+msgid "and"
+msgstr ""
+
 msgid "any-approver for the merge request already exists"
 msgstr ""
 
@@ -22615,6 +22613,11 @@ 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,12 +128,10 @@ 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 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
+      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
 
       expect(additions_content).to eq '15'
       expect(deletions_content).to eq '6'
@@ -156,12 +154,10 @@ 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 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
+      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
 
-      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
@@ -171,7 +167,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',
@@ -197,7 +193,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
 
@@ -223,7 +219,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,8 +49,7 @@ describe('CompareVersions', () => {
 
       expect(treeListBtn.exists()).toBe(true);
       expect(treeListBtn.attributes('title')).toBe('Hide file browser');
-      expect(treeListBtn.findAll(Icon).length).not.toBe(0);
-      expect(treeListBtn.find(Icon).props('name')).toBe('collapse-left');
+      expect(treeListBtn.find(Icon).props('name')).toBe('file-tree');
     });
 
     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';
+import Icon from '~/vue_shared/components/icon.vue';
 
 describe('diff_stats', () => {
   it('does not render a group if diffFileLengths is empty', () => {
@@ -37,18 +37,18 @@ describe('diff_stats', () => {
       },
     });
 
+    const findFileLine = name => wrapper.find(name);
     const findIcon = name =>
       wrapper
         .findAll(Icon)
         .filter(c => c.attributes('name') === name)
         .at(0).element.parentNode;
-
-    const additions = findIcon('file-addition');
-    const deletions = findIcon('file-deletion');
+    const additions = findFileLine('.js-file-addition-line');
+    const deletions = findFileLine('.js-file-deletion-line');
     const filesChanged = findIcon('doc-code');
 
-    expect(additions.textContent).toContain('100');
-    expect(deletions.textContent).toContain('200');
+    expect(additions.text()).toBe('100');
+    expect(deletions.text()).toBe('200');
     expect(filesChanged.textContent).toContain('300');
   });
 });