Commit 59f9a338 authored by Tom Quirk's avatar Tom Quirk Committed by Natalia Tepluhina

Add ability to move design note pin

This is WIP.

Adds code to design_overlay to (eventually)
facilitate moving design note pins
parent 737d8993
......@@ -144,8 +144,10 @@ which you can start a new discussion:
![Starting a new discussion on design](img/adding_note_to_design_1.png)
From GitLab 12.8 on, when you are starting a new discussion, you can adjust the badge's position by
dragging it around the image.
[Introduced](https://gitlab.com/gitlab-org/gitlab/issues/34353) in [GitLab Premium](https://about.gitlab.com/pricing/) 12.8,
you can adjust the badge's position by dragging it around the image. This is useful
for when your design layout has changed between revisions, or if you need to move an
existing badge to add a new one in its place.
Different discussions have different badge numbers:
......
......@@ -41,7 +41,7 @@ export default {
};
},
isMovingCurrentComment() {
return Boolean(this.movingNoteStartPosition);
return Boolean(this.movingNoteStartPosition && !this.movingNoteStartPosition.noteId);
},
currentCommentPositionStyle() {
return this.isMovingCurrentComment && this.movingNoteNewPosition
......@@ -84,13 +84,23 @@ export default {
deltaY,
};
},
isMovingNote(noteId) {
const movingNoteId = this.movingNoteStartPosition?.noteId;
return Boolean(movingNoteId && movingNoteId === noteId);
},
canMoveNote(note) {
const { userPermissions } = note;
const { adminNote } = userPermissions || {};
return Boolean(adminNote);
},
isPositionInOverlay(position) {
const { top, left } = this.getNoteRelativePosition(position);
const { height, width } = this.dimensions;
return top >= 0 && top <= height && left >= 0 && left <= width;
},
onOverlayMousemove(e) {
onNewNoteMove(e) {
if (!this.isMovingCurrentComment) return;
const { deltaX, deltaY } = this.getMovingNotePositionDelta(e);
......@@ -111,17 +121,76 @@ export default {
this.movingNoteNewPosition = movingNoteNewPosition;
},
onNewNoteMousedown({ clientX, clientY }) {
this.movingNoteStartPosition = {
clientX,
clientY,
onExistingNoteMove(e) {
const note = this.notes.find(({ id }) => id === this.movingNoteStartPosition.noteId);
if (!note) return;
const { position } = note;
const { width, height } = position;
const widthRatio = this.dimensions.width / width;
const heightRatio = this.dimensions.height / height;
const { deltaX, deltaY } = this.getMovingNotePositionDelta(e);
const x = position.x * widthRatio + deltaX;
const y = position.y * heightRatio + deltaY;
const movingNoteNewPosition = {
x,
y,
width: this.dimensions.width,
height: this.dimensions.height,
};
if (!this.isPositionInOverlay(movingNoteNewPosition)) {
this.onExistingNoteMouseup();
return;
}
this.movingNoteNewPosition = movingNoteNewPosition;
},
onNewNoteMouseup() {
if (!this.movingNoteNewPosition) return;
const { x, y } = this.movingNoteNewPosition;
this.setNewNoteCoordinates({ x, y });
},
onExistingNoteMouseup() {
if (!this.movingNoteStartPosition || !this.movingNoteNewPosition) return;
const { x, y } = this.movingNoteNewPosition;
this.$emit('moveNote', {
noteId: this.movingNoteStartPosition.noteId,
discussionId: this.movingNoteStartPosition.discussionId,
coordinates: { x, y },
});
},
onNoteMousedown({ clientX, clientY }, note) {
if (note && !this.canMoveNote(note)) return;
this.movingNoteStartPosition = {
noteId: note?.id,
discussionId: note?.discussion.id,
clientX,
clientY,
};
},
onOverlayMousemove(e) {
if (!this.movingNoteStartPosition) return;
if (this.isMovingCurrentComment) {
this.onNewNoteMove(e);
} else {
this.onExistingNoteMove(e);
}
},
onNoteMouseup() {
if (!this.movingNoteStartPosition) return;
if (this.isMovingCurrentComment) {
this.onNewNoteMouseup();
} else {
this.onExistingNoteMouseup();
}
this.movingNoteStartPosition = null;
this.movingNoteNewPosition = null;
......@@ -135,7 +204,7 @@ export default {
class="position-absolute image-diff-overlay frame"
:style="overlayStyle"
@mousemove="onOverlayMousemove"
@mouseleave="onNewNoteMouseup"
@mouseleave="onNoteMouseup"
>
<button
type="button"
......@@ -147,14 +216,21 @@ export default {
v-for="(note, index) in notes"
:key="note.id"
:label="`${index + 1}`"
:position="getNotePositionStyle(note.position)"
:repositioning="isMovingNote(note.id)"
:position="
isMovingNote(note.id) && movingNoteNewPosition
? getNotePositionStyle(movingNoteNewPosition)
: getNotePositionStyle(note.position)
"
@mousedown="onNoteMousedown($event, note)"
@mouseup="onNoteMouseup"
/>
<design-note-pin
v-if="currentCommentForm"
:position="currentCommentPositionStyle"
:repositioning="isMovingCurrentComment"
@mousedown="onNewNoteMousedown"
@mouseup="onNewNoteMouseup"
@mousedown="onNoteMousedown"
@mouseup="onNoteMouseup"
/>
</div>
</template>
......@@ -202,6 +202,10 @@ export default {
this.currentAnnotationPosition = this.getAnnotationPositon(coordinates);
this.$emit('openCommentForm', this.currentAnnotationPosition);
},
moveNote({ noteId, discussionId, coordinates }) {
const position = this.getAnnotationPositon(coordinates);
this.$emit('moveNote', { noteId, discussionId, position });
},
},
};
</script>
......@@ -226,6 +230,7 @@ export default {
:notes="discussionStartingNotes"
:current-comment-form="currentCommentForm"
@openCommentForm="openCommentForm"
@moveNote="moveNote"
/>
</div>
</div>
......
#import "./diffRefs.fragment.graphql"
#import "~/graphql_shared/fragments/author.fragment.graphql"
#import "./notePermissions.fragment.graphql"
fragment DesignNote on Note {
id
......@@ -18,4 +19,10 @@ fragment DesignNote on Note {
height
width
}
userPermissions {
...DesignNotePermissions
}
discussion {
id
}
}
fragment DesignNotePermissions on NotePermissions {
adminNote
}
\ No newline at end of file
#import "../fragments/designNote.fragment.graphql"
mutation updateImageDiffNote($input: UpdateImageDiffNoteInput!) {
updateImageDiffNote(input: $input) {
errors
note {
...DesignNote
}
}
}
......@@ -15,14 +15,21 @@ import DesignPresentation from '../../components/design_presentation.vue';
import getDesignQuery from '../../graphql/queries/getDesign.query.graphql';
import appDataQuery from '../../graphql/queries/appData.query.graphql';
import createImageDiffNoteMutation from '../../graphql/mutations/createImageDiffNote.mutation.graphql';
import updateImageDiffNoteMutation from '../../graphql/mutations/updateImageDiffNote.mutation.graphql';
import {
extractDiscussions,
extractDesign,
extractParticipants,
updateImageDiffNoteOptimisticResponse,
} from '../../utils/design_management_utils';
import { updateStoreAfterAddImageDiffNote } from '../../utils/cache_update';
import {
updateStoreAfterAddImageDiffNote,
updateStoreAfterUpdateImageDiffNote,
} from '../../utils/cache_update';
import {
ADD_DISCUSSION_COMMENT_ERROR,
ADD_IMAGE_DIFF_NOTE_ERROR,
UPDATE_IMAGE_DIFF_NOTE_ERROR,
DESIGN_NOT_FOUND_ERROR,
DESIGN_NOT_EXIST_ERROR,
designDeletionError,
......@@ -165,19 +172,62 @@ export default {
this.designVariables,
);
},
updateImageDiffNoteInStore(
store,
{
data: { updateImageDiffNote },
},
) {
return updateStoreAfterUpdateImageDiffNote(
store,
updateImageDiffNote,
getDesignQuery,
this.designVariables,
);
},
onMoveNote({ noteId, discussionId, position }) {
const discussion = this.discussions.find(({ id }) => id === discussionId);
const note = discussion.notes.find(
({ discussion: noteDiscussion }) => noteDiscussion.id === discussionId,
);
const mutationPayload = {
optimisticResponse: updateImageDiffNoteOptimisticResponse(note, {
position,
}),
variables: {
input: {
id: noteId,
position,
},
},
mutation: updateImageDiffNoteMutation,
update: this.updateImageDiffNoteInStore,
};
return this.$apollo.mutate(mutationPayload).catch(e => this.onUpdateImageDiffNoteError(e));
},
onQueryError(message) {
// because we redirect user to /designs (the issue page),
// we want to create these flashes on the issue page
createFlash(message);
this.$router.push({ name: this.$options.DESIGNS_ROUTE_NAME });
},
onDiffNoteError(e) {
this.errorMessage = ADD_DISCUSSION_COMMENT_ERROR;
onError(message, e) {
this.errorMessage = message;
throw e;
},
onCreateImageDiffNoteError(e) {
this.onError(ADD_IMAGE_DIFF_NOTE_ERROR, e);
},
onDesignDiscussionError(e) {
this.onError(ADD_DISCUSSION_COMMENT_ERROR, e);
},
onUpdateImageDiffNoteError(e) {
this.onError(UPDATE_IMAGE_DIFF_NOTE_ERROR, e);
},
onDesignDeleteError(e) {
this.errorMessage = designDeletionError({ singular: true });
throw e;
this.onError(designDeletionError({ singular: true }), e);
},
openCommentForm(annotationCoordinates) {
this.annotationCoordinates = annotationCoordinates;
......@@ -239,6 +289,7 @@ export default {
:is-annotating="isAnnotating"
:scale="scale"
@openCommentForm="openCommentForm"
@moveNote="onMoveNote"
/>
<div class="design-scaler-wrapper position-absolute mb-4 d-flex-center">
......@@ -264,7 +315,7 @@ export default {
:noteable-id="design.id"
:discussion-index="index + 1"
:markdown-preview-path="markdownPreviewPath"
@error="onDiffNoteError"
@error="onDesignDiscussionError"
/>
<apollo-mutation
v-if="annotationCoordinates"
......@@ -275,7 +326,7 @@ export default {
}"
:update="addImageDiffNoteToStore"
@done="closeCommentForm"
@error="onDiffNoteError"
@error="onCreateImageDiffNoteError"
>
<design-reply-form
v-model="comment"
......
......@@ -2,6 +2,7 @@ import createFlash from '~/flash';
import { extractCurrentDiscussion, extractDesign } from './design_management_utils';
import {
ADD_IMAGE_DIFF_NOTE_ERROR,
UPDATE_IMAGE_DIFF_NOTE_ERROR,
ADD_DISCUSSION_COMMENT_ERROR,
UPLOAD_DESIGN_ERROR,
designDeletionError,
......@@ -148,6 +149,43 @@ const addImageDiffNoteToStore = (store, createImageDiffNote, query, variables) =
});
};
const updateImageDiffNoteInStore = (store, updateImageDiffNote, query, variables) => {
const data = store.readQuery({
query,
variables,
});
const design = extractDesign(data);
const discussion = extractCurrentDiscussion(
design.discussions,
updateImageDiffNote.note.discussion.id,
);
discussion.node = {
...discussion.node,
notes: {
...discussion.node.notes,
edges: [
// the first note is original discussion, and includes the pin `position`
{
__typename: 'NoteEdge',
node: updateImageDiffNote.note,
},
...discussion.node.notes.edges.slice(1),
],
},
};
store.writeQuery({
query,
variables,
data: {
...data,
design,
},
});
};
const addNewDesignToStore = (store, designManagementUpload, query) => {
const data = store.readQuery(query);
......@@ -244,6 +282,14 @@ export const updateStoreAfterAddImageDiffNote = (store, data, query, queryVariab
}
};
export const updateStoreAfterUpdateImageDiffNote = (store, data, query, queryVariables) => {
if (hasErrors(data)) {
onError(data, UPDATE_IMAGE_DIFF_NOTE_ERROR);
} else {
updateImageDiffNoteInStore(store, data, query, queryVariables);
}
};
export const updateStoreAfterUploadDesign = (store, data, query) => {
if (hasErrors(data)) {
onError(data, UPLOAD_DESIGN_ERROR);
......
......@@ -89,6 +89,27 @@ export const designUploadOptimisticResponse = files => {
};
};
/**
* Generates optimistic response for a design upload mutation
* @param {Array<File>} files
*/
export const updateImageDiffNoteOptimisticResponse = (note, { position }) => ({
// False positive i18n lint: https://gitlab.com/gitlab-org/frontend/eslint-plugin-i18n/issues/26
// eslint-disable-next-line @gitlab/i18n/no-non-i18n-strings
__typename: 'Mutation',
updateImageDiffNote: {
__typename: 'UpdateImageDiffNotePayload',
note: {
...note,
position: {
...note.position,
...position,
},
},
errors: [],
},
});
const normalizeAuthor = author => ({
...author,
web_url: author.webUrl,
......
......@@ -8,6 +8,10 @@ export const ADD_IMAGE_DIFF_NOTE_ERROR = s__(
'DesignManagement|Could not create new discussion. Please try again.',
);
export const UPDATE_IMAGE_DIFF_NOTE_ERROR = s__(
'DesignManagement|Could not update discussion. Please try again.',
);
export const UPLOAD_DESIGN_ERROR = s__(
'DesignManagement|Error uploading a new design. Please try again.',
);
......
---
title: Moveable design note pins
merge_request: 24934
author:
type: added
import { mount } from '@vue/test-utils';
import DesignOverlay from 'ee/design_management/components/design_overlay.vue';
import notes from '../mock_data/notes';
describe('Design overlay component', () => {
let wrapper;
const notes = [
{
position: {
height: 100,
width: 100,
x: 10,
y: 15,
},
},
{
position: {
height: 50,
width: 50,
x: 25,
y: 25,
},
},
];
const mockDimensions = { width: 100, height: 100 };
const mockNoteNotAuthorised = {
id: 'note-not-authorised',
discussion: { id: 'discussion-not-authorised' },
position: {
x: 1,
y: 80,
...mockDimensions,
},
userPermissions: {},
};
const findOverlay = () => wrapper.find('.image-diff-overlay');
const findAllNotes = () => wrapper.findAll('.js-image-badge');
......@@ -66,10 +58,10 @@ describe('Design overlay component', () => {
x: 10,
y: 10,
};
wrapper
.find('.image-diff-overlay-add-comment')
.trigger('click', { offsetX: newCoordinates.x, offsetY: newCoordinates.y });
return wrapper.vm.$nextTick().then(() => {
expect(wrapper.emitted('openCommentForm')).toEqual([
[{ x: newCoordinates.x, y: newCoordinates.y }],
......@@ -117,6 +109,78 @@ describe('Design overlay component', () => {
});
});
describe('when moving notes', () => {
it('should update badge style when note is being moved', () => {
createComponent({
notes,
});
const { position } = notes[0];
return clickAndDragBadge(
findFirstBadge(),
{ x: position.x, y: position.y },
{ x: 20, y: 20 },
).then(() => {
expect(findFirstBadge().attributes().style).toBe('left: 20px; top: 20px; cursor: move;');
});
});
it('should emit `moveNote` event when note-moving action ends', () => {
createComponent({ notes });
const note = notes[0];
const { position } = note;
const newCoordinates = { x: 20, y: 20 };
wrapper.setData({
movingNoteNewPosition: {
...position,
...newCoordinates,
},
movingNoteStartPosition: {
noteId: notes[0].id,
discussionId: notes[0].discussion.id,
...position,
},
});
const badge = findFirstBadge();
return clickAndDragBadge(badge, { x: position.x, y: position.y }, newCoordinates)
.then(() => {
badge.trigger('mouseup');
return wrapper.vm.$nextTick();
})
.then(() => {
expect(wrapper.emitted('moveNote')).toEqual([
[
{
noteId: notes[0].id,
discussionId: notes[0].discussion.id,
coordinates: newCoordinates,
},
],
]);
});
});
it('should do nothing if [adminNote] permission is not present', () => {
createComponent({
dimensions: mockDimensions,
notes: [mockNoteNotAuthorised],
});
const badge = findAllNotes().at(0);
return clickAndDragBadge(
badge,
{ x: mockNoteNotAuthorised.x, y: mockNoteNotAuthorised.y },
{ x: 20, y: 20 },
).then(() => {
expect(wrapper.vm.movingNoteStartPosition).toBeNull();
expect(findFirstBadge().attributes().style).toBe('left: 1px; top: 80px;');
});
});
});
describe('with a new form', () => {
it('should render a new comment badge', () => {
createComponent({
......@@ -255,4 +319,25 @@ describe('Design overlay component', () => {
expect(wrapper.vm.getNoteRelativePosition(position)).toEqual({ left: 25, top: 25 });
});
});
describe('canMoveNote', () => {
it.each`
adminNotePermission | canMoveNoteResult
${true} | ${true}
${false} | ${false}
${undefined} | ${false}
`(
'returns [$canMoveNoteResult] when [adminNote permission] is [$adminNotePermission]',
({ adminNotePermission, canMoveNoteResult }) => {
createComponent();
const note = {
userPermissions: {
adminNote: adminNotePermission,
},
};
expect(wrapper.vm.canMoveNote(note)).toBe(canMoveNoteResult);
},
);
});
});
export default [
{
id: 'note-id-1',
position: {
height: 100,
width: 100,
x: 10,
y: 15,
},
userPermissions: {
adminNote: true,
},
discussion: {
id: 'discussion-id-1',
},
},
{
id: 'note-id-2',
position: {
height: 50,
width: 50,
x: 25,
y: 25,
},
userPermissions: {
adminNote: true,
},
discussion: {
id: 'discussion-id-2',
},
},
];
......@@ -4,12 +4,14 @@ import {
updateStoreAfterAddDiscussionComment,
updateStoreAfterAddImageDiffNote,
updateStoreAfterUploadDesign,
updateStoreAfterUpdateImageDiffNote,
} from 'ee/design_management/utils/cache_update';
import {
designDeletionError,
ADD_DISCUSSION_COMMENT_ERROR,
ADD_IMAGE_DIFF_NOTE_ERROR,
UPLOAD_DESIGN_ERROR,
UPDATE_IMAGE_DIFF_NOTE_ERROR,
} from 'ee/design_management/utils/error_messages';
import design from '../mock_data/design';
import createFlash from '~/flash';
......@@ -32,6 +34,7 @@ describe('Design Management cache update', () => {
${'updateStoreAfterAddDiscussionComment'} | ${updateStoreAfterAddDiscussionComment} | ${ADD_DISCUSSION_COMMENT_ERROR} | ${[]}
${'updateStoreAfterAddImageDiffNote'} | ${updateStoreAfterAddImageDiffNote} | ${ADD_IMAGE_DIFF_NOTE_ERROR} | ${[]}
${'updateStoreAfterUploadDesign'} | ${updateStoreAfterUploadDesign} | ${UPLOAD_DESIGN_ERROR} | ${[]}
${'updateStoreAfterUpdateImageDiffNote'} | ${updateStoreAfterUpdateImageDiffNote} | ${UPDATE_IMAGE_DIFF_NOTE_ERROR} | ${[]}
`('$fnName handles errors in response', ({ subject, extraArgs, errorMessage }) => {
expect(createFlash).not.toHaveBeenCalled();
expect(() => subject(mockStore, { errors: mockErrors }, {}, ...extraArgs)).toThrow();
......
......@@ -4,6 +4,7 @@ import {
extractDiscussions,
findVersionId,
designUploadOptimisticResponse,
updateImageDiffNoteOptimisticResponse,
} from 'ee/design_management/utils/design_management_utils';
describe('extractCurrentDiscussion', () => {
......@@ -72,7 +73,7 @@ describe('version parser', () => {
});
describe('optimistic responses', () => {
it('correctly generated for design upload', () => {
it('correctly generated for designManagementUpload', () => {
jest.spyOn(underscore, 'uniqueId').mockImplementation(() => 1);
const expectedResponse = {
__typename: 'Mutation',
......@@ -104,4 +105,32 @@ describe('optimistic responses', () => {
};
expect(designUploadOptimisticResponse([{ name: 'test' }])).toEqual(expectedResponse);
});
it('correctly generated for updateImageDiffNoteOptimisticResponse', () => {
const mockNote = {
id: 'test-note-id',
};
const mockPosition = {
x: 10,
y: 10,
width: 10,
height: 10,
};
const expectedResponse = {
__typename: 'Mutation',
updateImageDiffNote: {
__typename: 'UpdateImageDiffNotePayload',
note: {
...mockNote,
position: mockPosition,
},
errors: [],
},
};
expect(updateImageDiffNoteOptimisticResponse(mockNote, { position: mockPosition })).toEqual(
expectedResponse,
);
});
});
......@@ -6540,6 +6540,9 @@ msgstr ""
msgid "DesignManagement|Could not create new discussion. Please try again."
msgstr ""
msgid "DesignManagement|Could not update discussion. Please try again."
msgstr ""
msgid "DesignManagement|Delete"
msgstr ""
......
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