Skip to content
2 changes: 1 addition & 1 deletion src/actions/clipboard.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ const createSerializedKey = ShortcutRegistry.registry.createSerializedKey.bind(
* menu; changing individual weights relative to base weight can change
* the order within the clipboard group.
*/
const BASE_WEIGHT = 11;
const BASE_WEIGHT = 12;

/**
* Logic and state for cut/copy/paste actions as both keyboard shortcuts
Expand Down
2 changes: 1 addition & 1 deletion src/actions/delete.ts
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ export class DeleteAction {
},
scopeType: ContextMenuRegistry.ScopeType.BLOCK,
id: 'blockDeleteFromContextMenu',
weight: 10,
weight: 11,
};

ContextMenuRegistry.registry.register(deleteItem);
Expand Down
81 changes: 81 additions & 0 deletions src/actions/edit.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
/**
* @license
* Copyright 2025 Google LLC
* SPDX-License-Identifier: Apache-2.0
*/

import {
Connection,
ContextMenuRegistry,
ShortcutRegistry,
comments,
utils as BlocklyUtils,
} from 'blockly';
import * as Constants from '../constants';
import type {BlockSvg, WorkspaceSvg} from 'blockly';
import {LineCursor} from '../line_cursor';
import {NavigationController} from '../navigation_controller';

const KeyCodes = BlocklyUtils.KeyCodes;

/**
* Action to edit a block—this just moves the cursor to the first
* field or input (if there is one), as an aid to navigational
* discoverability.
*
* N.B.: This item is shown any time the cursor is on a block and not
* in the rightmost position 'on the current line'; that means that
* sometimes the label ("Edit block contents") is possibly misleading,
* because it might not be the contents of the _current_ block that's
* being edited, but rather that of a sibling or parent block.
*
* This action registers itself only as a context menu item, as there
* is already a corresponding "right" shortcut item.
Comment on lines +38 to +39
Copy link
Collaborator

Choose a reason for hiding this comment

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

Does it perhaps make sense to consolidate this with the shortcut for workspace contexts? It seems to be doing roughly the same thing, and it feels a bit odd to have an inconsistency here as compared to other action classes (where those manage both context menu and shortcut triggers).

It wouldn't seem odd, I think, for there to be a workspace-only shortcut handled here either since the context menu callback and precondition are already assuming workspace only. I suppose the only aspects I'm unsure of is having multiple actions bound for the same key (RIGHT) or whether splitting that into multiple locations is wise.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think this code could be merged in to actions/ws_movement.ts, now that has been separated out in #281.

The precondition for showing the "edit" option is a bit different from the precondition for allowing the right arrow to work, though: the right arrow key is practically always enabled, wheras the edit option is only shown in some places.

(And the edit option will need to be updated once the cursor properly supports RTL, since in that case it will do the same thing as the left arrow key, rather than the right arrow key.)

Unfortunately I won't have time to do such a refactor before I leave today, and I won't be back for a couple of weeks.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think it's fine to merge this as-is for the benefits of the clean-up. Will approve, thanks!

*/
export class EditAction {
constructor(private canCurrentlyNavigate: (ws: WorkspaceSvg) => boolean) {}

/**
* Install this action as a context menu item.
*/
install() {
this.registerContextMenuAction();
}

/**
* Uninstall this action as both a keyboard shortcut and a context menu item.
* Reinstall the original context menu action if possible.
*/
uninstall() {
ContextMenuRegistry.registry.unregister('edit');
}

/**
* Register the edit block action as a context menu item on blocks.
*/
private registerContextMenuAction() {
const editAboveItem: ContextMenuRegistry.RegistryItem = {
displayText: 'Edit Block contents (→︎)',
preconditionFn: (scope: ContextMenuRegistry.Scope) => {
const workspace = scope.block?.workspace;
if (!workspace || !this.canCurrentlyNavigate(workspace)) {
return 'disabled';
}
const cursor = workspace.getCursor() as LineCursor | null;
if (!cursor) return 'disabled';
return cursor.atEndOfLine() ? 'hidden' : 'enabled';
},
callback: (scope: ContextMenuRegistry.Scope) => {
const workspace = scope.block?.workspace;
if (!workspace) return false;
workspace.getCursor()?.in();
return true;
},
scopeType: ContextMenuRegistry.ScopeType.BLOCK,
id: 'edit',
weight: 10,
};

ContextMenuRegistry.registry.register(editAboveItem);
}
}
21 changes: 21 additions & 0 deletions src/line_cursor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,27 @@ export class LineCursor extends Marker {
return newNode;
}

/**
* Returns true iff the node to which we would navigate if in() were
* called, which will be a validInLineNode, is also a validLineNode
* - in effect, if the LineCursor is at the end of the 'current
* line' of the program.
*
* This is used to determine whether to display an "Edit Block
* contents (→︎)" entry in the block context menu, to help users
* discover that the right arrow key can be used to navigate to
* block contents.
*/
public atEndOfLine(): boolean {
const curNode = this.getCurNode();
if (!curNode) return false;
const rightNode = this.getNextNode(
curNode,
this.validInLineNode.bind(this),
);
return this.validLineNode(rightNode);
}

/**
* Returns true iff the given node represents the "beginning of a
* new line of code" (and thus can be visited by pressing the
Expand Down
14 changes: 10 additions & 4 deletions src/navigation_controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,14 @@ import {
} from 'blockly/core';

import * as Constants from './constants';
import {Navigation} from './navigation';
import {Announcer} from './announcer';
import {LineCursor} from './line_cursor';
import {ShortcutDialog} from './shortcut_dialog';
import {Clipboard} from './actions/clipboard';
import {DeleteAction} from './actions/delete';
import {EditAction} from './actions/edit';
import {InsertAction} from './actions/insert';
import {Clipboard} from './actions/clipboard';
import {LineCursor} from './line_cursor';
import {Navigation} from './navigation';
import {ShortcutDialog} from './shortcut_dialog';
import {WorkspaceMovement} from './actions/ws_movement';

const KeyCodes = BlocklyUtils.KeyCodes;
Expand All @@ -50,6 +51,9 @@ export class NavigationController {
this.canCurrentlyEdit.bind(this),
);

/** Context menu and keyboard action for deletion. */
editAction: EditAction = new EditAction(this.canCurrentlyEdit.bind(this));

/** Context menu and keyboard action for insertion. */
insertAction: InsertAction = new InsertAction(
this.navigation,
Expand Down Expand Up @@ -678,6 +682,7 @@ export class NavigationController {
ShortcutRegistry.registry.register(shortcut);
}
this.deleteAction.install();
this.editAction.install();
this.insertAction.install();
this.workspaceMovement.install();

Expand All @@ -698,6 +703,7 @@ export class NavigationController {
}

this.deleteAction.uninstall();
this.editAction.uninstall();
this.insertAction.uninstall();
this.clipboard.uninstall();
this.workspaceMovement.uninstall();
Expand Down