Merge branch '32145-show-the-size-of-the-files-in-image-diff' into 'master'

Resolve "Show the size of the files in image diff"

See merge request gitlab-org/gitlab!19710

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

@@ -124,8 +124,10 @@ export default {
         :diff-viewer-mode="diffViewerMode"
         :new-path="diffFile.new_path"
         :new-sha="diffFile.diff_refs.head_sha"
+        :new-size="diffFile.new_size"
         :old-path="diffFile.old_path"
         :old-sha="diffFile.diff_refs.base_sha"
+        :old-size="diffFile.old_size"
         :file-hash="diffFileHash"
         :project-path="projectPath"
         :a-mode="diffFile.a_mode"

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

@@ -23,6 +23,11 @@ export default {
       type: String,
       required: true,
     },
+    newSize: {
+      type: Number,
+      required: false,
+      default: 0,
+    },
     oldPath: {
       type: String,
       required: true,
@@ -31,6 +36,11 @@ export default {
       type: String,
       required: true,
     },
+    oldSize: {
+      type: Number,
+      required: false,
+      default: 0,
+    },
     projectPath: {
       type: String,
       required: false,
@@ -85,6 +95,8 @@ export default {
       :diff-mode="diffMode"
       :new-path="fullNewPath"
       :old-path="fullOldPath"
+      :old-size="oldSize"
+      :new-size="newSize"
       :project-path="projectPath"
       :a-mode="aMode"
       :b-mode="bMode"

app/assets/javascripts/vue_shared/components/diff_viewer/viewers/image_diff/two_up_viewer.vue

@@ -14,6 +14,16 @@ export default {
       type: String,
       required: true,
     },
+    newSize: {
+      type: Number,
+      required: false,
+      default: 0,
+    },
+    oldSize: {
+      type: Number,
+      required: false,
+      default: 0,
+    },
   },
 };
 </script>
@@ -22,12 +32,14 @@ export default {
   <div class="two-up view d-flex">
     <image-viewer
       :path="oldPath"
+      :file-size="oldSize"
       :render-info="true"
       inner-css-classes="frame deleted"
       class="wrap w-50"
     />
     <image-viewer
       :path="newPath"
+      :file-size="newSize"
       :render-info="true"
       :inner-css-classes="['frame', 'added']"
       class="wrap w-50"

app/assets/javascripts/vue_shared/components/diff_viewer/viewers/image_diff_viewer.vue

@@ -22,6 +22,16 @@ export default {
       type: String,
       required: true,
     },
+    newSize: {
+      type: Number,
+      required: false,
+      default: 0,
+    },
+    oldSize: {
+      type: Number,
+      required: false,
+      default: 0,
+    },
   },
   data() {
     return {

changelogs/unreleased/defect-diff-file-size.yml

+---
+title: Re-add missing file sizes in 2-Up diff file viewer
+merge_request: 19710
+author:
+type: fixed

spec/frontend/vue_shared/components/content_viewer/viewers/image_viewer_spec.js

+import { shallowMount } from '@vue/test-utils';
+
+import ImageViewer from '~/vue_shared/components/content_viewer/viewers/image_viewer.vue';
+import { GREEN_BOX_IMAGE_URL } from 'spec/test_constants';
+
+describe('Image Viewer', () => {
+  const requiredProps = {
+    path: GREEN_BOX_IMAGE_URL,
+    renderInfo: true,
+  };
+  let wrapper;
+  let imageInfo;
+
+  function createElement({ props, includeRequired = true } = {}) {
+    const data = includeRequired ? { ...requiredProps, ...props } : { ...props };
+
+    wrapper = shallowMount(ImageViewer, {
+      propsData: data,
+    });
+    imageInfo = wrapper.find('.image-info');
+  }
+
+  describe('file sizes', () => {
+    it('should show the humanized file size when `renderInfo` is true and there is size info', () => {
+      createElement({ props: { fileSize: 1024 } });
+
+      expect(imageInfo.text()).toContain('1.00 KiB');
+    });
+
+    it('should not show the humanized file size when `renderInfo` is true and there is no size', () => {
+      const FILESIZE_RE = /\d+(\.\d+)?\s*([KMGTP]i)*B/;
+
+      createElement({ props: { fileSize: 0 } });
+
+      // It shouldn't show any filesize info
+      expect(imageInfo.text()).not.toMatch(FILESIZE_RE);
+    });
+
+    it('should not show any image information when `renderInfo` is false', () => {
+      createElement({ props: { renderInfo: false } });
+
+      expect(imageInfo.exists()).toBe(false);
+    });
+  });
+});

spec/javascripts/vue_shared/components/diff_viewer/diff_viewer_spec.js

 import Vue from 'vue';
+
 import diffViewer from '~/vue_shared/components/diff_viewer/diff_viewer.vue';
 import mountComponent from 'spec/helpers/vue_mount_component_helper';
 import { GREEN_BOX_IMAGE_URL, RED_BOX_IMAGE_URL } from 'spec/test_constants';
 
 describe('DiffViewer', () => {
+  const requiredProps = {
+    diffMode: 'replaced',
+    diffViewerMode: 'image',
+    newPath: GREEN_BOX_IMAGE_URL,
+    newSha: 'ABC',
+    oldPath: RED_BOX_IMAGE_URL,
+    oldSha: 'DEF',
+  };
   let vm;
 
   function createComponent(props) {
     const DiffViewer = Vue.extend(diffViewer);
+
     vm = mountComponent(DiffViewer, props);
   }
 
@@ -20,15 +30,11 @@ describe('DiffViewer', () => {
       relative_url_root: '',
     };
 
-    createComponent({
-      diffMode: 'replaced',
-      diffViewerMode: 'image',
-      newPath: GREEN_BOX_IMAGE_URL,
-      newSha: 'ABC',
-      oldPath: RED_BOX_IMAGE_URL,
-      oldSha: 'DEF',
-      projectPath: '',
-    });
+    createComponent(
+      Object.assign({}, requiredProps, {
+        projectPath: '',
+      }),
+    );
 
     setTimeout(() => {
       expect(vm.$el.querySelector('.deleted img').getAttribute('src')).toBe(
@@ -44,14 +50,13 @@ describe('DiffViewer', () => {
   });
 
   it('renders fallback download diff display', done => {
-    createComponent({
-      diffMode: 'replaced',
-      diffViewerMode: 'added',
-      newPath: 'test.abc',
-      newSha: 'ABC',
-      oldPath: 'testold.abc',
-      oldSha: 'DEF',
-    });
+    createComponent(
+      Object.assign({}, requiredProps, {
+        diffViewerMode: 'added',
+        newPath: 'test.abc',
+        oldPath: 'testold.abc',
+      }),
+    );
 
     setTimeout(() => {
       expect(vm.$el.querySelector('.deleted .file-info').textContent.trim()).toContain(
@@ -72,29 +77,28 @@ describe('DiffViewer', () => {
   });
 
   it('renders renamed component', () => {
-    createComponent({
-      diffMode: 'renamed',
-      diffViewerMode: 'renamed',
-      newPath: 'test.abc',
-      newSha: 'ABC',
-      oldPath: 'testold.abc',
-      oldSha: 'DEF',
-    });
+    createComponent(
+      Object.assign({}, requiredProps, {
+        diffMode: 'renamed',
+        diffViewerMode: 'renamed',
+        newPath: 'test.abc',
+        oldPath: 'testold.abc',
+      }),
+    );
 
     expect(vm.$el.textContent).toContain('File moved');
   });
 
   it('renders mode changed component', () => {
-    createComponent({
-      diffMode: 'mode_changed',
-      diffViewerMode: 'image',
-      newPath: 'test.abc',
-      newSha: 'ABC',
-      oldPath: 'testold.abc',
-      oldSha: 'DEF',
-      aMode: '123',
-      bMode: '321',
-    });
+    createComponent(
+      Object.assign({}, requiredProps, {
+        diffMode: 'mode_changed',
+        newPath: 'test.abc',
+        oldPath: 'testold.abc',
+        aMode: '123',
+        bMode: '321',
+      }),
+    );
 
     expect(vm.$el.textContent).toContain('File mode changed from 123 to 321');
   });

spec/javascripts/vue_shared/components/diff_viewer/viewers/image_diff_viewer_spec.js

@@ -4,6 +4,11 @@ import mountComponent from 'spec/helpers/vue_mount_component_helper';
 import { GREEN_BOX_IMAGE_URL, RED_BOX_IMAGE_URL } from 'spec/test_constants';
 
 describe('ImageDiffViewer', () => {
+  const requiredProps = {
+    diffMode: 'replaced',
+    newPath: GREEN_BOX_IMAGE_URL,
+    oldPath: RED_BOX_IMAGE_URL,
+  };
   let vm;
 
   function createComponent(props) {
@@ -45,11 +50,7 @@ describe('ImageDiffViewer', () => {
   });
 
   it('renders image diff for replaced', done => {
-    createComponent({
-      diffMode: 'replaced',
-      newPath: GREEN_BOX_IMAGE_URL,
-      oldPath: RED_BOX_IMAGE_URL,
-    });
+    createComponent(requiredProps);
 
     setTimeout(() => {
       expect(vm.$el.querySelector('.added img').getAttribute('src')).toBe(GREEN_BOX_IMAGE_URL);
@@ -70,11 +71,12 @@ describe('ImageDiffViewer', () => {
   });
 
   it('renders image diff for new', done => {
-    createComponent({
-      diffMode: 'new',
-      newPath: GREEN_BOX_IMAGE_URL,
-      oldPath: '',
-    });
+    createComponent(
+      Object.assign({}, requiredProps, {
+        diffMode: 'new',
+        oldPath: '',
+      }),
+    );
 
     setTimeout(() => {
       expect(vm.$el.querySelector('.added img').getAttribute('src')).toBe(GREEN_BOX_IMAGE_URL);
@@ -84,11 +86,12 @@ describe('ImageDiffViewer', () => {
   });
 
   it('renders image diff for deleted', done => {
-    createComponent({
-      diffMode: 'deleted',
-      newPath: '',
-      oldPath: RED_BOX_IMAGE_URL,
-    });
+    createComponent(
+      Object.assign({}, requiredProps, {
+        diffMode: 'deleted',
+        newPath: '',
+      }),
+    );
 
     setTimeout(() => {
       expect(vm.$el.querySelector('.deleted img').getAttribute('src')).toBe(RED_BOX_IMAGE_URL);
@@ -119,11 +122,7 @@ describe('ImageDiffViewer', () => {
 
   describe('swipeMode', () => {
     beforeEach(done => {
-      createComponent({
-        diffMode: 'replaced',
-        newPath: GREEN_BOX_IMAGE_URL,
-        oldPath: RED_BOX_IMAGE_URL,
-      });
+      createComponent(requiredProps);
 
       setTimeout(() => {
         done();
@@ -142,11 +141,7 @@ describe('ImageDiffViewer', () => {
 
   describe('onionSkin', () => {
     beforeEach(done => {
-      createComponent({
-        diffMode: 'replaced',
-        newPath: GREEN_BOX_IMAGE_URL,
-        oldPath: RED_BOX_IMAGE_URL,
-      });
+      createComponent(requiredProps);
 
       setTimeout(() => {
         done();