Skip to content

fix: don't visit connections with the cursor. #9030

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 13, 2025
Merged

fix: don't visit connections with the cursor. #9030

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
126 changes: 77 additions & 49 deletions core/keyboard_nav/block_navigation_policy.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,11 @@
* SPDX-License-Identifier: Apache-2.0
*/

import type {BlockSvg} from '../block_svg.js';
import {BlockSvg} from '../block_svg.js';
import type {Field} from '../field.js';
import type {INavigable} from '../interfaces/i_navigable.js';
import type {INavigationPolicy} from '../interfaces/i_navigation_policy.js';
import type {RenderedConnection} from '../rendered_connection.js';
import {WorkspaceSvg} from '../workspace_svg.js';

/**
* Set of rules controlling keyboard navigation from a block.
Expand All @@ -24,7 +25,8 @@ export class BlockNavigationPolicy implements INavigationPolicy<BlockSvg> {
for (const field of input.fieldRow) {
return field;
}
if (input.connection) return input.connection as RenderedConnection;
if (input.connection?.targetBlock())
return input.connection.targetBlock() as BlockSvg;
}

return null;
Expand All @@ -38,12 +40,14 @@ export class BlockNavigationPolicy implements INavigationPolicy<BlockSvg> {
* which it is attached.
*/
getParent(current: BlockSvg): INavigable<unknown> | null {
const topBlock = current.getTopStackBlock();
if (current.previousConnection?.targetBlock()) {
const surroundParent = current.getSurroundParent();
if (surroundParent) return surroundParent;
} else if (current.outputConnection?.targetBlock()) {
return current.outputConnection.targetBlock();
}

return (
(this.getParentConnection(topBlock)?.targetConnection?.getParentInput()
?.connection as RenderedConnection) ?? topBlock
);
return current.workspace;
}

/**
Expand All @@ -54,21 +58,40 @@ export class BlockNavigationPolicy implements INavigationPolicy<BlockSvg> {
* block, or its next connection.
*/
getNextSibling(current: BlockSvg): INavigable<unknown> | null {
const nextConnection = current.nextConnection;
if (!current.outputConnection?.targetConnection && !nextConnection) {
// If this block has no connected output connection and no next
// connection, it must be the last block in the stack, so its next sibling
// is the first block of the next stack on the workspace.
const topBlocks = current.workspace.getTopBlocks(true);
let targetIndex = topBlocks.indexOf(current.getRootBlock()) + 1;
if (targetIndex >= topBlocks.length) {
targetIndex = 0;
if (current.nextConnection?.targetBlock()) {
return current.nextConnection?.targetBlock();
}

const parent = this.getParent(current);
let navigatingCrossStacks = false;
let siblings: (BlockSvg | Field)[] = [];
if (parent instanceof BlockSvg) {
for (let i = 0, input; (input = parent.inputList[i]); i++) {
if (input.connection) {
siblings.push(...input.fieldRow);
const child = input.connection.targetBlock();
if (child) {
siblings.push(child as BlockSvg);
}
}
}
const previousBlock = topBlocks[targetIndex];
return this.getParentConnection(previousBlock) ?? previousBlock;
} else if (parent instanceof WorkspaceSvg) {
siblings = parent.getTopBlocks(true);
navigatingCrossStacks = true;
} else {
return null;
}

return nextConnection;
const currentIndex = siblings.indexOf(
navigatingCrossStacks ? current.getRootBlock() : current,
);
if (currentIndex >= 0 && currentIndex < siblings.length - 1) {
return siblings[currentIndex + 1];
} else if (currentIndex === siblings.length - 1 && navigatingCrossStacks) {
return siblings[0];
}

return null;
}

/**
Expand All @@ -79,39 +102,44 @@ export class BlockNavigationPolicy implements INavigationPolicy<BlockSvg> {
* connection/block of the previous block stack if it is a root block.
*/
getPreviousSibling(current: BlockSvg): INavigable<unknown> | null {
const parentConnection = this.getParentConnection(current);
if (parentConnection) return parentConnection;

// If this block has no output/previous connection, it must be a root block,
// so its previous sibling is the last connection of the last block of the
// previous stack on the workspace.
const topBlocks = current.workspace.getTopBlocks(true);
let targetIndex = topBlocks.indexOf(current.getRootBlock()) - 1;
if (targetIndex < 0) {
targetIndex = topBlocks.length - 1;
if (current.previousConnection?.targetBlock()) {
return current.previousConnection?.targetBlock();
}

const lastBlock = topBlocks[targetIndex]
.getDescendants(true)
.reverse()
.pop();
const parent = this.getParent(current);
let navigatingCrossStacks = false;
let siblings: (BlockSvg | Field)[] = [];
if (parent instanceof BlockSvg) {
for (let i = 0, input; (input = parent.inputList[i]); i++) {
if (input.connection) {
siblings.push(...input.fieldRow);
const child = input.connection.targetBlock();
if (child) {
siblings.push(child as BlockSvg);
}
}
}
} else if (parent instanceof WorkspaceSvg) {
siblings = parent.getTopBlocks(true);
navigatingCrossStacks = true;
} else {
return null;
}

return lastBlock?.nextConnection ?? lastBlock ?? null;
}
const currentIndex = siblings.indexOf(current);
let result: INavigable<any> | null = null;
if (currentIndex >= 1) {
result = siblings[currentIndex - 1];
} else if (currentIndex === 0 && navigatingCrossStacks) {
result = siblings[siblings.length - 1];
}

/**
* Gets the parent connection on a block.
* This is either an output connection, previous connection or undefined.
* If both connections exist return the one that is actually connected
* to another block.
*
* @param block The block to find the parent connection on.
* @returns The connection connecting to the parent of the block.
*/
protected getParentConnection(block: BlockSvg) {
if (!block.outputConnection || block.previousConnection?.isConnected()) {
return block.previousConnection;
// If navigating to a previous stack, our previous sibling is the last
// block in it.
if (navigatingCrossStacks && result instanceof BlockSvg) {
return result.lastConnectionInStack(false)?.getSourceBlock() ?? result;
Copy link
Contributor

@RoboErikG RoboErikG May 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Something I didn't have time to investigate, but I suspect this will return the first block in a stack if you have a termination block (blocks without a next connection) on the end of the stack. This is rare enough that it probably makes sense to fix in a follow-up issue since you'll likely need to add a method to get the last block in a stack instead of the last connection.

}
return block.outputConnection;

return result;
}
}
9 changes: 4 additions & 5 deletions core/keyboard_nav/field_navigation_policy.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import type {BlockSvg} from '../block_svg.js';
import type {Field} from '../field.js';
import type {INavigable} from '../interfaces/i_navigable.js';
import type {INavigationPolicy} from '../interfaces/i_navigation_policy.js';
import type {RenderedConnection} from '../rendered_connection.js';

/**
* Set of rules controlling keyboard navigation from a field.
Expand Down Expand Up @@ -52,8 +51,8 @@ export class FieldNavigationPolicy implements INavigationPolicy<Field<any>> {
const fieldRow = newInput.fieldRow;
if (fieldIdx < fieldRow.length) return fieldRow[fieldIdx];
fieldIdx = 0;
if (newInput.connection) {
return newInput.connection as RenderedConnection;
if (newInput.connection?.targetBlock()) {
return newInput.connection.targetBlock() as BlockSvg;
}
}
return null;
Expand All @@ -74,8 +73,8 @@ export class FieldNavigationPolicy implements INavigationPolicy<Field<any>> {
let fieldIdx = parentInput.fieldRow.indexOf(current) - 1;
for (let i = curIdx; i >= 0; i--) {
const input = block.inputList[i];
if (input.connection && input !== parentInput) {
return input.connection as RenderedConnection;
if (input.connection?.targetBlock() && input !== parentInput) {
return input.connection.targetBlock() as BlockSvg;
}
const fieldRow = input.fieldRow;
if (fieldIdx > -1) return fieldRow[fieldIdx];
Expand Down
Loading