Skip to content

Commit 9809ff2

Browse files
authored
fix: Fix when the context menu is allowed to open (#550)
* Work on fixing menu shortcuts * Add tests and fix case where ws is flyout * Add tests for real * Remove unused import * Update tests to check for menu existence * Add param description * reuse node
1 parent 24a1f1c commit 9809ff2

File tree

4 files changed

+145
-8
lines changed

4 files changed

+145
-8
lines changed

src/actions/action_menu.ts

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,12 +45,23 @@ export class ActionMenu {
4545
private registerShortcut() {
4646
const menuShortcut: ShortcutRegistry.KeyboardShortcut = {
4747
name: Constants.SHORTCUT_NAMES.MENU,
48-
preconditionFn: (workspace) =>
49-
this.navigation.canCurrentlyNavigate(workspace),
48+
preconditionFn: (workspace) => {
49+
return (
50+
this.navigation.canCurrentlyNavigate(workspace) &&
51+
!workspace.isDragging()
52+
);
53+
},
5054
callback: (workspace) => {
5155
switch (this.navigation.getState(workspace)) {
5256
case Constants.STATE.WORKSPACE:
5357
return this.openActionMenu(workspace);
58+
case Constants.STATE.FLYOUT: {
59+
const flyoutWorkspace = workspace.getFlyout()?.getWorkspace();
60+
if (flyoutWorkspace) {
61+
return this.openActionMenu(flyoutWorkspace);
62+
}
63+
return false;
64+
}
5465
default:
5566
return false;
5667
}

src/navigation.ts

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -99,19 +99,17 @@ export class Navigation {
9999
getState(workspace: Blockly.WorkspaceSvg): Constants.STATE {
100100
const focusedTree = Blockly.getFocusManager().getFocusedTree();
101101
if (focusedTree instanceof Blockly.WorkspaceSvg) {
102-
if (focusedTree.isFlyout && workspace === focusedTree.targetWorkspace) {
102+
if (focusedTree.isFlyout) {
103103
return Constants.STATE.FLYOUT;
104-
} else if (workspace === focusedTree) {
104+
} else {
105105
return Constants.STATE.WORKSPACE;
106106
}
107107
} else if (focusedTree instanceof Blockly.Toolbox) {
108108
if (workspace === focusedTree.getWorkspace()) {
109109
return Constants.STATE.TOOLBOX;
110110
}
111111
} else if (focusedTree instanceof Blockly.Flyout) {
112-
if (workspace === focusedTree.targetWorkspace) {
113-
return Constants.STATE.FLYOUT;
114-
}
112+
return Constants.STATE.FLYOUT;
115113
}
116114
// Either a non-Blockly element currently has DOM focus, or a different
117115
// workspace holds it.
@@ -822,8 +820,11 @@ export class Navigation {
822820
* @returns whether keyboard navigation is currently allowed.
823821
*/
824822
canCurrentlyNavigate(workspace: Blockly.WorkspaceSvg) {
823+
const accessibilityMode = workspace.isFlyout
824+
? workspace.targetWorkspace?.keyboardAccessibilityMode
825+
: workspace.keyboardAccessibilityMode;
825826
return (
826-
workspace.keyboardAccessibilityMode &&
827+
!!accessibilityMode &&
827828
this.getState(workspace) !== Constants.STATE.NOWHERE
828829
);
829830
}

test/webdriverio/test/actions_test.ts

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
/**
2+
* @license
3+
* Copyright 2025 Google LLC
4+
* SPDX-License-Identifier: Apache-2.0
5+
*/
6+
7+
import * as chai from 'chai';
8+
import {Key} from 'webdriverio';
9+
import {
10+
contextMenuExists,
11+
moveToToolboxCategory,
12+
PAUSE_TIME,
13+
setCurrentCursorNodeById,
14+
tabNavigateToWorkspace,
15+
testFileLocations,
16+
testSetup,
17+
} from './test_setup.js';
18+
19+
suite('Menus test', function () {
20+
// Setting timeout to unlimited as these tests take longer time to run
21+
this.timeout(0);
22+
23+
// Clear the workspace and load start blocks
24+
setup(async function () {
25+
this.browser = await testSetup(testFileLocations.BASE);
26+
await this.browser.pause(PAUSE_TIME);
27+
});
28+
29+
test('Menu action opens menu', async function () {
30+
// Navigate to draw_circle_1.
31+
await tabNavigateToWorkspace(this.browser);
32+
await setCurrentCursorNodeById(this.browser, 'draw_circle_1');
33+
await this.browser.pause(PAUSE_TIME);
34+
await this.browser.keys([Key.Ctrl, Key.Return]);
35+
await this.browser.pause(PAUSE_TIME);
36+
chai.assert.isTrue(
37+
await contextMenuExists(this.browser, 'Duplicate'),
38+
'The menu should be openable on a block',
39+
);
40+
});
41+
42+
test('Menu action returns true in the toolbox', async function () {
43+
// Navigate to draw_circle_1.
44+
await tabNavigateToWorkspace(this.browser);
45+
await setCurrentCursorNodeById(this.browser, 'draw_circle_1');
46+
// Navigate to a toolbox category
47+
await moveToToolboxCategory(this.browser, 'Functions');
48+
// Move to flyout.
49+
await this.browser.keys(Key.ArrowRight);
50+
await this.browser.keys([Key.Ctrl, Key.Return]);
51+
await this.browser.pause(PAUSE_TIME);
52+
53+
chai.assert.isTrue(
54+
await contextMenuExists(this.browser, 'Help'),
55+
'The menu should be openable on a block in the toolbox',
56+
);
57+
});
58+
59+
test('Menu action returns false during drag', async function () {
60+
// Navigate to draw_circle_1.
61+
await tabNavigateToWorkspace(this.browser);
62+
await setCurrentCursorNodeById(this.browser, 'draw_circle_1');
63+
// Start moving the block
64+
await this.browser.keys('m');
65+
await this.browser.keys([Key.Ctrl, Key.Return]);
66+
await this.browser.pause(PAUSE_TIME);
67+
chai.assert.isTrue(
68+
await contextMenuExists(this.browser, 'Duplicate', true),
69+
'The menu should not be openable during a move',
70+
);
71+
});
72+
});

test/webdriverio/test/test_setup.ts

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -431,3 +431,56 @@ export async function isDragging(
431431
return workspaceSvg.isDragging();
432432
});
433433
}
434+
435+
/**
436+
* Returns the result of the specificied action precondition.
437+
*
438+
* @param browser The active WebdriverIO Browser object.
439+
* @param action The action to check the precondition for.
440+
*/
441+
export async function checkActionPrecondition(
442+
browser: WebdriverIO.Browser,
443+
action: string,
444+
): Promise<boolean> {
445+
return await browser.execute((action) => {
446+
const node = Blockly.getFocusManager().getFocusedNode();
447+
let workspace;
448+
if (node instanceof Blockly.BlockSvg) {
449+
workspace = node.workspace as Blockly.WorkspaceSvg;
450+
} else if (node instanceof Blockly.Workspace) {
451+
workspace = node as Blockly.WorkspaceSvg;
452+
} else if (node instanceof Blockly.Field) {
453+
workspace = node.getSourceBlock()?.workspace as Blockly.WorkspaceSvg;
454+
}
455+
456+
if (!workspace) {
457+
throw new Error('Unable to derive workspace from focused node');
458+
}
459+
const actionItem = Blockly.ShortcutRegistry.registry.getRegistry()[action];
460+
if (!actionItem || !actionItem.preconditionFn) {
461+
throw new Error(
462+
`No registered action or missing precondition: ${action}`,
463+
);
464+
}
465+
return actionItem.preconditionFn(workspace, {
466+
focusedNode: node ?? undefined,
467+
});
468+
}, action);
469+
}
470+
471+
/**
472+
* Wait for the specified context menu item to exist.
473+
*
474+
* @param browser The active WebdriverIO Browser object.
475+
* @param itemText The display text of the context menu item to click.
476+
* @param reverse Whether to check for non-existence instead.
477+
* @return A Promise that resolves when the actions are completed.
478+
*/
479+
export async function contextMenuExists(
480+
browser: WebdriverIO.Browser,
481+
itemText: string,
482+
reverse = false,
483+
): Promise<boolean> {
484+
const item = await browser.$(`div=${itemText}`);
485+
return await item.waitForExist({timeout: 200, reverse: reverse});
486+
}

0 commit comments

Comments
 (0)