8000 fix: Synchronize gestures and focus by BenHenning · Pull Request #8981 · google/blockly · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

fix: Synchronize gestures and focus #8981

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.

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
Show file tree
Hide file tree
Changes from all 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 @@ -1290,6 +1291,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 @@ -1306,6 +1308,13 @@ export class BlockSvg
if (blockOnly) break;
block = block.getParent();
} while (block);
if (previouslyFocused) {
// 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
12 changes: 10 additions & 2 deletions core/clipboard/block_paster.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,14 @@
*/

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 * as renderManagement from '../render_management.js';
import {State, append} from '../serialization/blocks.js';
import {Coordinate} from '../utils/coordinate.js';
import {WorkspaceSvg} from '../workspace_svg.js';
Expand Down Expand Up @@ -55,7 +57,13 @@ 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;
renderManagement
.finishQueuedRenders()
.then(() => getFocusManager().focusNode(nodeToFocus));
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
// 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();
>
Expand All @@ -648,10 +644,6 @@ export function hideWithoutAnimation() {
clearTimeout(animateOutTimer);
}

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

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

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

/**
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;
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
0