From 35faecb06b4b921d12a62344976918e88cd73fca Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Tue, 6 Nov 2018 09:40:03 +0000 Subject: [PATCH] Restored width & height properties --- .../diffs/components/diff_content.vue | 2 ++ .../diffs/components/image_diff_overlay.vue | 30 ++++++++++++++-- app/assets/javascripts/diffs/store/utils.js | 2 ++ lib/gitlab/diff/formatters/image_formatter.rb | 2 +- .../diffs/components/diff_content_spec.js | 10 +++++- .../components/image_diff_overlay_spec.js | 35 ++++++++++++++++--- .../diffs/mock_data/diff_discussions.js | 4 +++ 7 files changed, 75 insertions(+), 10 deletions(-) diff --git a/app/assets/javascripts/diffs/components/diff_content.vue b/app/assets/javascripts/diffs/components/diff_content.vue index f677999a268..547742a5ff4 100644 --- a/app/assets/javascripts/diffs/components/diff_content.vue +++ b/app/assets/javascripts/diffs/components/diff_content.vue @@ -57,6 +57,8 @@ export default { positionType: IMAGE_DIFF_POSITION_TYPE, x: this.diffFileCommentForm.x, y: this.diffFileCommentForm.y, + width: this.diffFileCommentForm.width, + height: this.diffFileCommentForm.height, }, }); }, diff --git a/app/assets/javascripts/diffs/components/image_diff_overlay.vue b/app/assets/javascripts/diffs/components/image_diff_overlay.vue index 4b99f38d293..ae1b0a52901 100644 --- a/app/assets/javascripts/diffs/components/image_diff_overlay.vue +++ b/app/assets/javascripts/diffs/components/image_diff_overlay.vue @@ -50,15 +50,39 @@ export default { methods: { ...mapActions(['toggleDiscussion']), ...mapActions('diffs', ['openDiffFileCommentForm']), - getPosition(discussion) { + getImageDimensions() { return { - left: `${discussion.position.x}px`, - top: `${discussion.position.y}px`, + width: this.$parent.width, + height: this.$parent.height, + }; + }, + getPositionForObject(meta) { + const { x, y, width, height } = meta; + const imageWidth = this.getImageDimensions().width; + const imageHeight = this.getImageDimensions().height; + const widthRatio = imageWidth / width; + const heightRatio = imageHeight / height; + + return { + x: Math.round(x * widthRatio), + y: Math.round(y * heightRatio), + }; + }, + getPosition(discussion) { + const { x, y } = this.getPositionForObject(discussion.position); + + return { + left: `${x}px`, + top: `${y}px`, }; }, clickedImage(x, y) { + const { width, height } = this.getImageDimensions(); + this.openDiffFileCommentForm({ fileHash: this.fileHash, + width, + height, x, y, }); diff --git a/app/assets/javascripts/diffs/store/utils.js b/app/assets/javascripts/diffs/store/utils.js index 178745cffc2..a935b9b1ffa 100644 --- a/app/assets/javascripts/diffs/store/utils.js +++ b/app/assets/javascripts/diffs/store/utils.js @@ -49,6 +49,8 @@ export function getFormData(params) { new_line: noteTargetLine ? noteTargetLine.newLine : null, x: params.x, y: params.y, + width: params.width, + height: params.height, }); const postData = { diff --git a/lib/gitlab/diff/formatters/image_formatter.rb b/lib/gitlab/diff/formatters/image_formatter.rb index 219a4b9b348..ccd0d309972 100644 --- a/lib/gitlab/diff/formatters/image_formatter.rb +++ b/lib/gitlab/diff/formatters/image_formatter.rb @@ -21,7 +21,7 @@ module Gitlab end def complete? - x && y + x && y && width && height end def to_h diff --git a/spec/javascripts/diffs/components/diff_content_spec.js b/spec/javascripts/diffs/components/diff_content_spec.js index a31e04d426b..36bd042f3c4 100644 --- a/spec/javascripts/diffs/components/diff_content_spec.js +++ b/spec/javascripts/diffs/components/diff_content_spec.js @@ -62,7 +62,13 @@ describe('DiffContent', () => { vm.diffFile.oldSha = 'ABC'; vm.diffFile.viewPath = ''; vm.diffFile.discussions = [{ ...discussionsMockData }]; - vm.$store.state.diffs.commentForms.push({ fileHash: vm.diffFile.fileHash, x: 10, y: 20 }); + vm.$store.state.diffs.commentForms.push({ + fileHash: vm.diffFile.fileHash, + x: 10, + y: 20, + width: 100, + height: 200, + }); vm.$nextTick(done); }); @@ -96,6 +102,8 @@ describe('DiffContent', () => { positionType: 'image', x: 10, y: 20, + width: 100, + height: 200, }, }); }); diff --git a/spec/javascripts/diffs/components/image_diff_overlay_spec.js b/spec/javascripts/diffs/components/image_diff_overlay_spec.js index 4ab5ebd6c01..d76ab745fe1 100644 --- a/spec/javascripts/diffs/components/image_diff_overlay_spec.js +++ b/spec/javascripts/diffs/components/image_diff_overlay_spec.js @@ -1,10 +1,14 @@ import Vue from 'vue'; import ImageDiffOverlay from '~/diffs/components/image_diff_overlay.vue'; import { createStore } from '~/mr_notes/stores'; -import { mountComponentWithStore } from 'spec/helpers/vue_mount_component_helper'; +import { createComponentWithStore } from 'spec/helpers/vue_mount_component_helper'; import { imageDiffDiscussions } from '../mock_data/diff_discussions'; describe('Diffs image diff overlay component', () => { + const dimensions = { + width: 100, + height: 200, + }; let Component; let vm; @@ -13,9 +17,10 @@ describe('Diffs image diff overlay component', () => { extendStore(store); - vm = mountComponentWithStore(Component, { - store, - props: { discussions: [...imageDiffDiscussions], fileHash: 'ABC', ...props }, + vm = createComponentWithStore(Component, store, { + discussions: [...imageDiffDiscussions], + fileHash: 'ABC', + ...props, }); } @@ -29,12 +34,16 @@ describe('Diffs image diff overlay component', () => { it('renders comment badges', () => { createComponent(); + spyOn(vm, 'getImageDimensions').and.returnValue(dimensions); + vm.$mount(); expect(vm.$el.querySelectorAll('.js-image-badge').length).toBe(2); }); it('renders index of discussion in badge', () => { createComponent(); + spyOn(vm, 'getImageDimensions').and.returnValue(dimensions); + vm.$mount(); expect(vm.$el.querySelectorAll('.js-image-badge')[0].textContent.trim()).toBe('1'); expect(vm.$el.querySelectorAll('.js-image-badge')[1].textContent.trim()).toBe('2'); @@ -42,12 +51,16 @@ describe('Diffs image diff overlay component', () => { it('renders icon when showCommentIcon is true', () => { createComponent({ showCommentIcon: true }); + spyOn(vm, 'getImageDimensions').and.returnValue(dimensions); + vm.$mount(); expect(vm.$el.querySelector('.js-image-badge svg')).not.toBe(null); }); it('sets badge comment positions', () => { createComponent(); + spyOn(vm, 'getImageDimensions').and.returnValue(dimensions); + vm.$mount(); expect(vm.$el.querySelectorAll('.js-image-badge')[0].style.left).toBe('10px'); expect(vm.$el.querySelectorAll('.js-image-badge')[0].style.top).toBe('10px'); @@ -62,12 +75,16 @@ describe('Diffs image diff overlay component', () => { ...imageDiffDiscussions[0], }, }); + spyOn(vm, 'getImageDimensions').and.returnValue(dimensions); + vm.$mount(); expect(vm.$el.querySelectorAll('.js-image-badge').length).toBe(1); }); - it('dispatches openDiffFileCommentForm when clcking overlay', () => { + it('dispatches openDiffFileCommentForm when clicking overlay', () => { createComponent({ canComment: true }); + spyOn(vm, 'getImageDimensions').and.returnValue(dimensions); + vm.$mount(); spyOn(vm.$store, 'dispatch').and.stub(); @@ -77,18 +94,24 @@ describe('Diffs image diff overlay component', () => { fileHash: 'ABC', x: 0, y: 0, + width: 100, + height: 200, }); }); describe('toggle discussion', () => { it('disables buttons when shouldToggleDiscussion is false', () => { createComponent({ shouldToggleDiscussion: false }); + spyOn(vm, 'getImageDimensions').and.returnValue(dimensions); + vm.$mount(); expect(vm.$el.querySelector('.js-image-badge').hasAttribute('disabled')).toBe(true); }); it('dispatches toggleDiscussion when clicking image badge', () => { createComponent(); + spyOn(vm, 'getImageDimensions').and.returnValue(dimensions); + vm.$mount(); spyOn(vm.$store, 'dispatch').and.stub(); @@ -107,6 +130,8 @@ describe('Diffs image diff overlay component', () => { y: 10, }); }); + spyOn(vm, 'getImageDimensions').and.returnValue(dimensions); + vm.$mount(); }); it('renders comment form badge', () => { diff --git a/spec/javascripts/diffs/mock_data/diff_discussions.js b/spec/javascripts/diffs/mock_data/diff_discussions.js index 908a27417b6..5ffe5a366ba 100644 --- a/spec/javascripts/diffs/mock_data/diff_discussions.js +++ b/spec/javascripts/diffs/mock_data/diff_discussions.js @@ -499,6 +499,8 @@ export const imageDiffDiscussions = [ position: { x: 10, y: 10, + width: 100, + height: 200, }, }, { @@ -506,6 +508,8 @@ export const imageDiffDiscussions = [ position: { x: 5, y: 5, + width: 100, + height: 200, }, }, ];