Commit a2e81cec authored by Mark Florian's avatar Mark Florian

Fix image comment note submission

This uses similar rounding logic as in [Design Management].

Addresses https://gitlab.com/gitlab-org/gitlab/-/issues/349389.

The crux of why rounding is necessary is that the image may be scaled to
fit in the UI, and so measurements may result in non-integer values.  In
addition, [MouseEvent.offsetX/Y are technically floats][1], not strictly
integers.

[1]: https://developer.mozilla.org/en-US/docs/Web/API/MouseEvent/offsetX

[Design Management]: https://gitlab.com/gitlab-org/gitlab/blob/73a6999553598919ea2db32bb3266e56cea03852/app/assets/javascripts/design_management/components/design_presentation.vue#L213-216

Changelog: fixed
parent b0921e5d
...@@ -65,8 +65,8 @@ export default { ...@@ -65,8 +65,8 @@ export default {
...mapActions('diffs', ['openDiffFileCommentForm']), ...mapActions('diffs', ['openDiffFileCommentForm']),
getImageDimensions() { getImageDimensions() {
return { return {
width: this.$parent.width, width: Math.round(this.$parent.width),
height: this.$parent.height, height: Math.round(this.$parent.height),
}; };
}, },
getPositionForObject(meta) { getPositionForObject(meta) {
...@@ -94,8 +94,8 @@ export default { ...@@ -94,8 +94,8 @@ export default {
fileHash: this.fileHash, fileHash: this.fileHash,
width, width,
height, height,
x: width * (xPercent / 100), x: Math.round(width * (xPercent / 100)),
y: height * (yPercent / 100), y: Math.round(height * (yPercent / 100)),
xPercent, xPercent,
yPercent, yPercent,
}); });
......
...@@ -6,8 +6,8 @@ import { imageDiffDiscussions } from '../mock_data/diff_discussions'; ...@@ -6,8 +6,8 @@ import { imageDiffDiscussions } from '../mock_data/diff_discussions';
describe('Diffs image diff overlay component', () => { describe('Diffs image diff overlay component', () => {
const dimensions = { const dimensions = {
width: 100, width: 99.9,
height: 200, height: 199.5,
}; };
let wrapper; let wrapper;
let dispatch; let dispatch;
...@@ -80,17 +80,21 @@ describe('Diffs image diff overlay component', () => { ...@@ -80,17 +80,21 @@ describe('Diffs image diff overlay component', () => {
it('dispatches openDiffFileCommentForm when clicking overlay', () => { it('dispatches openDiffFileCommentForm when clicking overlay', () => {
createComponent({ canComment: true }); createComponent({ canComment: true });
wrapper.find('.js-add-image-diff-note-button').trigger('click', { offsetX: 0, offsetY: 0 }); wrapper.find('.js-add-image-diff-note-button').trigger('click', { offsetX: 1.2, offsetY: 3.8 });
expect(dispatch).toHaveBeenCalledWith('diffs/openDiffFileCommentForm', { expect(dispatch).toHaveBeenCalledWith('diffs/openDiffFileCommentForm', {
fileHash: 'ABC', fileHash: 'ABC',
x: 0, x: 1,
y: 0, y: 4,
width: 100, width: 100,
height: 200, height: 200,
xPercent: 0, xPercent: expect.any(Number),
yPercent: 0, yPercent: expect.any(Number),
}); });
const { xPercent, yPercent } = dispatch.mock.calls[0][1];
expect(xPercent).toBeCloseTo(0.6);
expect(yPercent).toBeCloseTo(1.9);
}); });
describe('toggle discussion', () => { describe('toggle discussion', () => {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment