Commit ec5f86bd authored by Illya Klymov's avatar Illya Klymov

Merge branch '33161-highlight-focused-design-discussion-in-image-markers' into 'master'

Highlight focused Design discussion in image markers

See merge request gitlab-org/gitlab!31323
parents d061fed4 c97c456d
......@@ -4,9 +4,11 @@ import ReplyPlaceholder from '~/notes/components/discussion_reply_placeholder.vu
import allVersionsMixin from '../../mixins/all_versions';
import createNoteMutation from '../../graphql/mutations/createNote.mutation.graphql';
import getDesignQuery from '../../graphql/queries/getDesign.query.graphql';
import activeDiscussionQuery from '../../graphql/queries/active_discussion.query.graphql';
import DesignNote from './design_note.vue';
import DesignReplyForm from './design_reply_form.vue';
import { updateStoreAfterAddDiscussionComment } from '../../utils/cache_update';
import { ACTIVE_DISCUSSION_SOURCE_TYPES } from '../../constants';
export default {
components: {
......@@ -39,10 +41,31 @@ export default {
default: '',
},
},
apollo: {
activeDiscussion: {
query: activeDiscussionQuery,
result({ data }) {
const discussionId = data.activeDiscussion.id;
// We watch any changes to the active discussion from the design pins and scroll to this discussion if it exists
// We don't want scrollIntoView to be triggered from the discussion click itself
if (
discussionId &&
data.activeDiscussion.source === ACTIVE_DISCUSSION_SOURCE_TYPES.pin &&
discussionId === this.discussion.notes[0].id
) {
this.$el.scrollIntoView({
behavior: 'smooth',
inline: 'start',
});
}
},
},
},
data() {
return {
discussionComment: '',
isFormRendered: false,
activeDiscussion: {},
};
},
computed: {
......@@ -61,6 +84,9 @@ export default {
atVersion: this.designsVersion,
};
},
isDiscussionHighlighted() {
return this.discussion.notes[0].id === this.activeDiscussion.id;
},
},
methods: {
addDiscussionComment(
......@@ -108,6 +134,7 @@ export default {
:key="note.id"
:note="note"
:markdown-preview-path="markdownPreviewPath"
:class="{ 'gl-bg-blue-50': isDiscussionHighlighted }"
@error="$emit('updateNoteError', $event)"
/>
<div class="reply-wrapper">
......
<script>
import activeDiscussionQuery from '../graphql/queries/active_discussion.query.graphql';
import updateActiveDiscussionMutation from '../graphql/mutations/update_active_discussion.mutation.graphql';
import DesignNotePin from './design_note_pin.vue';
import { ACTIVE_DISCUSSION_SOURCE_TYPES } from '../constants';
export default {
name: 'DesignOverlay',
......@@ -31,10 +34,16 @@ export default {
default: false,
},
},
apollo: {
activeDiscussion: {
query: activeDiscussionQuery,
},
},
data() {
return {
movingNoteNewPosition: null,
movingNoteStartPosition: null,
activeDiscussion: {},
};
},
computed: {
......@@ -162,8 +171,12 @@ export default {
const { x, y } = this.movingNoteNewPosition;
this.setNewNoteCoordinates({ x, y });
},
onExistingNoteMouseup() {
if (!this.movingNoteStartPosition || !this.movingNoteNewPosition) return;
onExistingNoteMouseup(note) {
if (!this.movingNoteStartPosition || !this.movingNoteNewPosition) {
this.updateActiveDiscussion(note.id);
this.$emit('closeCommentForm');
return;
}
const { x, y } = this.movingNoteNewPosition;
this.$emit('moveNote', {
......@@ -191,13 +204,13 @@ export default {
this.onExistingNoteMove(e);
}
},
onNoteMouseup() {
onNoteMouseup(note) {
if (!this.movingNoteStartPosition) return;
if (this.isMovingCurrentComment) {
this.onNewNoteMouseup();
} else {
this.onExistingNoteMouseup();
this.onExistingNoteMouseup(note);
}
this.movingNoteStartPosition = null;
......@@ -205,9 +218,24 @@ export default {
},
onAddCommentMouseup({ offsetX, offsetY }) {
if (this.disableCommenting) return;
if (this.activeDiscussion.id) {
this.updateActiveDiscussion();
}
this.setNewNoteCoordinates({ x: offsetX, y: offsetY });
},
updateActiveDiscussion(id) {
this.$apollo.mutate({
mutation: updateActiveDiscussionMutation,
variables: {
id,
source: ACTIVE_DISCUSSION_SOURCE_TYPES.pin,
},
});
},
isNoteInactive(note) {
return this.activeDiscussion.id && this.activeDiscussion.id !== note.id;
},
},
};
</script>
......@@ -236,8 +264,9 @@ export default {
? getNotePositionStyle(movingNoteNewPosition)
: getNotePositionStyle(note.position)
"
:class="{ inactive: isNoteInactive(note) }"
@mousedown.stop="onNoteMousedown($event, note)"
@mouseup.stop="onNoteMouseup"
@mouseup.stop="onNoteMouseup(note)"
/>
<design-note-pin
v-if="currentCommentForm"
......
......@@ -211,6 +211,10 @@ export default {
this.currentAnnotationPosition = this.getAnnotationPositon(coordinates);
this.$emit('openCommentForm', this.currentAnnotationPosition);
},
closeCommentForm() {
this.currentAnnotationPosition = null;
this.$emit('closeCommentForm');
},
moveNote({ noteId, discussionId, coordinates }) {
const position = this.getAnnotationPositon(coordinates);
this.$emit('moveNote', { noteId, discussionId, position });
......@@ -302,6 +306,7 @@ export default {
:current-comment-form="currentCommentForm"
:disable-commenting="isDraggingDesign"
@openCommentForm="openCommentForm"
@closeCommentForm="closeCommentForm"
@moveNote="moveNote"
/>
</div>
......
......@@ -7,3 +7,8 @@ export const VALID_DESIGN_FILE_MIMETYPE = {
// https://developer.mozilla.org/en-US/docs/Web/API/DataTransfer/types
export const VALID_DATA_TRANSFER_TYPE = 'Files';
export const ACTIVE_DISCUSSION_SOURCE_TYPES = {
pin: 'pin',
discussion: 'discussion',
};
......@@ -3,11 +3,27 @@ import VueApollo from 'vue-apollo';
import { uniqueId } from 'lodash';
import { defaultDataIdFromObject } from 'apollo-cache-inmemory';
import createDefaultClient from '~/lib/graphql';
import activeDiscussionQuery from './graphql/queries/active_discussion.query.graphql';
import typeDefs from './graphql/typedefs.graphql';
Vue.use(VueApollo);
const resolvers = {
Mutation: {
updateActiveDiscussion: (_, { id = null, source }, { cache }) => {
const data = cache.readQuery({ query: activeDiscussionQuery });
data.activeDiscussion = {
__typename: 'ActiveDiscussion',
id,
source,
};
cache.writeQuery({ query: activeDiscussionQuery, data });
},
},
};
const defaultClient = createDefaultClient(
{},
resolvers,
// This config is added temporarily to resolve an issue with duplicate design IDs.
// Should be removed as soon as https://gitlab.com/gitlab-org/gitlab/issues/13495 is resolved
{
......@@ -20,6 +36,7 @@ const defaultClient = createDefaultClient(
return defaultDataIdFromObject(object);
},
},
typeDefs,
},
);
......
mutation updateActiveDiscussion($id: String, $source: String) {
updateActiveDiscussion (id: $id, source: $source ) @client
}
query activeDiscussion {
activeDiscussion @client {
id
source
}
}
type ActiveDiscussion {
id: ID
source: String
}
extend type Query {
activeDiscussion: ActiveDiscussion
}
extend type Mutation {
updateActiveDiscussion(id: ID!, source: String!): Boolean
}
......@@ -24,6 +24,11 @@ export default () => {
data: {
projectPath,
issueIid,
activeDiscussion: {
__typename: 'ActiveDiscussion',
id: null,
source: null,
},
},
});
......
......@@ -16,6 +16,7 @@ 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 updateActiveDiscussionMutation from '../../graphql/mutations/update_active_discussion.mutation.graphql';
import {
extractDiscussions,
extractDesign,
......@@ -37,6 +38,7 @@ import {
} from '../../utils/error_messages';
import { trackDesignDetailView } from '../../utils/tracking';
import { DESIGNS_ROUTE_NAME } from '../../router/constants';
import { ACTIVE_DISCUSSION_SOURCE_TYPES } from '../../constants';
export default {
components: {
......@@ -267,6 +269,15 @@ export default {
this.isLatestVersion,
);
},
updateActiveDiscussion(id) {
this.$apollo.mutate({
mutation: updateActiveDiscussionMutation,
variables: {
id,
source: ACTIVE_DISCUSSION_SOURCE_TYPES.discussion,
},
});
},
},
beforeRouteEnter(to, from, next) {
next(vm => {
......@@ -276,6 +287,13 @@ export default {
beforeRouteUpdate(to, from, next) {
this.trackEvent();
this.closeCommentForm();
// We need to reset the active discussion when opening a new design
this.updateActiveDiscussion();
next();
},
beforeRouteLeave(to, from, next) {
// We need to reset the active discussion when moving to design list view
this.updateActiveDiscussion();
next();
},
createImageDiffNoteMutation,
......@@ -303,7 +321,7 @@ export default {
:is-deleting="loading"
:is-latest-version="isLatestVersion"
v-bind="design"
@delete="mutate()"
@delete="mutate"
/>
</template>
</design-destroyer>
......@@ -320,6 +338,7 @@ export default {
:is-annotating="isAnnotating"
:scale="scale"
@openCommentForm="openCommentForm"
@closeCommentForm="closeCommentForm"
@moveNote="onMoveNote"
/>
......@@ -327,7 +346,7 @@ export default {
<design-scaler @scale="scale = $event" />
</div>
</div>
<div class="image-notes">
<div class="image-notes" @click="updateActiveDiscussion()">
<h2 class="gl-font-size-20-deprecated-no-really-do-not-use-me font-weight-bold mt-0">
{{ issue.title }}
</h2>
......@@ -350,6 +369,7 @@ export default {
:markdown-preview-path="markdownPreviewPath"
@error="onDesignDiscussionError"
@updateNoteError="onUpdateNoteError"
@click.native.stop="updateActiveDiscussion(discussion.notes[0].id)"
/>
<apollo-mutation
v-if="annotationCoordinates"
......
---
title: Highlight focused Design discussion in image markers
merge_request: 31323
author:
type: added
......@@ -4,6 +4,10 @@
.with-performance-bar & {
top: 35px;
}
.inactive {
opacity: 0.5;
}
}
.design-presentation-wrapper {
......
......@@ -28,7 +28,7 @@ describe('Design discussions component', () => {
mutate,
};
function createComponent(props = {}) {
function createComponent(props = {}, data = {}) {
wrapper = shallowMount(DesignDiscussion, {
propsData: {
discussion: {
......@@ -47,6 +47,11 @@ describe('Design discussions component', () => {
discussionIndex: 1,
...props,
},
data() {
return {
...data,
};
},
stubs: {
ReplyPlaceholder,
ApolloMutation,
......@@ -55,23 +60,22 @@ describe('Design discussions component', () => {
});
}
beforeEach(() => {
createComponent();
});
afterEach(() => {
wrapper.destroy();
});
it('renders correct amount of discussion notes', () => {
createComponent();
expect(wrapper.findAll(DesignNote)).toHaveLength(2);
});
it('renders reply placeholder by default', () => {
createComponent();
expect(findReplyPlaceholder().exists()).toBe(true);
});
it('hides reply placeholder and opens form on placeholder click', () => {
createComponent();
findReplyPlaceholder().trigger('click');
return wrapper.vm.$nextTick().then(() => {
......@@ -81,19 +85,14 @@ describe('Design discussions component', () => {
});
it('calls mutation on submitting form and closes the form', () => {
wrapper.setData({
discussionComment: 'test',
isFormRendered: true,
});
createComponent({}, { discussionComment: 'test', isFormRendered: true });
return wrapper.vm
.$nextTick()
.then(() => {
findReplyForm().vm.$emit('submitForm');
expect(mutate).toHaveBeenCalledWith(mutationVariables);
return mutate({ variables: mutationVariables });
return mutate()
.then(() => {
return wrapper.vm.$nextTick();
})
.then(() => {
expect(findReplyForm().exists()).toBe(false);
......@@ -101,10 +100,7 @@ describe('Design discussions component', () => {
});
it('clears the discussion comment on closing comment form', () => {
wrapper.setData({
discussionComment: 'test',
isFormRendered: true,
});
createComponent({}, { discussionComment: 'test', isFormRendered: true });
return wrapper.vm
.$nextTick()
......@@ -118,4 +114,20 @@ describe('Design discussions component', () => {
expect(findReplyForm().exists()).toBe(false);
});
});
it('applies correct class to design notes when discussion is highlighted', () => {
createComponent(
{},
{
activeDiscussion: {
id: '1',
source: 'pin',
},
},
);
expect(wrapper.findAll(DesignNote).wrappers.every(note => note.classes('gl-bg-blue-50'))).toBe(
true,
);
});
});
import { mount } from '@vue/test-utils';
import DesignOverlay from '~/design_management/components/design_overlay.vue';
import updateActiveDiscussion from '~/design_management/graphql/mutations/update_active_discussion.mutation.graphql';
import notes from '../mock_data/notes';
import { ACTIVE_DISCUSSION_SOURCE_TYPES } from '~/design_management/constants';
const mutate = jest.fn(() => Promise.resolve());
describe('Design overlay component', () => {
let wrapper;
......@@ -31,7 +35,7 @@ describe('Design overlay component', () => {
});
};
function createComponent(props = {}) {
function createComponent(props = {}, data = {}) {
wrapper = mount(DesignOverlay, {
propsData: {
dimensions: mockDimensions,
......@@ -41,6 +45,20 @@ describe('Design overlay component', () => {
},
...props,
},
data() {
return {
activeDiscussion: {
id: null,
source: null,
},
...data,
};
},
mocks: {
$apollo: {
mutate,
},
},
});
}
......@@ -107,6 +125,38 @@ describe('Design overlay component', () => {
expect(findFirstBadge().attributes().style).toBe('left: 20px; top: 30px;');
});
});
it('should call an update active discussion mutation when clicking a note without moving it', () => {
const note = notes[0];
const { position } = note;
const mutationVariables = {
mutation: updateActiveDiscussion,
variables: {
id: note.id,
source: ACTIVE_DISCUSSION_SOURCE_TYPES.pin,
},
};
findFirstBadge().trigger('mousedown', { clientX: position.x, clientY: position.y });
return wrapper.vm.$nextTick().then(() => {
findFirstBadge().trigger('mouseup', { clientX: position.x, clientY: position.y });
expect(mutate).toHaveBeenCalledWith(mutationVariables);
});
});
it('when there is an active discussion, should apply inactive class to all pins besides the active one', () => {
wrapper.setData({
activeDiscussion: {
id: notes[0].id,
source: 'discussion',
},
});
return wrapper.vm.$nextTick().then(() => {
expect(findSecondBadge().classes()).toContain('inactive');
});
});
});
describe('when moving notes', () => {
......
......@@ -30,9 +30,8 @@ export default {
id: 'discussion-id',
replyId: 'discussion-reply-id',
notes: {
edges: [
nodes: [
{
node: {
id: 'note-id',
body: '123',
author: {
......@@ -42,7 +41,6 @@ export default {
avatarUrl: 'link-to-avatar',
},
},
},
],
},
},
......
......@@ -53,14 +53,37 @@ exports[`Design management design index page renders design index 1`] = `
participants="[object Object]"
/>
<design-discussion-stub
designid="1"
discussion="[object Object]"
discussionindex="1"
<div
class="design-discussion-wrapper"
>
<div
class="badge badge-pill"
type="button"
>
1
</div>
<div
class="design-discussion bordered-box position-relative"
data-qa-selector="design_discussion_content"
>
<design-note-stub
class=""
markdownpreviewpath="//preview_markdown?target_type=Issue"
noteableid="design-id"
note="[object Object]"
/>
<div
class="reply-wrapper"
>
<reply-placeholder-stub
buttontext="Reply..."
class="qa-discussion-reply"
/>
</div>
</div>
</div>
<!---->
</div>
</div>
......
......@@ -7,6 +7,7 @@ import DesignDiscussion from '~/design_management/components/design_notes/design
import DesignReplyForm from '~/design_management/components/design_notes/design_reply_form.vue';
import Participants from '~/sidebar/components/participants/participants.vue';
import createImageDiffNoteMutation from '~/design_management/graphql/mutations/createImageDiffNote.mutation.graphql';
import updateActiveDiscussionMutation from '~/design_management/graphql/mutations/update_active_discussion.mutation.graphql';
import design from '../../mock_data/design';
import mockResponseWithDesigns from '../../mock_data/designs';
import mockResponseNoDesigns from '../../mock_data/no_designs';
......@@ -32,7 +33,7 @@ describe('Design management design index page', () => {
width: 100,
height: 100,
};
const mutationVariables = {
const createDiscussionMutationVariables = {
mutation: createImageDiffNoteMutation,
update: expect.anything(),
variables: {
......@@ -51,14 +52,24 @@ describe('Design management design index page', () => {
},
},
};
const updateActiveDiscussionMutationVariables = {
mutation: updateActiveDiscussionMutation,
variables: {
id: design.discussions.nodes[0].notes.nodes[0].id,
source: 'discussion',
},
};
const mutate = jest.fn().mockResolvedValue();
const routerPush = jest.fn();
const findDiscussions = () => wrapper.findAll(DesignDiscussion);
const findDiscussionForm = () => wrapper.find(DesignReplyForm);
const findParticipants = () => wrapper.find(Participants);
const findDiscussionsWrapper = () => wrapper.find('.image-notes');
function createComponent(loading = false, { routeQuery = {} } = {}) {
function createComponent(loading = false, data = {}, { routeQuery = {} } = {}) {
const $apollo = {
queries: {
design: {
......@@ -81,22 +92,18 @@ describe('Design management design index page', () => {
mocks: { $apollo, $router, $route },
stubs: {
ApolloMutation,
DesignDiscussion,
},
});
wrapper.setData({
data() {
return {
issueIid: '1',
});
}
function setDesign() {
createComponent(true);
wrapper.vm.$apollo.queries.design.loading = false;
}
function setDesignData() {
wrapper.setData({
design,
activeDiscussion: {
id: null,
source: null,
},
...data,
};
},
});
}
......@@ -107,39 +114,31 @@ describe('Design management design index page', () => {
it('sets loading state', () => {
createComponent(true);
return wrapper.vm.$nextTick().then(() => {
expect(wrapper.element).toMatchSnapshot();
});
});
it('renders design index', () => {
setDesign();
setDesignData();
createComponent(false, { design });
return wrapper.vm.$nextTick().then(() => {
expect(wrapper.element).toMatchSnapshot();
expect(wrapper.find(GlAlert).exists()).toBe(false);
});
});
it('renders participants', () => {
setDesign();
setDesignData();
createComponent(false, { design });
return wrapper.vm.$nextTick().then(() => {
expect(findParticipants().exists()).toBe(true);
});
});
it('passes the correct amount of participants to the Participants component', () => {
createComponent(false, { design });
expect(findParticipants().props('participants')).toHaveLength(1);
});
describe('when has no discussions', () => {
beforeEach(() => {
setDesign();
wrapper.setData({
createComponent(false, {
design: {
...design,
discussions: {
......@@ -160,19 +159,33 @@ describe('Design management design index page', () => {
describe('when has discussions', () => {
beforeEach(() => {
setDesign();
setDesignData();
createComponent(false, { design });
});
it('renders correct amount of discussions', () => {
expect(findDiscussions()).toHaveLength(1);
});
it('sends a mutation to set an active discussion when clicking on a discussion', () => {
findDiscussions()
.at(0)
.trigger('click');
expect(mutate).toHaveBeenCalledWith(updateActiveDiscussionMutationVariables);
});
it('opens a new discussion form', () => {
setDesign();
it('sends a mutation to reset an active discussion when clicking outside of discussion', () => {
findDiscussionsWrapper().trigger('click');
wrapper.setData({
expect(mutate).toHaveBeenCalledWith({
...updateActiveDiscussionMutationVariables,
variables: { id: undefined, source: 'discussion' },
});
});
});
it('opens a new discussion form', () => {
createComponent(false, {
design: {
...design,
discussions: {
......@@ -189,9 +202,7 @@ describe('Design management design index page', () => {
});
it('sends a mutation on submitting form and closes form', () => {
setDesign();
wrapper.setData({
createComponent(false, {
design: {
...design,
discussions: {
......@@ -202,13 +213,13 @@ describe('Design management design index page', () => {
comment: newComment,
});
findDiscussionForm().vm.$emit('submitForm');
expect(mutate).toHaveBeenCalledWith(createDiscussionMutationVariables);
return wrapper.vm
.$nextTick()
.then(() => {
findDiscussionForm().vm.$emit('submitForm');
expect(mutate).toHaveBeenCalledWith(mutationVariables);
return mutate({ variables: mutationVariables });
return mutate({ variables: createDiscussionMutationVariables });
})
.then(() => {
expect(findDiscussionForm().exists()).toBe(false);
......@@ -216,9 +227,7 @@ describe('Design management design index page', () => {
});
it('closes the form and clears the comment on canceling form', () => {
setDesign();
wrapper.setData({
createComponent(false, {
design: {
...design,
discussions: {
......@@ -229,24 +238,18 @@ describe('Design management design index page', () => {
comment: newComment,
});
return wrapper.vm
.$nextTick()
.then(() => {
findDiscussionForm().vm.$emit('cancelForm');
expect(wrapper.vm.comment).toBe('');
return wrapper.vm.$nextTick();
})
.then(() => {
return wrapper.vm.$nextTick().then(() => {
expect(findDiscussionForm().exists()).toBe(false);
});
});
describe('with error', () => {
beforeEach(() => {
setDesign();
wrapper.setData({
createComponent(false, {
design: {
...design,
discussions: {
......@@ -280,7 +283,7 @@ describe('Design management design index page', () => {
describe('when no design exists for given version', () => {
it('redirects to /designs', () => {
// attempt to query for a version of the design that doesn't exist
createComponent(true, { routeQuery: { version: '999' } });
createComponent(true, {}, { routeQuery: { version: '999' } });
wrapper.setData({
allVersions: mockAllVersions,
});
......
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