Merge branch 'ntepluhina-fix-note-scrolling' into 'master'

Fix design note scrolling

See merge request gitlab-org/gitlab!33939

app/assets/javascripts/design_management/components/design_notes/design_discussion.vue

@@ -63,10 +63,12 @@ export default {
       query: activeDiscussionQuery,
       result({ data }) {
         const discussionId = data.activeDiscussion.id;
+        if (this.discussion.resolved && !this.resolvedDiscussionsExpanded) {
+          return;
+        }
         // 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 (
-          this.resolvedDiscussionsExpanded &&
           discussionId &&
           data.activeDiscussion.source === ACTIVE_DISCUSSION_SOURCE_TYPES.pin &&
           discussionId === this.discussion.notes[0].id

app/assets/javascripts/design_management/components/design_overlay.vue

@@ -144,7 +144,7 @@ export default {
     },
     onExistingNoteMove(e) {
       const note = this.notes.find(({ id }) => id === this.movingNoteStartPosition.noteId);
-      if (!note) return;
+      if (!note || !this.canMoveNote(note)) return;
 
       const { position } = note;
       const { width, height } = position;
@@ -190,8 +190,6 @@ export default {
       });
     },
     onNoteMousedown({ clientX, clientY }, note) {
-      if (note && !this.canMoveNote(note)) return;
-
       this.movingNoteStartPosition = {
         noteId: note?.id,
         discussionId: note?.discussion.id,

changelogs/unreleased/ntepluhina-fix-note-scrolling.yml

+---
+title: Fix design note scrolling
+merge_request: 33939
+author:
+type: fixed

spec/frontend/design_management/components/design_overlay_spec.js

@@ -10,18 +10,6 @@ describe('Design overlay component', () => {
   let wrapper;
 
   const mockDimensions = { width: 100, height: 100 };
-  const mockNoteNotAuthorised = {
-    id: 'note-not-authorised',
-    index: 1,
-    discussion: { id: 'discussion-not-authorised' },
-    position: {
-      x: 1,
-      y: 80,
-      ...mockDimensions,
-    },
-    userPermissions: {},
-    resolved: false,
-  };
 
   const findOverlay = () => wrapper.find('.image-diff-overlay');
   const findAllNotes = () => wrapper.findAll('.js-image-badge');
@@ -230,20 +218,32 @@ describe('Design overlay component', () => {
         });
     });
 
-    it('should do nothing if [adminNote] permission is not present', () => {
+    describe('without [adminNote] permission', () => {
+      const mockNoteNotAuthorised = {
+        ...notes[0],
+        userPermissions: {
+          adminNote: false,
+        },
+      };
+
+      const mockNoteCoordinates = {
+        x: mockNoteNotAuthorised.position.x,
+        y: mockNoteNotAuthorised.position.y,
+      };
+
+      it('should be unable to move a note', () => {
         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;');
+        return clickAndDragBadge(badge, { ...mockNoteCoordinates }, { x: 20, y: 20 }).then(() => {
+          // note position should not change after a click-and-drag attempt
+          expect(findFirstBadge().attributes().style).toContain(
+            `left: ${mockNoteCoordinates.x}px; top: ${mockNoteCoordinates.y}px;`,
+          );
+        });
       });
     });
   });