Automatic merge of gitlab-org/gitlab-ce master

app/assets/javascripts/ide/components/new_dropdown/upload.vue

@@ -57,6 +57,8 @@ export default {
         type: 'blob',
         content: result,
         base64: !isText,
+        binary: !isText,
+        rawPath: !isText ? target.result : '',
       });
     },
     readFile(file) {

app/assets/javascripts/ide/components/repo_editor.vue

 <script>
 import { mapState, mapGetters, mapActions } from 'vuex';
+import { viewerInformationForPath } from '~/vue_shared/components/content_viewer/lib/viewer_utils';
 import flash from '~/flash';
 import ContentViewer from '~/vue_shared/components/content_viewer/content_viewer.vue';
 import DiffViewer from '~/vue_shared/components/diff_viewer/diff_viewer.vue';
@@ -56,6 +57,10 @@ export default {
         active: this.file.viewMode === 'preview',
       };
     },
+    fileType() {
+      const info = viewerInformationForPath(this.file.path);
+      return (info && info.id) || '';
+    },
   },
   watch: {
     file(newVal, oldVal) {
@@ -258,6 +263,7 @@ export default {
       :path="file.rawPath || file.path"
       :file-size="file.size"
       :project-path="file.projectId"
+      :type="fileType"
     />
     <diff-viewer
       v-if="showDiffViewer"

app/assets/javascripts/ide/lib/files.js

@@ -22,6 +22,8 @@ export const decorateFiles = ({
   tempFile = false,
   content = '',
   base64 = false,
+  binary = false,
+  rawPath = '',
 }) => {
   const treeList = [];
   const entries = {};
@@ -90,6 +92,8 @@ export const decorateFiles = ({
         changed: tempFile,
         content,
         base64,
+        binary,
+        rawPath,
         previewMode: viewerInformationForPath(name),
         parentPath,
       });

app/assets/javascripts/ide/stores/actions.js

@@ -53,7 +53,7 @@ export const setResizingStatus = ({ commit }, resizing) => {
 
 export const createTempEntry = (
   { state, commit, dispatch },
-  { name, type, content = '', base64 = false },
+  { name, type, content = '', base64 = false, binary = false, rawPath = '' },
 ) =>
   new Promise(resolve => {
     const fullName = name.slice(-1) !== '/' && type === 'tree' ? `${name}/` : name;
@@ -79,8 +79,10 @@ export const createTempEntry = (
       branchId: state.currentBranchId,
       type,
       tempFile: true,
-      base64,
       content,
+      base64,
+      binary,
+      rawPath,
     });
     const { file, parentPath } = data;
 

app/assets/javascripts/ide/stores/utils.js

@@ -69,6 +69,8 @@ export const decorateData = entity => {
     changed = false,
     parentTreeUrl = '',
     base64 = false,
+    binary = false,
+    rawPath = '',
     previewMode,
     file_lock,
     html,
@@ -92,6 +94,8 @@ export const decorateData = entity => {
     renderError,
     content,
     base64,
+    binary,
+    rawPath,
     previewMode,
     file_lock,
     html,

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

 <script>
-import { viewerInformationForPath } from './lib/viewer_utils';
 import MarkdownViewer from './viewers/markdown_viewer.vue';
 import ImageViewer from './viewers/image_viewer.vue';
 import DownloadViewer from './viewers/download_viewer.vue';
@@ -24,15 +23,18 @@ export default {
       required: false,
       default: '',
     },
+    type: {
+      type: String,
+      required: false,
+      default: '',
+    },
   },
   computed: {
     viewer() {
       if (!this.path) return null;
+      if (!this.type) return DownloadViewer;
 
-      const previewInfo = viewerInformationForPath(this.path);
-      if (!previewInfo) return DownloadViewer;
-
-      switch (previewInfo.id) {
+      switch (this.type) {
         case 'markdown':
           return MarkdownViewer;
         case 'image':

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

@@ -58,12 +58,11 @@ export default {
 
       const moveX = e.pageX || e.touches[0].pageX;
       let leftValue = moveX - this.$refs.swipeFrame.getBoundingClientRect().left;
-      const spaceLeft = 20;
       const { clientWidth } = this.$refs.swipeFrame;
       if (leftValue <= 0) {
         leftValue = 0;
-      } else if (leftValue > clientWidth - spaceLeft) {
-        leftValue = clientWidth - spaceLeft;
+      } else if (leftValue > clientWidth) {
+        leftValue = clientWidth;
       }
 
       this.swipeWrapWidth = (leftValue / clientWidth) * 100;
@@ -80,7 +79,7 @@ export default {
       document.body.removeEventListener('touchmove', this.dragMove);
     },
     prepareSwipe() {
-      if (this.swipeOldImgInfo && this.swipeNewImgInfo) {
+      if (this.swipeOldImgInfo && this.swipeNewImgInfo && this.swipeOldImgInfo.renderedWidth > 0) {
         // Add 2 for border width
         this.swipeMaxWidth =
           Math.max(this.swipeOldImgInfo.renderedWidth, this.swipeNewImgInfo.renderedWidth) + 2;
@@ -101,6 +100,8 @@ export default {
     },
     resize: _.throttle(function throttledResize() {
       this.swipeBarPos = 0;
+      this.swipeWrapWidth = 0;
+      this.prepareSwipe();
     }, 400),
   },
 };
@@ -111,6 +112,8 @@ export default {
     <div
       ref="swipeFrame"
       :style="{
+        width: swipeMaxPixelWidth,
+        height: swipeMaxPixelHeight,
         'user-select': dragging ? 'none' : null,
       }"
       class="swipe-frame"

changelogs/unreleased/58850-fix-misplaced-swipe-view-26969.yml

+---
+title: "Fix misaligned image diff swipe view"
+merge_request: 26969
+author: ftab
+type: fixed

changelogs/unreleased/59514-uploading-images-base64.yml

+---
+title: Show proper preview for uploaded images in Web IDE
+merge_request: 27471
+author:
+type: fixed

spec/features/merge_request/user_creates_image_diff_notes_spec.rb

@@ -191,29 +191,119 @@ describe 'Merge request > User creates image diff notes', :js do
     end
   end
 
-  describe 'image view modes' do
-    before do
-      visit project_commit_path(project, '2f63565e7aac07bcdadb654e253078b727143ec4')
+  shared_examples 'swipe view' do
+    it 'moves the swipe handle' do
+      # Simulate dragging swipe view slider
+      expect { drag_and_drop_by(find('.swipe-bar'), 20, 0) }
+        .to change { find('.swipe-bar')['style'] }
+        .from(a_string_matching('left: 1px'))
     end
 
-    it 'resizes image in onion skin view mode' do
-      find('.view-modes-menu .onion-skin').click
+    it 'shows both images at the same position' do
+      drag_and_drop_by(find('.swipe-bar'), 40, 0)
 
-      expect(find('.onion-skin-frame')['style']).to match('width: 228px; height: 240px;')
+      expect(left_position('.frame.added img'))
+        .to eq(left_position('.frame.deleted img'))
     end
+  end
 
-    it 'resets onion skin view mode opacity when toggling between view modes' do
-      find('.view-modes-menu .onion-skin').click
-
+  shared_examples 'onion skin' do
+    it 'resets opacity when toggling between view modes' do
       # Simulate dragging onion-skin slider
       drag_and_drop_by(find('.dragger'), -30, 0)
 
       expect(find('.onion-skin-frame .frame.added', visible: false)['style']).not_to match('opacity: 1;')
 
+      switch_to_swipe_view
+      switch_to_onion_skin
+
+      expect(find('.onion-skin-frame .frame.added', visible: false)['style']).to match('opacity: 1;')
+    end
+  end
+
+  describe 'changes tab image diff' do
+    let(:merge_request) { create(:merge_request, source_project: project, target_project: project, target_branch: 'master', source_branch: 'deleted-image-test', author: user) }
+
+    before do
+      visit diffs_project_merge_request_path(project, merge_request)
+      click_link "Changes"
+    end
+
+    def set_image_diff_sources
+      # set path of added and deleted images to something the spec can view
+      page.execute_script("document.querySelector('.frame.added img').src = '/apple-touch-icon.png';")
+      page.execute_script("document.querySelector('.frame.deleted img').src = '/favicon.png';")
+
+      wait_for_requests
+
+      expect(find('.frame.added img', visible: false)['src']).to match('/apple-touch-icon.png')
+      expect(find('.frame.deleted img', visible: false)['src']).to match('/favicon.png')
+    end
+
+    def switch_to_swipe_view
+      # it isn't given the .swipe class in the merge request diff
+      find('.view-modes-menu li:nth-child(2)').click
+      expect(find('.view-modes-menu li.active')).to have_content('Swipe')
+
+      set_image_diff_sources
+    end
+
+    def switch_to_onion_skin
+      # it isn't given the .onion-skin class in the merge request diff
+      find('.view-modes-menu li:nth-child(3)').click
+      expect(find('.view-modes-menu li.active')).to have_content('Onion skin')
+
+      set_image_diff_sources
+    end
+
+    describe 'onion skin' do
+      before do
+        switch_to_onion_skin
+      end
+
+      it_behaves_like 'onion skin'
+    end
+
+    describe 'swipe view' do
+      before do
+        switch_to_swipe_view
+      end
+
+      it_behaves_like 'swipe view'
+    end
+  end
+
+  describe 'image view modes' do
+    before do
+      visit project_commit_path(project, '2f63565e7aac07bcdadb654e253078b727143ec4')
+    end
+
+    def switch_to_swipe_view
       find('.view-modes-menu .swipe').click
+    end
+
+    def switch_to_onion_skin
       find('.view-modes-menu .onion-skin').click
+    end
 
-      expect(find('.onion-skin-frame .frame.added', visible: false)['style']).to match('opacity: 1;')
+    describe 'onion skin' do
+      before do
+        switch_to_onion_skin
+      end
+
+      it 'resizes image' do
+        expect(find('.onion-skin-frame')['style']).to match('width: 228px; height: 240px;')
+      end
+
+      it_behaves_like 'onion skin'
+    end
+
+    describe 'swipe view' do
+      before do
+        switch_to_swipe_view
+      end
+
+      it_behaves_like 'swipe view'
     end
   end
 
@@ -232,4 +322,8 @@ describe 'Merge request > User creates image diff notes', :js do
     click_button 'Comment'
     wait_for_requests
   end
+
+  def left_position(element)
+    page.evaluate_script("document.querySelectorAll('#{element}')[0].getBoundingClientRect().left;")
+  end
 end

spec/javascripts/ide/components/new_dropdown/upload_spec.js

@@ -78,6 +78,8 @@ describe('new dropdown upload', () => {
         type: 'blob',
         content: 'plain text',
         base64: false,
+        binary: false,
+        rawPath: '',
       });
     });
 
@@ -89,6 +91,8 @@ describe('new dropdown upload', () => {
         type: 'blob',
         content: binaryTarget.result.split('base64,')[1],
         base64: true,
+        binary: true,
+        rawPath: binaryTarget.result,
       });
     });
   });

spec/javascripts/vue_shared/components/content_viewer/content_viewer_spec.js

@@ -4,6 +4,7 @@ import axios from '~/lib/utils/axios_utils';
 import contentViewer from '~/vue_shared/components/content_viewer/content_viewer.vue';
 import mountComponent from 'spec/helpers/vue_mount_component_helper';
 import { GREEN_BOX_IMAGE_URL } from 'spec/test_constants';
+import '~/behaviors/markdown/render_gfm';
 
 describe('ContentViewer', () => {
   let vm;
@@ -29,6 +30,7 @@ describe('ContentViewer', () => {
       path: 'test.md',
       content: '*  Test',
       projectPath: 'testproject',
+      type: 'markdown',
     });
 
     const previewContainer = vm.$el.querySelector('.md-previewer');
@@ -44,6 +46,7 @@ describe('ContentViewer', () => {
     createComponent({
       path: GREEN_BOX_IMAGE_URL,
       fileSize: 1024,
+      type: 'image',
     });
 
     setTimeout(() => {

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

@@ -138,22 +138,6 @@ describe('ImageDiffViewer', () => {
         done();
       });
     });
-
-    it('drag handler is working', done => {
-      vm.$el.querySelector('.view-modes-menu li:nth-child(2)').click();
-
-      vm.$nextTick(() => {
-        expect(vm.$el.querySelector('.swipe-bar').style.left).toBe('1px');
-        expect(vm.$el.querySelector('.top-handle')).not.toBeNull();
-
-        dragSlider(vm.$el.querySelector('.swipe-bar'), 40);
-
-        vm.$nextTick(() => {
-          expect(vm.$el.querySelector('.swipe-bar').style.left).toBe('-20px');
-          done();
-        });
-      });
-    });
   });
 
   describe('onionSkin', () => {