From fa4fce5c12f8197b6459537cbf0e20249e545e36 Mon Sep 17 00:00:00 2001 From: Aaron Dodson Date: Thu, 27 Feb 2025 14:00:40 -0800 Subject: [PATCH] feat!: Added support for separators in menus. (#8767) * feat!: Added support for separators in menus. * chore: Do English gooder. * fix: Remove menu separators from the DOM during dispose. --- core/contextmenu.ts | 6 +++ core/contextmenu_registry.ts | 95 ++++++++++++++++++++++++++++++------ core/css.ts | 8 +++ core/extensions.ts | 5 +- core/field_dropdown.ts | 51 ++++++++++--------- core/menu.ts | 28 +++++++---- core/menu_separator.ts | 38 +++++++++++++++ core/utils/aria.ts | 3 ++ 8 files changed, 186 insertions(+), 48 deletions(-) create mode 100644 core/menu_separator.ts diff --git a/core/contextmenu.ts b/core/contextmenu.ts index b49dcba51..7123198c2 100644 --- a/core/contextmenu.ts +++ b/core/contextmenu.ts @@ -18,6 +18,7 @@ import type { import {EventType} from './events/type.js'; import * as eventUtils from './events/utils.js'; import {Menu} from './menu.js'; +import {MenuSeparator} from './menu_separator.js'; import {MenuItem} from './menuitem.js'; import * as serializationBlocks from './serialization/blocks.js'; import * as aria from './utils/aria.js'; @@ -111,6 +112,11 @@ function populate_( menu.setRole(aria.Role.MENU); for (let i = 0; i < options.length; i++) { const option = options[i]; + if (option.separator) { + menu.addChild(new MenuSeparator()); + continue; + } + const menuItem = new MenuItem(option.text); menuItem.setRightToLeft(rtl); menuItem.setRole(aria.Role.MENUITEM); diff --git a/core/contextmenu_registry.ts b/core/contextmenu_registry.ts index fb0d899d1..5bfb1eb63 100644 --- a/core/contextmenu_registry.ts +++ b/core/contextmenu_registry.ts @@ -87,21 +87,37 @@ export class ContextMenuRegistry { const menuOptions: ContextMenuOption[] = []; for (const item of this.registeredItems.values()) { if (scopeType === item.scopeType) { - const precondition = item.preconditionFn(scope); - if (precondition !== 'hidden') { + let menuOption: + | ContextMenuRegistry.CoreContextMenuOption + | ContextMenuRegistry.SeparatorContextMenuOption + | ContextMenuRegistry.ActionContextMenuOption; + menuOption = { + scope, + weight: item.weight, + }; + + if (item.separator) { + menuOption = { + ...menuOption, + separator: true, + }; + } else { + const precondition = item.preconditionFn(scope); + if (precondition === 'hidden') continue; + const displayText = typeof item.displayText === 'function' ? item.displayText(scope) : item.displayText; - const menuOption: ContextMenuOption = { + menuOption = { + ...menuOption, text: displayText, - enabled: precondition === 'enabled', callback: item.callback, - scope, - weight: item.weight, + enabled: precondition === 'enabled', }; - menuOptions.push(menuOption); } + + menuOptions.push(menuOption); } } menuOptions.sort(function (a, b) { @@ -134,9 +150,18 @@ export namespace ContextMenuRegistry { } /** - * A menu item as entered in the registry. + * Fields common to all context menu registry items. */ - export interface RegistryItem { + interface CoreRegistryItem { + scopeType: ScopeType; + weight: number; + id: string; + } + + /** + * A representation of a normal, clickable menu item in the registry. + */ + interface ActionRegistryItem extends CoreRegistryItem { /** * @param scope Object that provides a reference to the thing that had its * context menu opened. @@ -144,17 +169,38 @@ export namespace ContextMenuRegistry { * the event that triggered the click on the option. */ callback: (scope: Scope, e: PointerEvent) => void; - scopeType: ScopeType; displayText: ((p1: Scope) => string | HTMLElement) | string | HTMLElement; preconditionFn: (p1: Scope) => string; - weight: number; - id: string; + separator?: never; } /** - * A menu item as presented to contextmenu.js. + * A representation of a menu separator item in the registry. */ - export interface ContextMenuOption { + interface SeparatorRegistryItem extends CoreRegistryItem { + separator: true; + callback?: never; + displayText?: never; + preconditionFn?: never; + } + + /** + * A menu item as entered in the registry. + */ + export type RegistryItem = ActionRegistryItem | SeparatorRegistryItem; + + /** + * Fields common to all context menu items as used by contextmenu.ts. + */ + export interface CoreContextMenuOption { + scope: Scope; + weight: number; + } + + /** + * A representation of a normal, clickable menu item in contextmenu.ts. + */ + export interface ActionContextMenuOption extends CoreContextMenuOption { text: string | HTMLElement; enabled: boolean; /** @@ -164,10 +210,26 @@ export namespace ContextMenuRegistry { * the event that triggered the click on the option. */ callback: (scope: Scope, e: PointerEvent) => void; - scope: Scope; - weight: number; + separator?: never; } + /** + * A representation of a menu separator item in contextmenu.ts. + */ + export interface SeparatorContextMenuOption extends CoreContextMenuOption { + separator: true; + text?: never; + enabled?: never; + callback?: never; + } + + /** + * A menu item as presented to contextmenu.ts. + */ + export type ContextMenuOption = + | ActionContextMenuOption + | SeparatorContextMenuOption; + /** * A subset of ContextMenuOption corresponding to what was publicly * documented. ContextMenuOption should be preferred for new code. @@ -176,6 +238,7 @@ export namespace ContextMenuRegistry { text: string; enabled: boolean; callback: (p1: Scope) => void; + separator?: never; } /** diff --git a/core/css.ts b/core/css.ts index 57217f854..00bbc0e02 100644 --- a/core/css.ts +++ b/core/css.ts @@ -461,6 +461,14 @@ input[type=number] { margin-right: -24px; } +.blocklyMenuSeparator { + background-color: #ccc; + height: 1px; + border: 0; + margin-left: 4px; + margin-right: 4px; +} + .blocklyBlockDragSurface, .blocklyAnimationLayer { position: absolute; top: 0; diff --git a/core/extensions.ts b/core/extensions.ts index 0957b7f86..59d218d17 100644 --- a/core/extensions.ts +++ b/core/extensions.ts @@ -437,7 +437,10 @@ function checkDropdownOptionsInTable( } const options = dropdown.getOptions(); - for (const [, key] of options) { + for (const option of options) { + if (option === FieldDropdown.SEPARATOR) continue; + + const [, key] = option; if (lookupTable[key] === undefined) { console.warn( `No tooltip mapping for value ${key} of field ` + diff --git a/core/field_dropdown.ts b/core/field_dropdown.ts index 0be621d38..60977524a 100644 --- a/core/field_dropdown.ts +++ b/core/field_dropdown.ts @@ -23,6 +23,7 @@ import { } from './field.js'; import * as fieldRegistry from './field_registry.js'; import {Menu} from './menu.js'; +import {MenuSeparator} from './menu_separator.js'; import {MenuItem} from './menuitem.js'; import * as aria from './utils/aria.js'; import {Coordinate} from './utils/coordinate.js'; @@ -35,14 +36,10 @@ import {Svg} from './utils/svg.js'; * Class for an editable dropdown field. */ export class FieldDropdown extends Field { - /** Horizontal distance that a checkmark overhangs the dropdown. */ - static CHECKMARK_OVERHANG = 25; - /** - * Maximum height of the dropdown menu, as a percentage of the viewport - * height. + * Magic constant used to represent a separator in a list of dropdown items. */ - static MAX_MENU_HEIGHT_VH = 0.45; + static readonly SEPARATOR = 'separator'; static ARROW_CHAR = '▾'; @@ -323,7 +320,13 @@ export class FieldDropdown extends Field { const options = this.getOptions(false); this.selectedMenuItem = null; for (let i = 0; i < options.length; i++) { - const [label, value] = options[i]; + const option = options[i]; + if (option === FieldDropdown.SEPARATOR) { + menu.addChild(new MenuSeparator()); + continue; + } + + const [label, value] = option; const content = (() => { if (typeof label === 'object') { // Convert ImageProperties to an HTMLImageElement. @@ -667,7 +670,10 @@ export class FieldDropdown extends Field { suffix?: string; } { let hasImages = false; - const trimmedOptions = options.map(([label, value]): MenuOption => { + const trimmedOptions = options.map((option): MenuOption => { + if (option === FieldDropdown.SEPARATOR) return option; + + const [label, value] = option; if (typeof label === 'string') { return [parsing.replaceMessageReferences(label), value]; } @@ -748,28 +754,28 @@ export class FieldDropdown extends Field { } let foundError = false; for (let i = 0; i < options.length; i++) { - const tuple = options[i]; - if (!Array.isArray(tuple)) { + const option = options[i]; + if (!Array.isArray(option) && option !== FieldDropdown.SEPARATOR) { foundError = true; console.error( - `Invalid option[${i}]: Each FieldDropdown option must be an array. - Found: ${tuple}`, + `Invalid option[${i}]: Each FieldDropdown option must be an array or + the string literal 'separator'. Found: ${option}`, ); - } else if (typeof tuple[1] !== 'string') { + } else if (typeof option[1] !== 'string') { foundError = true; console.error( `Invalid option[${i}]: Each FieldDropdown option id must be a string. - Found ${tuple[1]} in: ${tuple}`, + Found ${option[1]} in: ${option}`, ); } else if ( - tuple[0] && - typeof tuple[0] !== 'string' && - typeof tuple[0].src !== 'string' + option[0] && + typeof option[0] !== 'string' && + typeof option[0].src !== 'string' ) { foundError = true; console.error( `Invalid option[${i}]: Each FieldDropdown option must have a string - label or image description. Found ${tuple[0]} in: ${tuple}`, + label or image description. Found ${option[0]} in: ${option}`, ); } } @@ -790,11 +796,12 @@ export interface ImageProperties { } /** - * An individual option in the dropdown menu. The first element is the human- - * readable value (text or image), and the second element is the language- - * neutral value. + * An individual option in the dropdown menu. Can be either the string literal + * `separator` for a menu separator item, or an array for normal action menu + * items. In the latter case, the first element is the human-readable value + * (text or image), and the second element is the language-neutral value. */ -export type MenuOption = [string | ImageProperties, string]; +export type MenuOption = [string | ImageProperties, string] | 'separator'; /** * A function that generates an array of menu options for FieldDropdown diff --git a/core/menu.ts b/core/menu.ts index 7746bb212..acb5a3780 100644 --- a/core/menu.ts +++ b/core/menu.ts @@ -12,7 +12,8 @@ // Former goog.module ID: Blockly.Menu import * as browserEvents from './browser_events.js'; -import type {MenuItem} from './menuitem.js'; +import type {MenuSeparator} from './menu_separator.js'; +import {MenuItem} from './menuitem.js'; import * as aria from './utils/aria.js'; import {Coordinate} from './utils/coordinate.js'; import type {Size} from './utils/size.js'; @@ -23,11 +24,9 @@ import * as style from './utils/style.js'; */ export class Menu { /** - * Array of menu items. - * (Nulls are never in the array, but typing the array as nullable prevents - * the compiler from objecting to .indexOf(null)) + * Array of menu items and separators. */ - private readonly menuItems: MenuItem[] = []; + private readonly menuItems: Array = []; /** * Coordinates of the mousedown event that caused this menu to open. Used to @@ -69,10 +68,10 @@ export class Menu { /** * Add a new menu item to the bottom of this menu. * - * @param menuItem Menu item to append. + * @param menuItem Menu item or separator to append. * @internal */ - addChild(menuItem: MenuItem) { + addChild(menuItem: MenuItem | MenuSeparator) { this.menuItems.push(menuItem); } @@ -227,7 +226,8 @@ export class Menu { while (currentElement && currentElement !== menuElem) { if (currentElement.classList.contains('blocklyMenuItem')) { // Having found a menu item's div, locate that menu item in this menu. - for (let i = 0, menuItem; (menuItem = this.menuItems[i]); i++) { + const items = this.getMenuItems(); + for (let i = 0, menuItem; (menuItem = items[i]); i++) { if (menuItem.getElement() === currentElement) { return menuItem; } @@ -309,7 +309,8 @@ export class Menu { private highlightHelper(startIndex: number, delta: number) { let index = startIndex + delta; let menuItem; - while ((menuItem = this.menuItems[index])) { + const items = this.getMenuItems(); + while ((menuItem = items[index])) { if (menuItem.isEnabled()) { this.setHighlighted(menuItem); break; @@ -459,4 +460,13 @@ export class Menu { menuSize.height = menuDom.scrollHeight; return menuSize; } + + /** + * Returns the action menu items (omitting separators) in this menu. + * + * @returns The MenuItem objects displayed in this menu. + */ + private getMenuItems(): MenuItem[] { + return this.menuItems.filter((item) => item instanceof MenuItem); + } } diff --git a/core/menu_separator.ts b/core/menu_separator.ts new file mode 100644 index 000000000..6f7f468ad --- /dev/null +++ b/core/menu_separator.ts @@ -0,0 +1,38 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import * as aria from './utils/aria.js'; + +/** + * Representation of a section separator in a menu. + */ +export class MenuSeparator { + /** + * DOM element representing this separator in a menu. + */ + private element: HTMLHRElement | null = null; + + /** + * Creates the DOM representation of this separator. + * + * @returns An
element. + */ + createDom(): HTMLHRElement { + this.element = document.createElement('hr'); + this.element.className = 'blocklyMenuSeparator'; + aria.setRole(this.element, aria.Role.SEPARATOR); + + return this.element; + } + + /** + * Disposes of this separator. + */ + dispose() { + this.element?.remove(); + this.element = null; + } +} diff --git a/core/utils/aria.ts b/core/utils/aria.ts index 567ea95ef..8089298e4 100644 --- a/core/utils/aria.ts +++ b/core/utils/aria.ts @@ -48,6 +48,9 @@ export enum Role { // ARIA role for a tree item that sometimes may be expanded or collapsed. TREEITEM = 'treeitem', + + // ARIA role for a visual separator in e.g. a menu. + SEPARATOR = 'separator', } /**