Skip to content

Commit

Permalink
refactor: Add an isFocusable() method to FlyoutItem.
Browse files Browse the repository at this point in the history
  • Loading branch information
gonfunko committed Jan 8, 2025
1 parent 5e0cafd commit 88e7245
Show file tree
Hide file tree
Showing 12 changed files with 195 additions and 86 deletions.
30 changes: 23 additions & 7 deletions core/block_flyout_inflater.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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.
*/
Expand All @@ -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);
Expand All @@ -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);
}

/**
Expand Down Expand Up @@ -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']) {
Expand All @@ -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);
Expand Down Expand Up @@ -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,
);
3 changes: 2 additions & 1 deletion core/blockly.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down
27 changes: 20 additions & 7 deletions core/button_flyout_inflater.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*/
Expand All @@ -22,15 +24,16 @@ 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!,
state as ButtonOrLabelInfo,
false,
);
button.show();
return button;

return new FlyoutItem(button, BUTTON_TYPE, true);
}

/**
Expand All @@ -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,
);
49 changes: 22 additions & 27 deletions core/flyout_base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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,
),
});
);
}
}
}
Expand All @@ -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)
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -959,11 +962,3 @@ export abstract class Flyout
return null;
}
}

/**
* A flyout content item.
*/
export interface FlyoutItem {
type: string;
element: IBoundedElement;
}
11 changes: 6 additions & 5 deletions core/flyout_horizontal.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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();
}
}

Expand Down Expand Up @@ -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;
Expand Down
42 changes: 42 additions & 0 deletions core/flyout_item.ts
Original file line number Diff line number Diff line change
@@ -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;
}
}
15 changes: 8 additions & 7 deletions core/flyout_vertical.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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();
}
}

Expand Down Expand Up @@ -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_;
Expand All @@ -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);
}
}

Expand Down
2 changes: 1 addition & 1 deletion core/interfaces/i_flyout.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down
Loading

0 comments on commit 88e7245

Please sign in to comment.