From 726bdfe4d3188bdadcf11aafe5da78d8ac57fd57 Mon Sep 17 00:00:00 2001 From: Grace Date: Thu, 17 Apr 2025 11:04:54 +0100 Subject: [PATCH 1/8] feat: scroll moving block into view --- src/actions/mover.ts | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/src/actions/mover.ts b/src/actions/mover.ts index 4d2c40e9..9444a83b 100644 --- a/src/actions/mover.ts +++ b/src/actions/mover.ts @@ -144,6 +144,8 @@ export class Mover { this.unpatchWorkspace(workspace); this.unpatchDragStrategy(info.block); this.moves.delete(workspace); + // Delay scroll until after block has finished moving. + setTimeout(() => this.scrollCurrentBlockIntoView(workspace), 0) return true; } @@ -175,6 +177,8 @@ export class Mover { this.unpatchWorkspace(workspace); this.unpatchDragStrategy(info.block); this.moves.delete(workspace); + // Delay scroll until after block has finished moving. + setTimeout(() => this.scrollCurrentBlockIntoView(workspace), 0) return true; } @@ -197,6 +201,7 @@ export class Mover { ); info.updateTotalDelta(); + this.scrollCurrentBlockIntoView(workspace); return true; } @@ -218,6 +223,7 @@ export class Mover { info.totalDelta.y += y * UNCONSTRAINED_MOVE_DISTANCE * workspace.scale; info.dragger.onDrag(info.fakePointerEvent('pointermove'), info.totalDelta); + this.scrollCurrentBlockIntoView(workspace); return true; } @@ -307,6 +313,20 @@ export class Mover { this.oldDragStrategy = null; } } + + /** + * Scrolls the current block into view if exists one. + * + * @param workspace The workspace to get current block from. + */ + private scrollCurrentBlockIntoView(workspace: WorkspaceSvg) { + const blockToView = this.getCurrentBlock(workspace); + if (blockToView) { + workspace.scrollBoundsIntoView( + blockToView.getBoundingRectangleWithoutChildren(), + ); + } + } } /** From 2bc7d7c3be965aca0f6a5f2285d5f854b89ffde1 Mon Sep 17 00:00:00 2001 From: Grace Date: Tue, 22 Apr 2025 10:16:33 +0100 Subject: [PATCH 2/8] chore: tweak doc and format mover.ts --- src/actions/mover.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/actions/mover.ts b/src/actions/mover.ts index 9444a83b..465e6871 100644 --- a/src/actions/mover.ts +++ b/src/actions/mover.ts @@ -145,7 +145,7 @@ export class Mover { this.unpatchDragStrategy(info.block); this.moves.delete(workspace); // Delay scroll until after block has finished moving. - setTimeout(() => this.scrollCurrentBlockIntoView(workspace), 0) + setTimeout(() => this.scrollCurrentBlockIntoView(workspace), 0); return true; } @@ -178,7 +178,7 @@ export class Mover { this.unpatchDragStrategy(info.block); this.moves.delete(workspace); // Delay scroll until after block has finished moving. - setTimeout(() => this.scrollCurrentBlockIntoView(workspace), 0) + setTimeout(() => this.scrollCurrentBlockIntoView(workspace), 0); return true; } @@ -315,7 +315,7 @@ export class Mover { } /** - * Scrolls the current block into view if exists one. + * Scrolls the current block into view if one exists. * * @param workspace The workspace to get current block from. */ From 6c8b3459f2169190ad0fa1716eead715695158fe Mon Sep 17 00:00:00 2001 From: Grace Date: Tue, 22 Apr 2025 10:21:25 +0100 Subject: [PATCH 3/8] feat: add padding param (def to 0) for mover scroll into view --- src/actions/mover.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/actions/mover.ts b/src/actions/mover.ts index 465e6871..637abc39 100644 --- a/src/actions/mover.ts +++ b/src/actions/mover.ts @@ -318,12 +318,15 @@ export class Mover { * Scrolls the current block into view if one exists. * * @param workspace The workspace to get current block from. + * @param padding Amount of spacing to put between the bounds and the edge of + * the workspace's viewport. */ - private scrollCurrentBlockIntoView(workspace: WorkspaceSvg) { + private scrollCurrentBlockIntoView(workspace: WorkspaceSvg, padding = 0) { const blockToView = this.getCurrentBlock(workspace); if (blockToView) { workspace.scrollBoundsIntoView( blockToView.getBoundingRectangleWithoutChildren(), + padding, ); } } From 140be725c15fd87ddc4655d30848eeff85c4e7f8 Mon Sep 17 00:00:00 2001 From: Grace Date: Wed, 23 Apr 2025 09:45:27 +0100 Subject: [PATCH 4/8] fix: correct default padding to 10 for move scroll to view --- src/actions/mover.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/actions/mover.ts b/src/actions/mover.ts index 637abc39..d2f67969 100644 --- a/src/actions/mover.ts +++ b/src/actions/mover.ts @@ -321,7 +321,7 @@ export class Mover { * @param padding Amount of spacing to put between the bounds and the edge of * the workspace's viewport. */ - private scrollCurrentBlockIntoView(workspace: WorkspaceSvg, padding = 0) { + private scrollCurrentBlockIntoView(workspace: WorkspaceSvg, padding = 10) { const blockToView = this.getCurrentBlock(workspace); if (blockToView) { workspace.scrollBoundsIntoView( From 9657acd0b8f61bbcb953fb2639712d65504cad26 Mon Sep 17 00:00:00 2001 From: Grace Date: Wed, 23 Apr 2025 10:19:16 +0100 Subject: [PATCH 5/8] fix: bypass get cur block when move scroll into view The issue is that scrollBoundsIntoView in scrollCurrentBlockIntoView is not taking effect because it has already been called via getCurrentBlock (which eventually calls scrollBoundsIntoView in the call stack). To bypass getCurrentBlock, a new class variable for current block is set and reset for a move. --- src/actions/mover.ts | 26 ++++++++++++++++++-------- 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/src/actions/mover.ts b/src/actions/mover.ts index d2f67969..2dcc01da 100644 --- a/src/actions/mover.ts +++ b/src/actions/mover.ts @@ -55,6 +55,12 @@ export class Mover { */ private oldDragStrategy: IDragStrategy | null = null; + /** + * The current block being moved, which is set at the start of a move and + * reset at the end of a move. + */ + private currentBlock: BlockSvg | null = null; + constructor(protected navigation: Navigation) {} /** @@ -101,6 +107,7 @@ export class Mover { const cursor = workspace?.getCursor(); const block = this.getCurrentBlock(workspace); if (!cursor || !block) throw new Error('precondition failure'); + this.currentBlock = block; // Select and focus block. common.setSelected(block); @@ -141,11 +148,13 @@ export class Mover { new utils.Coordinate(0, 0), ); + // Scroll into view after block has finished moving. + setTimeout(() => this.scrollCurrentBlockIntoView(workspace), 0); + this.unpatchWorkspace(workspace); this.unpatchDragStrategy(info.block); this.moves.delete(workspace); - // Delay scroll until after block has finished moving. - setTimeout(() => this.scrollCurrentBlockIntoView(workspace), 0); + this.currentBlock = null; return true; } @@ -174,11 +183,13 @@ export class Mover { new utils.Coordinate(0, 0), ); + // Scroll into view after block has finished moving. + setTimeout(() => this.scrollCurrentBlockIntoView(workspace), 0); + this.unpatchWorkspace(workspace); this.unpatchDragStrategy(info.block); this.moves.delete(workspace); - // Delay scroll until after block has finished moving. - setTimeout(() => this.scrollCurrentBlockIntoView(workspace), 0); + this.currentBlock = null; return true; } @@ -317,15 +328,14 @@ export class Mover { /** * Scrolls the current block into view if one exists. * - * @param workspace The workspace to get current block from. + * @param workspace The workspace to scroll the given bounds into view in. * @param padding Amount of spacing to put between the bounds and the edge of * the workspace's viewport. */ private scrollCurrentBlockIntoView(workspace: WorkspaceSvg, padding = 10) { - const blockToView = this.getCurrentBlock(workspace); - if (blockToView) { + if (this.currentBlock) { workspace.scrollBoundsIntoView( - blockToView.getBoundingRectangleWithoutChildren(), + this.currentBlock.getBoundingRectangleWithoutChildren(), padding, ); } From 420c2248fc124666a694c254682f026cc24b7d0d Mon Sep 17 00:00:00 2001 From: Grace Date: Wed, 23 Apr 2025 10:32:23 +0100 Subject: [PATCH 6/8] feat: set constrained move block spacing to 100 --- src/actions/mover.ts | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/actions/mover.ts b/src/actions/mover.ts index 2dcc01da..e50d8bb5 100644 --- a/src/actions/mover.ts +++ b/src/actions/mover.ts @@ -24,6 +24,12 @@ import {Navigation} from '../navigation'; */ const UNCONSTRAINED_MOVE_DISTANCE = 20; +/** + * Amount of space to put between a moving block and the edge of + * the workspace's viewport, when making a constrained move. + */ +const CONSTRAINED_MOVE_BLOCK_SPACING = 100; + /** * Low-level code for moving blocks with keyboard shortcuts. */ @@ -212,7 +218,7 @@ export class Mover { ); info.updateTotalDelta(); - this.scrollCurrentBlockIntoView(workspace); + this.scrollCurrentBlockIntoView(workspace, CONSTRAINED_MOVE_BLOCK_SPACING); return true; } From 90198510c82298f1c98284c76b2911252eeea0d3 Mon Sep 17 00:00:00 2001 From: Grace Date: Wed, 23 Apr 2025 17:49:53 +0100 Subject: [PATCH 7/8] Revert "feat: set constrained move block spacing to 100" This reverts commit 420c2248fc124666a694c254682f026cc24b7d0d. --- src/actions/mover.ts | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/src/actions/mover.ts b/src/actions/mover.ts index e50d8bb5..2dcc01da 100644 --- a/src/actions/mover.ts +++ b/src/actions/mover.ts @@ -24,12 +24,6 @@ import {Navigation} from '../navigation'; */ const UNCONSTRAINED_MOVE_DISTANCE = 20; -/** - * Amount of space to put between a moving block and the edge of - * the workspace's viewport, when making a constrained move. - */ -const CONSTRAINED_MOVE_BLOCK_SPACING = 100; - /** * Low-level code for moving blocks with keyboard shortcuts. */ @@ -218,7 +212,7 @@ export class Mover { ); info.updateTotalDelta(); - this.scrollCurrentBlockIntoView(workspace, CONSTRAINED_MOVE_BLOCK_SPACING); + this.scrollCurrentBlockIntoView(workspace); return true; } From f78d28eff550404b81fe63c442881824f38396a7 Mon Sep 17 00:00:00 2001 From: Grace Date: Wed, 23 Apr 2025 17:50:19 +0100 Subject: [PATCH 8/8] Revert "fix: bypass get cur block when move scroll into view" This reverts commit 9657acd0b8f61bbcb953fb2639712d65504cad26. --- src/actions/mover.ts | 26 ++++++++------------------ 1 file changed, 8 insertions(+), 18 deletions(-) diff --git a/src/actions/mover.ts b/src/actions/mover.ts index 2dcc01da..d2f67969 100644 --- a/src/actions/mover.ts +++ b/src/actions/mover.ts @@ -55,12 +55,6 @@ export class Mover { */ private oldDragStrategy: IDragStrategy | null = null; - /** - * The current block being moved, which is set at the start of a move and - * reset at the end of a move. - */ - private currentBlock: BlockSvg | null = null; - constructor(protected navigation: Navigation) {} /** @@ -107,7 +101,6 @@ export class Mover { const cursor = workspace?.getCursor(); const block = this.getCurrentBlock(workspace); if (!cursor || !block) throw new Error('precondition failure'); - this.currentBlock = block; // Select and focus block. common.setSelected(block); @@ -148,13 +141,11 @@ export class Mover { new utils.Coordinate(0, 0), ); - // Scroll into view after block has finished moving. - setTimeout(() => this.scrollCurrentBlockIntoView(workspace), 0); - this.unpatchWorkspace(workspace); this.unpatchDragStrategy(info.block); this.moves.delete(workspace); - this.currentBlock = null; + // Delay scroll until after block has finished moving. + setTimeout(() => this.scrollCurrentBlockIntoView(workspace), 0); return true; } @@ -183,13 +174,11 @@ export class Mover { new utils.Coordinate(0, 0), ); - // Scroll into view after block has finished moving. - setTimeout(() => this.scrollCurrentBlockIntoView(workspace), 0); - this.unpatchWorkspace(workspace); this.unpatchDragStrategy(info.block); this.moves.delete(workspace); - this.currentBlock = null; + // Delay scroll until after block has finished moving. + setTimeout(() => this.scrollCurrentBlockIntoView(workspace), 0); return true; } @@ -328,14 +317,15 @@ export class Mover { /** * Scrolls the current block into view if one exists. * - * @param workspace The workspace to scroll the given bounds into view in. + * @param workspace The workspace to get current block from. * @param padding Amount of spacing to put between the bounds and the edge of * the workspace's viewport. */ private scrollCurrentBlockIntoView(workspace: WorkspaceSvg, padding = 10) { - if (this.currentBlock) { + const blockToView = this.getCurrentBlock(workspace); + if (blockToView) { workspace.scrollBoundsIntoView( - this.currentBlock.getBoundingRectangleWithoutChildren(), + blockToView.getBoundingRectangleWithoutChildren(), padding, ); }