From 09424f55e4d45fc1afc0e5afe88b0e861afac696 Mon Sep 17 00:00:00 2001 From: Christopher Allen Date: Wed, 18 Jun 2025 17:03:56 +0100 Subject: [PATCH 1/5] 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/webdriverio/test/test_setup.ts | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/test/webdriverio/test/test_setup.ts b/test/webdriverio/test/test_setup.ts index 154e4593..7e98eb26 100644 --- a/test/webdriverio/test/test_setup.ts +++ b/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); } } From e3f533a2f454bc4a0ea8f0211073a128b5a2f403 Mon Sep 17 00:00:00 2001 From: Christopher Allen Date: Wed, 18 Jun 2025 19:10:47 +0100 Subject: [PATCH 2/5] test(Mover): Test unconstrained movement of unconnectable block This test currently fails due to issue #446. --- test/webdriverio/test/move_test.ts | 78 ++++++++++++++++++++++++++++++ 1 file changed, 78 insertions(+) diff --git a/test/webdriverio/test/move_test.ts b/test/webdriverio/test/move_test.ts index d5d911ac..ed66c724 100644 --- a/test/webdriverio/test/move_test.ts +++ b/test/webdriverio/test/move_test.ts @@ -13,6 +13,9 @@ import { tabNavigateToWorkspace, testFileLocations, testSetup, + sendKeyAndWait, + keyDown, + keyRight, } from './test_setup.js'; suite('Move tests', function () { @@ -145,6 +148,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 it 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); + let 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 +277,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 { + return browser.execute((id: string) => { + const block = Blockly.getMainWorkspace().getBlockById(id); + if (!block) throw new Error('block not found'); + return block.getRelativeToSurfaceXY(); + }, id); +} From 1bff92b0b1cc6798f62c81dc81c44f7f10cdd0ad Mon Sep 17 00:00:00 2001 From: Christopher Allen Date: Tue, 17 Jun 2025 16:06:32 +0100 Subject: [PATCH 3/5] fix(Mover): Pass drag delta to dragger.onDrag in pixels Fixes #446 --- src/actions/mover.ts | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/actions/mover.ts b/src/actions/mover.ts index c2e5d1a2..202e6c30 100644 --- a/src/actions/mover.ts +++ b/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; } From 129f1151eb8b52f4782742562cc8a5879fecf1a0 Mon Sep 17 00:00:00 2001 From: Christopher Allen Date: Wed, 18 Jun 2025 19:27:22 +0100 Subject: [PATCH 4/5] chore(tests): Fix lint --- test/webdriverio/test/move_test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/webdriverio/test/move_test.ts b/test/webdriverio/test/move_test.ts index ed66c724..55da6f46 100644 --- a/test/webdriverio/test/move_test.ts +++ b/test/webdriverio/test/move_test.ts @@ -173,7 +173,7 @@ suite('Move tests', function () { // Check constrained moves have no effect. await keyDown(this.browser, 5); - let coordinate = await getCoordinate(this.browser, BLOCK); + const coordinate = await getCoordinate(this.browser, BLOCK); chai.assert.deepEqual( coordinate, startCoordinate, From e470f970944b5a66fd10dfa3ff14cf0f5b3af480 Mon Sep 17 00:00:00 2001 From: Christopher Allen Date: Thu, 19 Jun 2025 11:20:09 +0100 Subject: [PATCH 5/5] chore(tests): Fix nits for PR #599 --- test/webdriverio/test/move_test.ts | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/test/webdriverio/test/move_test.ts b/test/webdriverio/test/move_test.ts index 55da6f46..3a603e26 100644 --- a/test/webdriverio/test/move_test.ts +++ b/test/webdriverio/test/move_test.ts @@ -15,7 +15,6 @@ import { testSetup, sendKeyAndWait, keyDown, - keyRight, } from './test_setup.js'; suite('Move tests', function () { @@ -154,8 +153,8 @@ suite('Move tests', function () { // // 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 it would result in the - // block unexpectedly moving (unless workspace scale was === 1) + // 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'; @@ -280,7 +279,7 @@ function getConnectedBlockInfo(browser: Browser, id: string, index: number) { /** * Given a block ID, get the coordinates of that block, as returned by - * .getRelativeTosSurfaceXY(). + * getRelativeTosSurfaceXY(). * * @param browser The webdriverio browser session. * @param id The ID of the block having the connection we wish to examine.