fix(mover): Pass drag delta to dragger.onDrag in pixels (#599)

* refactor(tests): export sendKeyAndWait and make it more flexible

  Change the signature of sendKeyAndWait:

  - Export it, so it can be used directly by tests.
  - Allow multiple keys to be specified.
  - Make times parameter optional (default 1).

* test(Mover): Test unconstrained movement of unconnectable block

  This test currently fails due to issue #446.

* fix(Mover): Pass drag delta to dragger.onDrag in pixels

  Fixes #446

* chore(tests): Fix lint

* chore(tests): Fix nits for PR #599

Author: cpcallen

src/actions/mover.ts

@@ -302,7 +302,7 @@ export class Mover {
 
     info.dragger.onDrag(
       info.fakePointerEvent('pointermove', direction),
-      info.totalDelta,
+      info.totalDelta.clone().scale(workspace.scale),
     );
 
     info.updateTotalDelta();
@@ -327,7 +327,10 @@ export class Mover {
     info.totalDelta.x += x * UNCONSTRAINED_MOVE_DISTANCE * workspace.scale;
     info.totalDelta.y += y * UNCONSTRAINED_MOVE_DISTANCE * workspace.scale;
 
-    info.dragger.onDrag(info.fakePointerEvent('pointermove'), info.totalDelta);
+    info.dragger.onDrag(
+      info.fakePointerEvent('pointermove'),
+      info.totalDelta.clone().scale(workspace.scale),
+    );
     this.scrollCurrentBlockIntoView(workspace);
     return true;
   }

test/webdriverio/test/move_test.ts

@@ -13,6 +13,8 @@ import {
   tabNavigateToWorkspace,
   testFileLocations,
   testSetup,
+  sendKeyAndWait,
+  keyDown,
 } from './test_setup.js';
 
 suite('Move tests', function () {
@@ -145,6 +147,62 @@ suite('Move tests', function () {
       await this.browser.keys(Key.Escape);
     }
   });
+
+  // When a top-level block with no previous, next or output
+  // connections is subject to a constrained move, it should not move.
+  //
+  // This includes a regression test for issue #446 (fixed in PR #599)
+  // where, due to an implementation error in Mover, constrained
+  // movement following unconstrained movement would result in the
+  // block unexpectedly moving (unless workspace scale was === 1).
+  test('Constrained move of unattachable top-level block', async function () {
+    // Block ID of an unconnectable block.
+    const BLOCK = 'p5_setup_1';
+
+    // Scale workspace.
+    await this.browser.execute(() => {
+      (Blockly.getMainWorkspace() as Blockly.WorkspaceSvg).setScale(0.9);
+    });
+
+    // Navigate to unconnectable block, get initial coords and start move.
+    await tabNavigateToWorkspace(this.browser);
+    await focusOnBlock(this.browser, BLOCK);
+    const startCoordinate = await getCoordinate(this.browser, BLOCK);
+    await this.browser.keys('m');
+
+    // Check constrained moves have no effect.
+    await keyDown(this.browser, 5);
+    const coordinate = await getCoordinate(this.browser, BLOCK);
+    chai.assert.deepEqual(
+      coordinate,
+      startCoordinate,
+      'constrained move should have no effect',
+    );
+
+    // Unconstrained moves.
+    await sendKeyAndWait(this.browser, [Key.Alt, Key.ArrowDown]);
+    await sendKeyAndWait(this.browser, [Key.Alt, Key.ArrowRight]);
+    const newCoordinate = await getCoordinate(this.browser, BLOCK);
+    chai.assert.notDeepEqual(
+      newCoordinate,
+      startCoordinate,
+      'unconstrained move should have effect',
+    );
+
+    // Try multiple constrained moves, as first might (correctly) do nothing.
+    for (let i = 0; i < 5; i++) {
+      await keyDown(this.browser);
+      const coordinate = await getCoordinate(this.browser, BLOCK);
+      chai.assert.deepEqual(
+        coordinate,
+        newCoordinate,
+        'constrained move after unconstrained move should have no effect',
+      );
+    }
+
+    // Abort move.
+    await this.browser.keys(Key.Escape);
+  });
 });
 
 /**
@@ -218,3 +276,22 @@ function getConnectedBlockInfo(browser: Browser, id: string, index: number) {
     index,
   );
 }
+
+/**
+ * Given a block ID, get the coordinates of that block, as returned by
+ * getRelativeTosSurfaceXY().
+ *
+ * @param browser The webdriverio browser session.
+ * @param id The ID of the block having the connection we wish to examine.
+ * @returns The coordinates of the block.
+ */
+function getCoordinate(
+  browser: Browser,
+  id: string,
+): Promise<Blockly.utils.Coordinate> {
+  return browser.execute((id: string) => {
+    const block = Blockly.getMainWorkspace().getBlockById(id);
+    if (!block) throw new Error('block not found');
+    return block.getRelativeToSurfaceXY();
+  }, id);
+}

test/webdriverio/test/test_setup.ts

@@ -416,8 +416,7 @@ export async function tabNavigateToWorkspace(
  * @param browser The active WebdriverIO Browser object.
  */
 export async function tabNavigateForward(browser: WebdriverIO.Browser) {
-  await browser.keys(webdriverio.Key.Tab);
-  await browser.pause(PAUSE_TIME);
+  await sendKeyAndWait(browser, webdriverio.Key.Tab);
 }
 
 /**
@@ -426,8 +425,7 @@ export async function tabNavigateForward(browser: WebdriverIO.Browser) {
  * @param browser The active WebdriverIO Browser object.
  */
 export async function tabNavigateBackward(browser: WebdriverIO.Browser) {
-  await browser.keys([webdriverio.Key.Shift, webdriverio.Key.Tab]);
-  await browser.pause(PAUSE_TIME);
+  await sendKeyAndWait(browser, [webdriverio.Key.Shift, webdriverio.Key.Tab]);
 }
 
 /**
@@ -471,20 +469,20 @@ export async function keyDown(browser: WebdriverIO.Browser, times = 1) {
 }
 
 /**
- * Sends the specified key for the specified number of times, waiting between
- * each key press to allow changes to keep up.
+ * Sends the specified key(s) for the specified number of times,
+ * waiting between each key press to allow changes to keep up.
  *
  * @param browser The active WebdriverIO Browser object.
- * @param key The WebdriverIO representative key value to press.
- * @param times The number of times to repeat the key press.
+ * @param keys The WebdriverIO representative key value(s) to press.
+ * @param times The number of times to repeat the key press (default 1).
  */
-async function sendKeyAndWait(
+export async function sendKeyAndWait(
   browser: WebdriverIO.Browser,
-  key: string,
-  times: number,
+  keys: string | string[],
+  times = 1,
 ) {
   for (let i = 0; i < times; i++) {
-    await browser.keys(key);
+    await browser.keys(keys);
     await browser.pause(PAUSE_TIME);
   }
 }