diff --git a/core/block_flyout_inflater.ts b/core/block_flyout_inflater.ts index b888e5f3a..0177ddf50 100644 --- a/core/block_flyout_inflater.ts +++ b/core/block_flyout_inflater.ts @@ -10,7 +10,7 @@ import * as common from './common.js'; import {MANUALLY_DISABLED} from './constants.js'; import type {Abstract as AbstractEvent} from './events/events_abstract.js'; import {EventType} from './events/type.js'; -import type {IBoundedElement} from './interfaces/i_bounded_element.js'; +import {FlyoutItem} from './flyout_item.js'; import type {IFlyout} from './interfaces/i_flyout.js'; import type {IFlyoutInflater} from './interfaces/i_flyout_inflater.js'; import * as registry from './registry.js'; @@ -27,6 +27,8 @@ import * as Xml from './xml.js'; const WORKSPACE_AT_BLOCK_CAPACITY_DISABLED_REASON = 'WORKSPACE_AT_BLOCK_CAPACITY'; +const BLOCK_TYPE = 'block'; + /** * Class responsible for creating blocks for flyouts. */ @@ -51,7 +53,7 @@ export class BlockFlyoutInflater implements IFlyoutInflater { * @param flyoutWorkspace The workspace to create the block on. * @returns A newly created block. */ - load(state: object, flyoutWorkspace: WorkspaceSvg): IBoundedElement { + load(state: object, flyoutWorkspace: WorkspaceSvg): FlyoutItem { this.setFlyoutWorkspace(flyoutWorkspace); this.flyout = flyoutWorkspace.targetWorkspace?.getFlyout() ?? undefined; const block = this.createBlock(state as BlockInfo, flyoutWorkspace); @@ -70,7 +72,7 @@ export class BlockFlyoutInflater implements IFlyoutInflater { block.getDescendants(false).forEach((b) => (b.isInFlyout = true)); this.addBlockListeners(block); - return block; + return new FlyoutItem(block, BLOCK_TYPE, true); } /** @@ -114,7 +116,7 @@ export class BlockFlyoutInflater implements IFlyoutInflater { * @param defaultGap The default spacing for flyout items. * @returns The amount of space that should follow this block. */ - gapForElement(state: object, defaultGap: number): number { + gapForItem(state: object, defaultGap: number): number { const blockState = state as BlockInfo; let gap; if (blockState['gap']) { @@ -134,9 +136,10 @@ export class BlockFlyoutInflater implements IFlyoutInflater { /** * Disposes of the given block. * - * @param element The flyout block to dispose of. + * @param item The flyout block to dispose of. */ - disposeElement(element: IBoundedElement): void { + disposeItem(item: FlyoutItem): void { + const element = item.getElement(); if (!(element instanceof BlockSvg)) return; this.removeListeners(element.id); element.dispose(false, false); @@ -257,6 +260,19 @@ export class BlockFlyoutInflater implements IFlyoutInflater { } }); } + + /** + * Returns the type of items this inflater is responsible for creating. + * + * @returns An identifier for the type of items this inflater creates. + */ + getType() { + return BLOCK_TYPE; + } } -registry.register(registry.Type.FLYOUT_INFLATER, 'block', BlockFlyoutInflater); +registry.register( + registry.Type.FLYOUT_INFLATER, + BLOCK_TYPE, + BlockFlyoutInflater, +); diff --git a/core/blockly.ts b/core/blockly.ts index d0543abbe..a743ca5a7 100644 --- a/core/blockly.ts +++ b/core/blockly.ts @@ -99,9 +99,10 @@ import { FieldVariableFromJsonConfig, FieldVariableValidator, } from './field_variable.js'; -import {Flyout, FlyoutItem} from './flyout_base.js'; +import {Flyout} from './flyout_base.js'; import {FlyoutButton} from './flyout_button.js'; import {HorizontalFlyout} from './flyout_horizontal.js'; +import {FlyoutItem} from './flyout_item.js'; import {FlyoutMetricsManager} from './flyout_metrics_manager.js'; import {FlyoutSeparator} from './flyout_separator.js'; import {VerticalFlyout} from './flyout_vertical.js'; diff --git a/core/button_flyout_inflater.ts b/core/button_flyout_inflater.ts index fc788ea5b..2a9c3e289 100644 --- a/core/button_flyout_inflater.ts +++ b/core/button_flyout_inflater.ts @@ -5,12 +5,14 @@ */ import {FlyoutButton} from './flyout_button.js'; -import type {IBoundedElement} from './interfaces/i_bounded_element.js'; +import {FlyoutItem} from './flyout_item.js'; import type {IFlyoutInflater} from './interfaces/i_flyout_inflater.js'; import * as registry from './registry.js'; import {ButtonOrLabelInfo} from './utils/toolbox.js'; import type {WorkspaceSvg} from './workspace_svg.js'; +const BUTTON_TYPE = 'button'; + /** * Class responsible for creating buttons for flyouts. */ @@ -22,7 +24,7 @@ export class ButtonFlyoutInflater implements IFlyoutInflater { * @param flyoutWorkspace The workspace to create the button on. * @returns A newly created FlyoutButton. */ - load(state: object, flyoutWorkspace: WorkspaceSvg): IBoundedElement { + load(state: object, flyoutWorkspace: WorkspaceSvg): FlyoutItem { const button = new FlyoutButton( flyoutWorkspace, flyoutWorkspace.targetWorkspace!, @@ -30,7 +32,8 @@ export class ButtonFlyoutInflater implements IFlyoutInflater { false, ); button.show(); - return button; + + return new FlyoutItem(button, BUTTON_TYPE, true); } /** @@ -40,24 +43,34 @@ export class ButtonFlyoutInflater implements IFlyoutInflater { * @param defaultGap The default spacing for flyout items. * @returns The amount of space that should follow this button. */ - gapForElement(state: object, defaultGap: number): number { + gapForItem(state: object, defaultGap: number): number { return defaultGap; } /** * Disposes of the given button. * - * @param element The flyout button to dispose of. + * @param item The flyout button to dispose of. */ - disposeElement(element: IBoundedElement): void { + disposeItem(item: FlyoutItem): void { + const element = item.getElement(); if (element instanceof FlyoutButton) { element.dispose(); } } + + /** + * Returns the type of items this inflater is responsible for creating. + * + * @returns An identifier for the type of items this inflater creates. + */ + getType() { + return BUTTON_TYPE; + } } registry.register( registry.Type.FLYOUT_INFLATER, - 'button', + BUTTON_TYPE, ButtonFlyoutInflater, ); diff --git a/core/flyout_base.ts b/core/flyout_base.ts index 87aba0119..817076b97 100644 --- a/core/flyout_base.ts +++ b/core/flyout_base.ts @@ -18,16 +18,17 @@ import {DeleteArea} from './delete_area.js'; import type {Abstract as AbstractEvent} from './events/events_abstract.js'; import {EventType} from './events/type.js'; import * as eventUtils from './events/utils.js'; +import {FlyoutItem} from './flyout_item.js'; import {FlyoutMetricsManager} from './flyout_metrics_manager.js'; import {FlyoutSeparator, SeparatorAxis} from './flyout_separator.js'; import {IAutoHideable} from './interfaces/i_autohideable.js'; -import type {IBoundedElement} from './interfaces/i_bounded_element.js'; import type {IFlyout} from './interfaces/i_flyout.js'; import type {IFlyoutInflater} from './interfaces/i_flyout_inflater.js'; import type {Options} from './options.js'; import * as registry from './registry.js'; import * as renderManagement from './render_management.js'; import {ScrollbarPair} from './scrollbar_pair.js'; +import {SEPARATOR_TYPE} from './separator_flyout_inflater.js'; import * as blocks from './serialization/blocks.js'; import {Coordinate} from './utils/coordinate.js'; import * as dom from './utils/dom.js'; @@ -677,20 +678,19 @@ export abstract class Flyout const type = info['kind'].toLowerCase(); const inflater = this.getInflaterForType(type); if (inflater) { - const element = inflater.load(info, this.getWorkspace()); - contents.push({ - type, - element, - }); - const gap = inflater.gapForElement(info, defaultGap); + contents.push(inflater.load(info, this.getWorkspace())); + const gap = inflater.gapForItem(info, defaultGap); if (gap) { - contents.push({ - type: 'sep', - element: new FlyoutSeparator( - gap, - this.horizontalLayout ? SeparatorAxis.X : SeparatorAxis.Y, + contents.push( + new FlyoutItem( + new FlyoutSeparator( + gap, + this.horizontalLayout ? SeparatorAxis.X : SeparatorAxis.Y, + ), + SEPARATOR_TYPE, + false, ), - }); + ); } } } @@ -711,9 +711,12 @@ export abstract class Flyout */ protected normalizeSeparators(contents: FlyoutItem[]): FlyoutItem[] { for (let i = contents.length - 1; i > 0; i--) { - const elementType = contents[i].type.toLowerCase(); - const previousElementType = contents[i - 1].type.toLowerCase(); - if (elementType === 'sep' && previousElementType === 'sep') { + const elementType = contents[i].getType().toLowerCase(); + const previousElementType = contents[i - 1].getType().toLowerCase(); + if ( + elementType === SEPARATOR_TYPE && + previousElementType === SEPARATOR_TYPE + ) { // Remove previousElement from the array, shifting the current element // forward as a result. This preserves the behavior where explicit // separator elements override the value of prior implicit (or explicit) @@ -752,9 +755,9 @@ export abstract class Flyout * Delete elements from a previous showing of the flyout. */ private clearOldBlocks() { - this.getContents().forEach((element) => { - const inflater = this.getInflaterForType(element.type); - inflater?.disposeElement(element.element); + this.getContents().forEach((item) => { + const inflater = this.getInflaterForType(item.getType()); + inflater?.disposeItem(item); }); // Clear potential variables from the previous showing. @@ -959,11 +962,3 @@ export abstract class Flyout return null; } } - -/** - * A flyout content item. - */ -export interface FlyoutItem { - type: string; - element: IBoundedElement; -} diff --git a/core/flyout_horizontal.ts b/core/flyout_horizontal.ts index d19320c82..47b7ab06a 100644 --- a/core/flyout_horizontal.ts +++ b/core/flyout_horizontal.ts @@ -13,7 +13,8 @@ import * as browserEvents from './browser_events.js'; import * as dropDownDiv from './dropdowndiv.js'; -import {Flyout, FlyoutItem} from './flyout_base.js'; +import {Flyout} from './flyout_base.js'; +import type {FlyoutItem} from './flyout_item.js'; import type {Options} from './options.js'; import * as registry from './registry.js'; import {Scrollbar} from './scrollbar.js'; @@ -263,10 +264,10 @@ export class HorizontalFlyout extends Flyout { } for (const item of contents) { - const rect = item.element.getBoundingRectangle(); + const rect = item.getElement().getBoundingRectangle(); const moveX = this.RTL ? cursorX + rect.getWidth() : cursorX; - item.element.moveBy(moveX, cursorY); - cursorX += item.element.getBoundingRectangle().getWidth(); + item.getElement().moveBy(moveX, cursorY); + cursorX += item.getElement().getBoundingRectangle().getWidth(); } } @@ -336,7 +337,7 @@ export class HorizontalFlyout extends Flyout { let flyoutHeight = this.getContents().reduce((maxHeightSoFar, item) => { return Math.max( maxHeightSoFar, - item.element.getBoundingRectangle().getHeight(), + item.getElement().getBoundingRectangle().getHeight(), ); }, 0); flyoutHeight += this.MARGIN * 1.5; diff --git a/core/flyout_item.ts b/core/flyout_item.ts new file mode 100644 index 000000000..d501ceedb --- /dev/null +++ b/core/flyout_item.ts @@ -0,0 +1,42 @@ +import type {IBoundedElement} from './interfaces/i_bounded_element.js'; + +/** + * Representation of an item displayed in a flyout. + */ +export class FlyoutItem { + /** + * Creates a new FlyoutItem. + * + * @param element The element that will be displayed in the flyout. + * @param type The type of element. Should correspond to the type of the + * flyout inflater that created this object. + * @param focusable True if the element should be allowed to be focused by + * e.g. keyboard navigation in the flyout. + */ + constructor( + private element: IBoundedElement, + private type: string, + private focusable: boolean, + ) {} + + /** + * Returns the element displayed in the flyout. + */ + getElement() { + return this.element; + } + + /** + * Returns the type of flyout element this item represents. + */ + getType() { + return this.type; + } + + /** + * Returns whether or not the flyout element can receive focus. + */ + isFocusable() { + return this.focusable; + } +} diff --git a/core/flyout_vertical.ts b/core/flyout_vertical.ts index 8e7c1691c..968b7c024 100644 --- a/core/flyout_vertical.ts +++ b/core/flyout_vertical.ts @@ -13,7 +13,8 @@ import * as browserEvents from './browser_events.js'; import * as dropDownDiv from './dropdowndiv.js'; -import {Flyout, FlyoutItem} from './flyout_base.js'; +import {Flyout} from './flyout_base.js'; +import type {FlyoutItem} from './flyout_item.js'; import type {Options} from './options.js'; import * as registry from './registry.js'; import {Scrollbar} from './scrollbar.js'; @@ -229,8 +230,8 @@ export class VerticalFlyout extends Flyout { let cursorY = margin; for (const item of contents) { - item.element.moveBy(cursorX, cursorY); - cursorY += item.element.getBoundingRectangle().getHeight(); + item.getElement().moveBy(cursorX, cursorY); + cursorY += item.getElement().getBoundingRectangle().getHeight(); } } @@ -301,7 +302,7 @@ export class VerticalFlyout extends Flyout { let flyoutWidth = this.getContents().reduce((maxWidthSoFar, item) => { return Math.max( maxWidthSoFar, - item.element.getBoundingRectangle().getWidth(), + item.getElement().getBoundingRectangle().getWidth(), ); }, 0); flyoutWidth += this.MARGIN * 1.5 + this.tabWidth_; @@ -312,13 +313,13 @@ export class VerticalFlyout extends Flyout { if (this.RTL) { // With the flyoutWidth known, right-align the flyout contents. for (const item of this.getContents()) { - const oldX = item.element.getBoundingRectangle().left; + const oldX = item.getElement().getBoundingRectangle().left; const newX = flyoutWidth / this.workspace_.scale - - item.element.getBoundingRectangle().getWidth() - + item.getElement().getBoundingRectangle().getWidth() - this.MARGIN - this.tabWidth_; - item.element.moveBy(newX - oldX, 0); + item.getElement().moveBy(newX - oldX, 0); } } diff --git a/core/interfaces/i_flyout.ts b/core/interfaces/i_flyout.ts index c79be344c..42204775e 100644 --- a/core/interfaces/i_flyout.ts +++ b/core/interfaces/i_flyout.ts @@ -7,7 +7,7 @@ // Former goog.module ID: Blockly.IFlyout import type {BlockSvg} from '../block_svg.js'; -import {FlyoutItem} from '../flyout_base.js'; +import type {FlyoutItem} from '../flyout_item.js'; import type {Coordinate} from '../utils/coordinate.js'; import type {Svg} from '../utils/svg.js'; import type {FlyoutDefinition} from '../utils/toolbox.js'; diff --git a/core/interfaces/i_flyout_inflater.ts b/core/interfaces/i_flyout_inflater.ts index 31f4c23fc..2deab770d 100644 --- a/core/interfaces/i_flyout_inflater.ts +++ b/core/interfaces/i_flyout_inflater.ts @@ -1,5 +1,5 @@ +import type {FlyoutItem} from '../flyout_item.js'; import type {WorkspaceSvg} from '../workspace_svg.js'; -import type {IBoundedElement} from './i_bounded_element.js'; export interface IFlyoutInflater { /** @@ -16,7 +16,7 @@ export interface IFlyoutInflater { * element, however. * @returns The newly inflated flyout element. */ - load(state: object, flyoutWorkspace: WorkspaceSvg): IBoundedElement; + load(state: object, flyoutWorkspace: WorkspaceSvg): FlyoutItem; /** * Returns the amount of spacing that should follow the element corresponding @@ -26,7 +26,7 @@ export interface IFlyoutInflater { * @param defaultGap The default gap for elements in this flyout. * @returns The gap that should follow the given element. */ - gapForElement(state: object, defaultGap: number): number; + gapForItem(state: object, defaultGap: number): number; /** * Disposes of the given element. @@ -37,5 +37,15 @@ export interface IFlyoutInflater { * * @param element The flyout element to dispose of. */ - disposeElement(element: IBoundedElement): void; + disposeItem(item: FlyoutItem): void; + + /** + * Returns the type of items that this inflater is responsible for inflating. + * This should be the same as the name under which this inflater registers + * itself, as well as the value returned by `getType()` on the `FlyoutItem` + * objects returned by `load()`. + * + * @returns The type of items this inflater creates. + */ + getType(): string; } diff --git a/core/keyboard_nav/ast_node.ts b/core/keyboard_nav/ast_node.ts index 1b3d72a01..ec46a4d3f 100644 --- a/core/keyboard_nav/ast_node.ts +++ b/core/keyboard_nav/ast_node.ts @@ -17,8 +17,8 @@ import {BlockSvg} from '../block_svg.js'; import type {Connection} from '../connection.js'; import {ConnectionType} from '../connection_type.js'; import type {Field} from '../field.js'; -import {FlyoutItem} from '../flyout_base.js'; import {FlyoutButton} from '../flyout_button.js'; +import type {FlyoutItem} from '../flyout_item.js'; import type {Input} from '../inputs/input.js'; import type {IASTNodeLocation} from '../interfaces/i_ast_node_location.js'; import type {IASTNodeLocationWithBlock} from '../interfaces/i_ast_node_location_with_block.js'; @@ -348,10 +348,11 @@ export class ASTNode { ); if (!nextItem) return null; - if (nextItem.element instanceof FlyoutButton) { - return ASTNode.createButtonNode(nextItem.element); - } else if (nextItem.element instanceof BlockSvg) { - return ASTNode.createStackNode(nextItem.element); + const element = nextItem.getElement(); + if (element instanceof FlyoutButton) { + return ASTNode.createButtonNode(element); + } else if (element instanceof BlockSvg) { + return ASTNode.createStackNode(element); } return null; @@ -373,13 +374,13 @@ export class ASTNode { const currentIndex = flyoutContents.findIndex((item: FlyoutItem) => { if ( currentLocation instanceof BlockSvg && - item.element === currentLocation + item.getElement() === currentLocation ) { return true; } if ( currentLocation instanceof FlyoutButton && - item.element === currentLocation + item.getElement() === currentLocation ) { return true; } @@ -388,7 +389,17 @@ export class ASTNode { if (currentIndex < 0) return null; - const resultIndex = forward ? currentIndex + 1 : currentIndex - 1; + let resultIndex = forward ? currentIndex + 1 : currentIndex - 1; + // The flyout may contain non-focusable elements like spacers or custom + // items. If the next/previous element is one of those, keep looking until a + // focusable element is encountered. + while ( + resultIndex >= 0 && + resultIndex < flyoutContents.length && + !flyoutContents[resultIndex].isFocusable() + ) { + resultIndex += forward ? 1 : -1; + } if (resultIndex === -1 || resultIndex === flyoutContents.length) { return null; } diff --git a/core/label_flyout_inflater.ts b/core/label_flyout_inflater.ts index ad304a9a6..51899ef23 100644 --- a/core/label_flyout_inflater.ts +++ b/core/label_flyout_inflater.ts @@ -5,12 +5,14 @@ */ import {FlyoutButton} from './flyout_button.js'; -import type {IBoundedElement} from './interfaces/i_bounded_element.js'; +import {FlyoutItem} from './flyout_item.js'; import type {IFlyoutInflater} from './interfaces/i_flyout_inflater.js'; import * as registry from './registry.js'; import {ButtonOrLabelInfo} from './utils/toolbox.js'; import type {WorkspaceSvg} from './workspace_svg.js'; +const LABEL_TYPE = 'label'; + /** * Class responsible for creating labels for flyouts. */ @@ -22,7 +24,7 @@ export class LabelFlyoutInflater implements IFlyoutInflater { * @param flyoutWorkspace The workspace to create the label on. * @returns A FlyoutButton configured as a label. */ - load(state: object, flyoutWorkspace: WorkspaceSvg): IBoundedElement { + load(state: object, flyoutWorkspace: WorkspaceSvg): FlyoutItem { const label = new FlyoutButton( flyoutWorkspace, flyoutWorkspace.targetWorkspace!, @@ -30,7 +32,8 @@ export class LabelFlyoutInflater implements IFlyoutInflater { true, ); label.show(); - return label; + + return new FlyoutItem(label, LABEL_TYPE, true); } /** @@ -40,20 +43,34 @@ export class LabelFlyoutInflater implements IFlyoutInflater { * @param defaultGap The default spacing for flyout items. * @returns The amount of space that should follow this label. */ - gapForElement(state: object, defaultGap: number): number { + gapForItem(state: object, defaultGap: number): number { return defaultGap; } /** * Disposes of the given label. * - * @param element The flyout label to dispose of. + * @param item The flyout label to dispose of. */ - disposeElement(element: IBoundedElement): void { + disposeItem(item: FlyoutItem): void { + const element = item.getElement(); if (element instanceof FlyoutButton) { element.dispose(); } } + + /** + * Returns the type of items this inflater is responsible for creating. + * + * @returns An identifier for the type of items this inflater creates. + */ + getType() { + return LABEL_TYPE; + } } -registry.register(registry.Type.FLYOUT_INFLATER, 'label', LabelFlyoutInflater); +registry.register( + registry.Type.FLYOUT_INFLATER, + LABEL_TYPE, + LabelFlyoutInflater, +); diff --git a/core/separator_flyout_inflater.ts b/core/separator_flyout_inflater.ts index 8c0acf2f5..0b9aa6026 100644 --- a/core/separator_flyout_inflater.ts +++ b/core/separator_flyout_inflater.ts @@ -4,13 +4,18 @@ * SPDX-License-Identifier: Apache-2.0 */ +import {FlyoutItem} from './flyout_item.js'; import {FlyoutSeparator, SeparatorAxis} from './flyout_separator.js'; -import type {IBoundedElement} from './interfaces/i_bounded_element.js'; import type {IFlyoutInflater} from './interfaces/i_flyout_inflater.js'; import * as registry from './registry.js'; import type {SeparatorInfo} from './utils/toolbox.js'; import type {WorkspaceSvg} from './workspace_svg.js'; +/** + * @internal + */ +export const SEPARATOR_TYPE = 'sep'; + /** * Class responsible for creating separators for flyouts. */ @@ -33,12 +38,13 @@ export class SeparatorFlyoutInflater implements IFlyoutInflater { * @param flyoutWorkspace The workspace the separator belongs to. * @returns A newly created FlyoutSeparator. */ - load(_state: object, flyoutWorkspace: WorkspaceSvg): IBoundedElement { + load(_state: object, flyoutWorkspace: WorkspaceSvg): FlyoutItem { const flyoutAxis = flyoutWorkspace.targetWorkspace?.getFlyout() ?.horizontalLayout ? SeparatorAxis.X : SeparatorAxis.Y; - return new FlyoutSeparator(0, flyoutAxis); + const separator = new FlyoutSeparator(0, flyoutAxis); + return new FlyoutItem(separator, SEPARATOR_TYPE, false); } /** @@ -48,7 +54,7 @@ export class SeparatorFlyoutInflater implements IFlyoutInflater { * @param defaultGap The default spacing for flyout items. * @returns The desired size of the separator. */ - gapForElement(state: object, defaultGap: number): number { + gapForItem(state: object, defaultGap: number): number { const separatorState = state as SeparatorInfo; const newGap = parseInt(String(separatorState['gap'])); return newGap ?? defaultGap; @@ -57,13 +63,22 @@ export class SeparatorFlyoutInflater implements IFlyoutInflater { /** * Disposes of the given separator. Intentional no-op. * - * @param _element The flyout separator to dispose of. + * @param _item The flyout separator to dispose of. */ - disposeElement(_element: IBoundedElement): void {} + disposeItem(_item: FlyoutItem): void {} + + /** + * Returns the type of items this inflater is responsible for creating. + * + * @returns An identifier for the type of items this inflater creates. + */ + getType() { + return SEPARATOR_TYPE; + } } registry.register( registry.Type.FLYOUT_INFLATER, - 'sep', + SEPARATOR_TYPE, SeparatorFlyoutInflater, );