fix: set dialog position on drag start (#10191) (CP: 24.9) (#10196)

* fix: set dialog position on drag start (#10191)

* fix: align setting bounds with v25, remove not needed tests

---------

Co-authored-by: Sascha Ißbrücker <sissbruecker@vaadin.com>
Co-authored-by: web-padawan <iamkulykov@gmail.com>

Author: 3 people

packages/dialog/src/vaadin-dialog-draggable-mixin.js

@@ -95,7 +95,8 @@ export const DialogDraggableMixin = (superClass) =>
           window.addEventListener('touchmove', this._drag);
           if (this.$.overlay.$.overlay.style.position !== 'absolute') {
             const { top, left } = this._originalBounds;
-            this.$.overlay.setBounds({ top, left });
+            this.top = top;
+            this.left = left;
           }
         }
       }

packages/dialog/src/vaadin-dialog-resizable-mixin.js

@@ -72,10 +72,7 @@ export const DialogResizableMixin = (superClass) =>
         window.addEventListener('touchmove', this._resizeListeners.resize[direction]);
         window.addEventListener('mouseup', this._resizeListeners.stop[direction]);
         window.addEventListener('touchend', this._resizeListeners.stop[direction]);
-        if (this.$.overlay.$.overlay.style.position !== 'absolute' || this.width || this.height) {
-          this.$.overlay.setBounds(this._originalBounds);
-        }
-
+        this.$.overlay.setBounds(this._originalBounds);
         this.$.overlay.setAttribute('has-bounds-set', '');
       }
     }

packages/dialog/test/draggable-resizable.test.js

@@ -305,14 +305,6 @@ describe('resizable', () => {
     expect(Math.floor(resizedBounds.width)).to.be.eql(Math.floor(bounds.width + dx));
   });
 
-  it('should not set bounds again after position is set to absolute', () => {
-    const spy = sinon.spy(dialog.$.overlay, 'setBounds');
-    dispatchMouseEvent(overlayPart.querySelector('.n'), 'mousedown');
-    dialog.$.overlay.$.overlay.style.position = 'absolute';
-    dispatchMouseEvent(overlayPart.querySelector('.n'), 'mousedown');
-    expect(spy.calledOnce).to.be.true;
-  });
-
   it('should dispatch resize event with correct details', () => {
     const onResize = sinon.spy();
     const content = dialog.$.overlay.$.content;
@@ -440,8 +432,9 @@ describe('draggable', () => {
     expect(Math.floor(draggedBounds.left)).to.be.eql(Math.floor(bounds.left + dx));
   });
 
-  it('should only change "position", "top", and "left" values on drag', () => {
+  it('should only change "position", "top", and "left" values on drag', async () => {
     drag(content);
+    await nextRender();
     const overlay = dialog.$.overlay.$.overlay;
     const style = overlay.style;
     expect(style.length).to.be.eql(3);
@@ -450,6 +443,18 @@ describe('draggable', () => {
     expect(style.left).to.be.ok;
   });
 
+  it('should assign "top", and "left" properties on drag start', async () => {
+    const overlayBounds = dialog.$.overlay.getBounds();
+    expect(dialog.left).to.be.undefined;
+    expect(dialog.top).to.be.undefined;
+
+    dispatchMouseEvent(content, 'mousedown');
+    await nextRender();
+
+    expect(dialog.left).to.be.equal(overlayBounds.left);
+    expect(dialog.top).to.be.equal(overlayBounds.top);
+  });
+
   it('should drag and move dialog if mousedown on element with [class="draggable"] in another shadow root', async () => {
     drag(dialog.$.overlay.querySelector('internally-draggable').shadowRoot.querySelector('.draggable'));
     await nextRender();
@@ -532,7 +537,7 @@ describe('draggable', () => {
     expect(Math.floor(draggedBounds.left)).to.be.eql(Math.floor(bounds.left));
   });
 
-  it('should not drag dialog if mousedown on .resizer-container scrollbar', () => {
+  it('should not drag dialog if mousedown on .resizer-container scrollbar', async () => {
     const boundsSpy = sinon.spy(dialog.$.overlay, 'setBounds');
     content.style.width = `${content.clientWidth * 4}px`;
     const scrollbarHeight = container.offsetHeight - container.clientHeight;
@@ -541,6 +546,7 @@ describe('draggable', () => {
       x: containerBounds.left + containerBounds.width / 2,
       y: containerBounds.top + containerBounds.height - scrollbarHeight / 2,
     });
+    await nextRender();
     expect(boundsSpy.called).to.equal(!scrollbarHeight);
   });
 
@@ -578,14 +584,6 @@ describe('draggable', () => {
     expect(Math.floor(draggedBounds.height)).to.be.eql(Math.floor(bounds.height));
   });
 
-  it('should not update overlay bounds with position: absolute', () => {
-    const spy = sinon.spy(dialog.$.overlay, 'setBounds');
-    dispatchMouseEvent(content, 'mousedown');
-    dialog.$.overlay.$.overlay.style.position = 'absolute';
-    dispatchMouseEvent(content, 'mousedown');
-    expect(spy.calledOnce).to.be.true;
-  });
-
   it('should not reset scroll position on dragstart', async () => {
     dialog.modeless = true;
     button.style.marginBottom = '200px';
@@ -617,6 +615,22 @@ describe('draggable', () => {
     expect(detail.left).to.be.equal(dialog.left);
   });
 
+  it('should use correct coordinates in "dragged" event without moving the dialog during drag', async () => {
+    const overlayBounds = dialog.$.overlay.getBounds();
+    const onDragged = sinon.spy();
+    dialog.addEventListener('dragged', onDragged);
+
+    dispatchMouseEvent(content, 'mousedown');
+    await nextRender();
+    dispatchMouseEvent(content, 'mouseup');
+    await nextRender();
+
+    expect(onDragged.calledOnce).to.be.true;
+    const { detail } = onDragged.args[0][0];
+    expect(detail.top).to.be.equal(overlayBounds.top);
+    expect(detail.left).to.be.equal(overlayBounds.left);
+  });
+
   it('should not set overlay max-width to none on drag', async () => {
     drag(container);
     await nextRender();