Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 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
28 changes: 28 additions & 0 deletions src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down
62 changes: 46 additions & 16 deletions src/line_cursor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -475,6 +475,43 @@ 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. We typically update the node from the selection via a 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(): ASTNode | null {
const curNode = super.getCurNode();
let selected = Blockly.common.getSelected();
if (
selected === null &&
curNode?.getType() === Blockly.ASTNode.types.BLOCK
) {
// Selection says our curNode is not selected anymore.
// this.setCurNode(null as never, true);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oops, this is definitely a problem. Re-enabling it threw up a shadow block issue addressed in 1233f57

Copy link
Collaborator

Choose a reason for hiding this comment

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

I actually missed this commented out bit the first time around--great catch and thanks for fixing it in the follow-up!

return super.getCurNode();
}
if (selected?.workspace !== this.workspace) return curNode;
// Selection has a block in our workspace.
if (selected instanceof Blockly.BlockSvg) {
if (selected.isShadow()) {
// Although the shadow block is selected it's the parent that has the
// visual selection.
selected = selected.getParent();
}
if (selected) {
this.setCurNode(new ASTNode(ASTNode.types.BLOCK, selected), true);
}
}

return super.getCurNode();
}

/**
* Sets the object in charge of drawing the marker.
*
Expand Down Expand Up @@ -516,14 +553,13 @@ 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) {
override setCurNode(newNode: ASTNode, selectionUpToDate = false) {
if (!selectionUpToDate) {
if (
newNode?.getType() === ASTNode.types.BLOCK &&
// For shadow blocks we need to clear the selection that's drawn on their parent.
!(newNode.getLocation() as Blockly.BlockSvg).isShadow()
) {
if (Blockly.common.getSelected() !== newNode.getLocation()) {
Expand Down Expand Up @@ -673,22 +709,16 @@ export class LineCursor extends Marker {
/**
* Event listener that syncs the cursor location to the selected
* block on SELECTED events.
*
* @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);
}
}
} else {
this.setCurNode(null as never, true);
}
// This runs too late so the logic to update the selection is in
// `getCurNode`.
this.getCurNode();
}
}

Expand Down
40 changes: 37 additions & 3 deletions src/navigation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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 bugs 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).
*
Expand Down
7 changes: 7 additions & 0 deletions src/navigation_controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down