diff --git a/src/actions/action_menu.ts b/src/actions/action_menu.ts index 5eab1325..584624d7 100644 --- a/src/actions/action_menu.ts +++ b/src/actions/action_menu.ts @@ -111,7 +111,7 @@ export class ActionMenu { const cursor = workspace.getCursor(); if (!cursor) throw new Error('workspace has no cursor'); const node = cursor.getCurNode(); - if (!node) throw new Error('No node is currently selected'); + if (!node) return false; const nodeType = node.getType(); switch (nodeType) { case ASTNode.types.BLOCK: diff --git a/src/index.ts b/src/index.ts index b0060689..7decd1fb 100644 --- a/src/index.ts +++ b/src/index.ts @@ -30,6 +30,9 @@ export class KeyboardNavigation { /** Event handler run when the workspace loses focus. */ private blurListener: (e: Event) => void; + /** Event handler run when the widget or dropdown div loses focus. */ + private widgetDropDownDivFocusOutListener: (e: Event) => void; + /** Event handler run when the toolbox gains focus. */ private toolboxFocusListener: () => void; @@ -142,6 +145,22 @@ export class KeyboardNavigation { workspace.getSvgGroup().addEventListener('focus', this.focusListener); workspace.getSvgGroup().addEventListener('blur', this.blurListener); + this.widgetDropDownDivFocusOutListener = (e: Event) => { + this.navigationController.handleFocusOutWidgetDropdownDiv( + workspace, + (e as FocusEvent).relatedTarget, + ); + }; + + Blockly.WidgetDiv.getDiv()?.addEventListener( + 'focusout', + this.widgetDropDownDivFocusOutListener, + ); + Blockly.DropDownDiv.getContentDiv()?.addEventListener( + 'focusout', + this.widgetDropDownDivFocusOutListener, + ); + const toolboxElement = getToolboxElement(workspace); this.toolboxFocusListener = () => { this.navigationController.handleFocusToolbox(workspace); @@ -197,6 +216,15 @@ export class KeyboardNavigation { .getSvgGroup() .removeEventListener('focus', this.focusListener); + Blockly.WidgetDiv.getDiv()?.removeEventListener( + 'focusout', + this.widgetDropDownDivFocusOutListener, + ); + Blockly.DropDownDiv.getContentDiv()?.removeEventListener( + 'focusout', + this.widgetDropDownDivFocusOutListener, + ); + const toolboxElement = getToolboxElement(this.workspace); toolboxElement?.removeEventListener('focus', this.toolboxFocusListener); toolboxElement?.removeEventListener('blur', this.toolboxBlurListener); diff --git a/src/line_cursor.ts b/src/line_cursor.ts index 5bc2848c..54508590 100644 --- a/src/line_cursor.ts +++ b/src/line_cursor.ts @@ -15,7 +15,7 @@ import * as Blockly from 'blockly/core'; import {ASTNode, Marker} from 'blockly/core'; -import {getWorkspaceElement, scrollBoundsIntoView} from './workspace_utilities'; +import {scrollBoundsIntoView} from './workspace_utilities'; /** Options object for LineCursor instances. */ export type CursorOptions = { @@ -61,7 +61,7 @@ export class LineCursor extends Marker { ) { super(); // Bind selectListener to facilitate future install/uninstall. - this.selectListener = this.selectListener.bind(this); + this.changeListener = this.changeListener.bind(this); // Regularise options and apply defaults. this.options = {...defaultOptions, ...options}; @@ -81,7 +81,7 @@ export class LineCursor extends Marker { markerManager.setCursor(this); const oldCursorNode = this.oldCursor?.getCurNode(); if (oldCursorNode) this.setCurNode(oldCursorNode); - this.workspace.addChangeListener(this.selectListener); + this.workspace.addChangeListener(this.changeListener); this.installed = true; } @@ -92,7 +92,7 @@ export class LineCursor extends Marker { */ uninstall() { if (!this.installed) throw new Error('LineCursor not yet installed'); - this.workspace.removeChangeListener(this.selectListener.bind(this)); + this.workspace.removeChangeListener(this.changeListener.bind(this)); if (this.oldCursor) { this.workspace.getMarkerManager().setCursor(this.oldCursor); } @@ -475,6 +475,21 @@ export class LineCursor extends Marker { throw new Error('no valid nodes in this.potentialNodes'); } + /** + * Get the current location of the cursor. + * + * Overrides normal Marker getCurNode to update the current node from the selected + * block. This typically happens via the selection listener but that is not called + * immediately when `Gesture` calls `Blockly.common.setSelected`. + * In particular the listener runs after showing the context menu. + * + * @returns The current field, connection, or block the cursor is on. + */ + override getCurNode(): Blockly.ASTNode | null { + this.updateCurNodeFromSelection(); + return super.getCurNode(); + } + /** * Sets the object in charge of drawing the marker. * @@ -516,28 +531,11 @@ export class LineCursor extends Marker { * this.drawMarker() instead of this.drawer.draw() directly. * * @param newNode The new location of the cursor. + * @param selectionUpToDate If false (the default) we'll update the selection too. */ - override setCurNode(newNode: ASTNode | null, selectionInSync = false) { - if (newNode?.getLocation() === this.getCurNode()?.getLocation()) { - return; - } - if (!selectionInSync) { - if ( - newNode?.getType() === ASTNode.types.BLOCK && - !(newNode.getLocation() as Blockly.BlockSvg).isShadow() - ) { - if (Blockly.common.getSelected() !== newNode.getLocation()) { - Blockly.Events.disable(); - Blockly.common.setSelected(newNode.getLocation() as Blockly.BlockSvg); - Blockly.Events.enable(); - } - } else { - if (Blockly.common.getSelected()) { - Blockly.Events.disable(); - Blockly.common.setSelected(null); - Blockly.Events.enable(); - } - } + override setCurNode(newNode: ASTNode | null, selectionUpToDate = false) { + if (!selectionUpToDate) { + this.updateSelectionFromNode(newNode); } super.setCurNode(newNode); @@ -616,6 +614,7 @@ export class LineCursor extends Marker { // Selection should already be in sync. } else { block.addSelect(); + block.getParent()?.removeSelect(); } } @@ -671,23 +670,98 @@ export class LineCursor extends Marker { } /** - * Event listener that syncs the cursor location to the selected - * block on SELECTED events. + * Event listener that syncs the cursor location to the selected block on + * SELECTED events. + * + * This does not run early enough in all cases so `getCurNode()` also updates + * the node from the selection. + * + * @param event The `Selected` event. */ - private selectListener(event: Blockly.Events.Abstract) { - if (event.type !== Blockly.Events.SELECTED) return; - const selectedEvent = event as Blockly.Events.Selected; - if (selectedEvent.workspaceId !== this.workspace.id) return; - if (selectedEvent.newElementId) { - const block = this.workspace.getBlockById(selectedEvent.newElementId); - if (block) { - const node = ASTNode.createBlockNode(block); - if (node) { - this.setCurNode(node, true); + private changeListener(event: Blockly.Events.Abstract) { + switch (event.type) { + case Blockly.Events.SELECTED: + this.updateCurNodeFromSelection(); + break; + case Blockly.Events.CLICK: { + const click = event as Blockly.Events.Click; + if ( + click.workspaceId === this.workspace.id && + click.targetType === Blockly.Events.ClickTarget.WORKSPACE + ) { + this.setCurNode(null); } } + } + } + + /** + * Updates the current node to match the selection. + * + * Clears the current node if it's on a block but the selection is null. + * Sets the node to a block if selected for our workspace. + * For shadow blocks selections the parent is used by default (unless we're + * already on the shadow block via keyboard) as that's where the visual + * selection is. + */ + private updateCurNodeFromSelection() { + const curNode = super.getCurNode(); + const selected = Blockly.common.getSelected(); + + if ( + selected === null && + curNode?.getType() === Blockly.ASTNode.types.BLOCK + ) { + this.setCurNode(null, true); + return; + } + if (selected?.workspace !== this.workspace) { + return; + } + if (selected instanceof Blockly.BlockSvg) { + let block: Blockly.BlockSvg | null = selected; + if (selected.isShadow()) { + // OK if the current node is on the parent OR the shadow block. + // The former happens for clicks, the latter for keyboard nav. + if ( + curNode && + (curNode.getLocation() === block || + curNode.getLocation() === block.getParent()) + ) { + return; + } + block = block.getParent(); + } + if (block) { + this.setCurNode(Blockly.ASTNode.createBlockNode(block)!, true); + } + } + } + + /** + * Updates the selection from the node. + * + * Clears the selection for non-block nodes. + * Clears the selection for shadow blocks as the selection is drawn on + * the parent but the cursor will be drawn on the shadow block itself. + * We need to take care not to later clear the current node due to that null + * selection, so we track the latest selection we're in sync with. + * + * @param newNode The new node. + */ + private updateSelectionFromNode(newNode: Blockly.ASTNode | null) { + if (newNode?.getType() === ASTNode.types.BLOCK) { + if (Blockly.common.getSelected() !== newNode.getLocation()) { + Blockly.Events.disable(); + Blockly.common.setSelected(newNode.getLocation() as Blockly.BlockSvg); + Blockly.Events.enable(); + } } else { - this.setCurNode(null as never, true); + if (Blockly.common.getSelected()) { + Blockly.Events.disable(); + Blockly.common.setSelected(null); + Blockly.Events.enable(); + } } } } diff --git a/src/navigation.ts b/src/navigation.ts index 5f879aae..bb02f1a1 100644 --- a/src/navigation.ts +++ b/src/navigation.ts @@ -415,14 +415,22 @@ export class Navigation { } /** - * Clears the navigation state and switches to using the passive focus indicator. + * Clears navigation state and switches to using the passive focus indicator + * if it is not the context menu / field input that is causing blur. * * @param workspace The workspace that has lost focus. + * @param ignorePopUpDivs Whether to skip the focus indicator change when + * the widget/dropdown divs are open. */ - handleBlurWorkspace(workspace: Blockly.WorkspaceSvg) { + handleBlurWorkspace( + workspace: Blockly.WorkspaceSvg, + ignorePopUpDivs = false, + ) { this.setState(workspace, Constants.STATE.NOWHERE); const cursor = workspace.getCursor(); - if (cursor) { + const popUpDivsShowing = + Blockly.WidgetDiv.isVisible() || Blockly.DropDownDiv.isVisible(); + if (cursor && (ignorePopUpDivs || !popUpDivsShowing)) { const curNode = cursor.getCurNode(); if (curNode) { this.passiveFocusIndicator.show(curNode); @@ -432,6 +440,32 @@ export class Navigation { } } + /** + * Handle the widget or dropdown div losing focus (via focusout). + * + * Because we skip the widget/dropdown div cases in `handleBlurWorkspace` we need + * to catch them here. + * + * @param workspace The workspace. + * @param relatedTarget The related target (newly focused element if any). + */ + handleFocusOutWidgetDropdownDiv( + workspace: Blockly.WorkspaceSvg, + relatedTarget: EventTarget | null, + ) { + if (relatedTarget === null) { + // Workaround: + // Skip document.body/null case until this blur bug is fixed to avoid + // flipping to passive focus as the user moves their mouse over the + // colour picker. + // https://github.com/google/blockly-samples/issues/2498 + return; + } + if (relatedTarget !== getWorkspaceElement(workspace)) { + this.handleBlurWorkspace(workspace, true); + } + } + /** * Sets browser focus to the toolbox (if any). * diff --git a/src/navigation_controller.ts b/src/navigation_controller.ts index b5c6c9ab..9d9d2c62 100644 --- a/src/navigation_controller.ts +++ b/src/navigation_controller.ts @@ -196,6 +196,13 @@ export class NavigationController { this.navigation.handleBlurWorkspace(workspace); } + handleFocusOutWidgetDropdownDiv( + workspace: Blockly.WorkspaceSvg, + relatedTarget: EventTarget | null, + ) { + this.navigation.handleFocusOutWidgetDropdownDiv(workspace, relatedTarget); + } + focusToolbox(workspace: Blockly.WorkspaceSvg) { this.navigation.focusToolbox(workspace); }