Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
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
9 changes: 9 additions & 0 deletions core/block_svg.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ import type {BlockMove} from './events/events_block_move.js';
import {EventType} from './events/type.js';
import * as eventUtils from './events/utils.js';
import {FieldLabel} from './field_label.js';
import {getFocusManager} from './focus_manager.js';
import {IconType} from './icons/icon_types.js';
import {MutatorIcon} from './icons/mutator_icon.js';
import {WarningIcon} from './icons/warning_icon.js';
Expand Down Expand Up @@ -1268,6 +1269,7 @@ export class BlockSvg
* adjusting its parents.
*/
bringToFront(blockOnly = false) {
const previouslyFocused = getFocusManager().getFocusedNode();
/* eslint-disable-next-line @typescript-eslint/no-this-alias */
let block: this | null = this;
if (block.isDeadOrDying()) {
Expand All @@ -1284,6 +1286,13 @@ export class BlockSvg
if (blockOnly) break;
block = block.getParent();
} while (block);
if (previouslyFocused) {
Comment thread
BenHenning marked this conversation as resolved.
// Bringing a block to the front of the stack doesn't fundamentally change
// the logical structure of the page, but it does change element ordering
// which can take automatically take away focus from a node. Ensure focus
// is restored to avoid a discontinuity.
getFocusManager().focusNode(previouslyFocused);
}
}

/**
Expand Down
9 changes: 7 additions & 2 deletions core/clipboard/block_paster.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,11 @@
*/

import {BlockSvg} from '../block_svg.js';
import * as common from '../common.js';
import {IFocusableNode} from '../blockly.js';
import {config} from '../config.js';
import {EventType} from '../events/type.js';
import * as eventUtils from '../events/utils.js';
import {getFocusManager} from '../focus_manager.js';
import {ICopyData} from '../interfaces/i_copyable.js';
import {IPaster} from '../interfaces/i_paster.js';
import {State, append} from '../serialization/blocks.js';
Expand Down Expand Up @@ -55,7 +56,11 @@ export class BlockPaster implements IPaster<BlockCopyData, BlockSvg> {
if (eventUtils.isEnabled() && !block.isShadow()) {
eventUtils.fire(new (eventUtils.get(EventType.BLOCK_CREATE))(block));
}
common.setSelected(block);

// Sometimes there's a delay before the block is fully created and ready for
// focusing, so wait slightly before focusing the newly pasted block.
const nodeToFocus: IFocusableNode = block;
setTimeout(() => getFocusManager().focusNode(nodeToFocus), 1);
Comment thread
BenHenning marked this conversation as resolved.
Outdated
return block;
}
}
Expand Down
8 changes: 8 additions & 0 deletions core/dragging/dragger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,13 @@ import * as blockAnimations from '../block_animations.js';
import {BlockSvg} from '../block_svg.js';
import {ComponentManager} from '../component_manager.js';
import * as eventUtils from '../events/utils.js';
import {getFocusManager} from '../focus_manager.js';
import {IDeletable, isDeletable} from '../interfaces/i_deletable.js';
import {IDeleteArea} from '../interfaces/i_delete_area.js';
import {IDragTarget} from '../interfaces/i_drag_target.js';
import {IDraggable} from '../interfaces/i_draggable.js';
import {IDragger} from '../interfaces/i_dragger.js';
import {isFocusableNode} from '../interfaces/i_focusable_node.js';
import * as registry from '../registry.js';
import {Coordinate} from '../utils/coordinate.js';
import {WorkspaceSvg} from '../workspace_svg.js';
Expand Down Expand Up @@ -129,6 +131,12 @@ export class Dragger implements IDragger {
root.dispose();
}
eventUtils.setGroup(false);

if (!wouldDelete && isFocusableNode(this.draggable)) {
// Ensure focusable nodes that have finished dragging (but aren't being)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

"(but aren't being) deleted" -> "(but aren't being deleted)"

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Nice catch--fixed!

// deleted end with focus and selection.
getFocusManager().focusNode(this.draggable);
}
}

// We need to special case blocks for now so that we look at the root block
Expand Down
13 changes: 5 additions & 8 deletions core/dropdowndiv.ts
Original file line number Diff line number Diff line change
Expand Up @@ -629,10 +629,6 @@ export function hide() {
animateOutTimer = setTimeout(function () {
hideWithoutAnimation();
}, ANIMATION_TIME * 1000);
if (returnEphemeralFocus) {
returnEphemeralFocus();
returnEphemeralFocus = null;
}
if (onHide) {
onHide();
onHide = null;
Expand All @@ -648,10 +644,6 @@ export function hideWithoutAnimation() {
clearTimeout(animateOutTimer);
}

if (returnEphemeralFocus) {
returnEphemeralFocus();
returnEphemeralFocus = null;
}
if (onHide) {
onHide();
onHide = null;
Expand All @@ -660,6 +652,11 @@ export function hideWithoutAnimation() {
owner = null;

(common.getMainWorkspace() as WorkspaceSvg).markFocused();

if (returnEphemeralFocus) {
returnEphemeralFocus();
returnEphemeralFocus = null;
}
}

/**
Expand Down
4 changes: 2 additions & 2 deletions core/field_dropdown.ts
Original file line number Diff line number Diff line change
Expand Up @@ -769,7 +769,7 @@ export class FieldDropdown extends Field<string> {
} else if (typeof option[1] !== 'string') {
foundError = true;
console.error(
`Invalid option[${i}]: Each FieldDropdown option id must be a string.
`Invalid option[${i}]: Each FieldDropdown option id must be a string.
Comment thread
BenHenning marked this conversation as resolved.
Outdated
Found ${option[1]} in: ${option}`,
);
} else if (
Expand All @@ -780,7 +780,7 @@ export class FieldDropdown extends Field<string> {
) {
foundError = true;
console.error(
`Invalid option[${i}]: Each FieldDropdown option must have a string
`Invalid option[${i}]: Each FieldDropdown option must have a string
label, image description, or HTML element. Found ${option[0]} in: ${option}`,
);
}
Expand Down
40 changes: 35 additions & 5 deletions core/focus_manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,17 +56,20 @@ export class FocusManager {
*/
static readonly PASSIVE_FOCUS_NODE_CSS_CLASS_NAME = 'blocklyPassiveFocus';

focusedNode: IFocusableNode | null = null;
registeredTrees: Array<IFocusableTree> = [];
private focusedNode: IFocusableNode | null = null;
private previouslyFocusedNode: IFocusableNode | null = null;
private registeredTrees: Array<IFocusableTree> = [];

private currentlyHoldsEphemeralFocus: boolean = false;
private lockFocusStateChanges: boolean = false;
private recentlyLostAllFocus: boolean = false;

constructor(
addGlobalEventListener: (type: string, listener: EventListener) => void,
) {
// Note that 'element' here is the element *gaining* focus.
const maybeFocus = (element: Element | EventTarget | null) => {
this.recentlyLostAllFocus = !element;
let newNode: IFocusableNode | null | undefined = null;
if (element instanceof HTMLElement || element instanceof SVGElement) {
// If the target losing or gaining focus maps to any tree, then it
Expand Down Expand Up @@ -164,7 +167,7 @@ export class FocusManager {
const root = tree.getRootFocusableNode();
if (focusedNode) this.removeHighlight(focusedNode);
if (this.focusedNode === focusedNode || this.focusedNode === root) {
this.focusedNode = null;
this.updateFocusedNode(null);
}
this.removeHighlight(root);
}
Expand Down Expand Up @@ -277,7 +280,7 @@ export class FocusManager {
// Only change the actively focused node if ephemeral state isn't held.
this.activelyFocusNode(focusableNode, prevTree ?? null);
}
this.focusedNode = focusableNode;
this.updateFocusedNode(focusableNode);
}

/**
Expand Down Expand Up @@ -328,6 +331,22 @@ export class FocusManager {

if (this.focusedNode) {
this.activelyFocusNode(this.focusedNode, null);

// Even though focus was restored, check if it's lost again. It's
// possible for the browser to force focus away from all elements once
// the ephemeral element disappears. This ensures focus is restored.
const capturedNode = this.focusedNode;
setTimeout(() => {
// These checks are set up to minimize the risk that a legitimate
// focus change occurred within the delay that this would override.
if (
!this.focusedNode &&
this.previouslyFocusedNode === capturedNode &&
this.recentlyLostAllFocus
) {
this.focusNode(capturedNode);
}
}, 0);
}
};
}
Expand All @@ -348,6 +367,17 @@ export class FocusManager {
}
}

/**
* Updates the internally tracked focused node to the specified node, or null
* if focus is being lost. This also updates previous focus tracking.
*
* @param newFocusedNode The new node to set as focused.
*/
private updateFocusedNode(newFocusedNode: IFocusableNode | null) {
this.previouslyFocusedNode = this.focusedNode;
this.focusedNode = newFocusedNode;
}

/**
* Defocuses the current actively focused node tracked by the manager, iff
* there's a node being tracked and the manager doesn't have ephemeral focus.
Expand All @@ -358,7 +388,7 @@ export class FocusManager {
// restored upon exiting ephemeral focus mode.
if (this.focusedNode && !this.currentlyHoldsEphemeralFocus) {
this.passivelyFocusNode(this.focusedNode, null);
this.focusedNode = null;
this.updateFocusedNode(null);
}
}

Expand Down
33 changes: 22 additions & 11 deletions core/gesture.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import * as dropDownDiv from './dropdowndiv.js';
import {EventType} from './events/type.js';
import * as eventUtils from './events/utils.js';
import type {Field} from './field.js';
import {getFocusManager} from './focus_manager.js';
import type {IBubble} from './interfaces/i_bubble.js';
import {IDraggable, isDraggable} from './interfaces/i_draggable.js';
import {IDragger} from './interfaces/i_dragger.js';
Expand Down Expand Up @@ -289,7 +290,7 @@ export class Gesture {
// The start block is no longer relevant, because this is a drag.
this.startBlock = null;
this.targetBlock = this.flyout.createBlock(this.targetBlock);
common.setSelected(this.targetBlock);
getFocusManager().focusNode(this.targetBlock);
return true;
}
return false;
Expand Down Expand Up @@ -734,6 +735,7 @@ export class Gesture {
this.startComment.showContextMenu(e);
} else if (this.startWorkspace_ && !this.flyout) {
this.startWorkspace_.hideChaff();
getFocusManager().focusNode(this.startWorkspace_);
this.startWorkspace_.showContextMenu(e);
}

Expand Down Expand Up @@ -762,9 +764,12 @@ export class Gesture {
this.mostRecentEvent = e;

if (!this.startBlock && !this.startBubble && !this.startComment) {
// Selection determines what things start drags. So to drag the workspace,
// we need to deselect anything that was previously selected.
common.setSelected(null);
// Ensure the workspace is selected if nothing else should be. Note that
// this is focusNode() instead of focusTree() because if any active node
// is focused in the workspace it should be defocused.
getFocusManager().focusNode(ws);
} else if (this.startBlock) {
getFocusManager().focusNode(this.startBlock);
}

this.doStart(e);
Expand Down Expand Up @@ -865,13 +870,18 @@ export class Gesture {
);
}

// Note that the order is important here: bringing a block to the front will
// cause it to become focused and showing the field editor will capture
// focus ephemerally. It's important to ensure that focus is properly
// restored back to the block after field editing has completed.
this.bringBlockToFront();

// Only show the editor if the field's editor wasn't already open
// right before this gesture started.
const dropdownAlreadyOpen = this.currentDropdownOwner === this.startField;
if (!dropdownAlreadyOpen) {
this.startField.showEditor(this.mostRecentEvent);
}
this.bringBlockToFront();
}

/** Execute an icon click. */
Expand Down Expand Up @@ -901,6 +911,9 @@ export class Gesture {
const newBlock = this.flyout.createBlock(this.targetBlock);
newBlock.snapToGrid();
newBlock.bumpNeighbours();

// If a new block was added, make sure that it's correctly focused.
getFocusManager().focusNode(newBlock);
}
} else {
if (!this.startWorkspace_) {
Expand Down Expand Up @@ -928,11 +941,7 @@ export class Gesture {
* @param _e A pointerup event.
*/
private doWorkspaceClick(_e: PointerEvent) {
const ws = this.creatorWorkspace;
Comment thread
BenHenning marked this conversation as resolved.
if (common.getSelected()) {
common.getSelected()!.unselect();
}
this.fireWorkspaceClick(this.startWorkspace_ || ws);
this.fireWorkspaceClick(this.startWorkspace_ || this.creatorWorkspace);
}

/* End functions defining what actions to take to execute clicks on each type
Expand All @@ -947,6 +956,8 @@ export class Gesture {
private bringBlockToFront() {
// Blocks in the flyout don't overlap, so skip the work.
if (this.targetBlock && !this.flyout) {
// Always ensure the block being dragged/clicked has focus.
getFocusManager().focusNode(this.targetBlock);
this.targetBlock.bringToFront();
}
}
Expand Down Expand Up @@ -1023,7 +1034,6 @@ export class Gesture {
// If the gesture already went through a bubble, don't set the start block.
if (!this.startBlock && !this.startBubble) {
this.startBlock = block;
common.setSelected(this.startBlock);
if (block.isInFlyout && block !== block.getRootBlock()) {
this.setTargetBlock(block.getRootBlock());
} else {
Expand All @@ -1046,6 +1056,7 @@ export class Gesture {
this.setTargetBlock(block.getParent()!);
} else {
this.targetBlock = block;
getFocusManager().focusNode(block);
}
}

Expand Down
9 changes: 5 additions & 4 deletions core/widgetdiv.ts
Original file line number Diff line number Diff line change
Expand Up @@ -131,10 +131,6 @@ export function hide() {
div.style.display = 'none';
div.style.left = '';
div.style.top = '';
if (returnEphemeralFocus) {
returnEphemeralFocus();
returnEphemeralFocus = null;
}
if (dispose) {
dispose();
dispose = null;
Expand All @@ -150,6 +146,11 @@ export function hide() {
themeClassName = '';
}
(common.getMainWorkspace() as WorkspaceSvg).markFocused();

if (returnEphemeralFocus) {
returnEphemeralFocus();
returnEphemeralFocus = null;
}
}

/**
Expand Down