diff options
author | Phil Hughes <me@iamphill.com> | 2019-04-23 10:25:07 +0300 |
---|---|---|
committer | Phil Hughes <me@iamphill.com> | 2019-04-23 10:25:07 +0300 |
commit | cdada210286176b7b59ed8c1296ecac01b81b89b (patch) | |
tree | fdac1ec427ba701f97d94d59e09c9bb9a70d3eac | |
parent | dfb2c3784e06684e3d76b0f9b30906ef086b938a (diff) | |
parent | 3dfd2451b0dfae67f14e338e61c8415fcb0e5d08 (diff) |
Merge branch '58850-fix-misplaced-swipe-view' into 'master'
Make swipe view images line up
Closes #58850
See merge request gitlab-org/gitlab-ce!26969
4 files changed, 116 insertions, 30 deletions
diff --git a/app/assets/javascripts/vue_shared/components/diff_viewer/viewers/image_diff/swipe_viewer.vue b/app/assets/javascripts/vue_shared/components/diff_viewer/viewers/image_diff/swipe_viewer.vue index ad3b3b81ac5..8d77b156aa4 100644 --- a/app/assets/javascripts/vue_shared/components/diff_viewer/viewers/image_diff/swipe_viewer.vue +++ b/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" diff --git a/changelogs/unreleased/58850-fix-misplaced-swipe-view-26969.yml b/changelogs/unreleased/58850-fix-misplaced-swipe-view-26969.yml new file mode 100644 index 00000000000..fa3e81df4e0 --- /dev/null +++ b/changelogs/unreleased/58850-fix-misplaced-swipe-view-26969.yml @@ -0,0 +1,5 @@ +--- +title: "Fix misaligned image diff swipe view" +merge_request: 26969 +author: ftab +type: fixed diff --git a/spec/features/merge_request/user_creates_image_diff_notes_spec.rb b/spec/features/merge_request/user_creates_image_diff_notes_spec.rb index 65de0afae0c..5db54f42264 100644 --- a/spec/features/merge_request/user_creates_image_diff_notes_spec.rb +++ b/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 diff --git a/spec/javascripts/vue_shared/components/diff_viewer/viewers/image_diff_viewer_spec.js b/spec/javascripts/vue_shared/components/diff_viewer/viewers/image_diff_viewer_spec.js index 7f2e246d656..97c870f27d9 100644 --- a/spec/javascripts/vue_shared/components/diff_viewer/viewers/image_diff_viewer_spec.js +++ b/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', () => { |