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.

Author: gonfunko

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);

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,27 +150,57 @@ 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.
      * @param e The original event that triggered the context menu to open. Not
      *     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;
+    separator?: never;
+  }
+
+  /**
+   * A representation of a menu separator item in the registry.
+   */
+  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;
-    id: string;
   }
 
   /**
-   * A menu item as presented to contextmenu.js.
+   * A representation of a normal, clickable menu item in contextmenu.ts.
    */
-  export interface ContextMenuOption {
+  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;
   }
 
   /**

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;

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 ` +

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<string> {
-  /** 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<string> {
     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<string> {
     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<string> {
     }
     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

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<MenuItem | MenuSeparator> = [];
 
   /**
    * 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);
+  }
 }