From d1dc38f582cdd6235f4983055a6525aaeb4acf1d Mon Sep 17 00:00:00 2001 From: Maribeth Moffatt Date: Fri, 11 Apr 2025 15:10:05 -0700 Subject: [PATCH] feat: support menuOpenEvent, menuSelectEvent, location for context menu items (#8877) * feat: support menuOpenEvent, menuSelectEvent, location for context menu items * feat: show context menu based on location * fix: rtl --- core/block_svg.ts | 53 ++++++++++++++++++--- core/comments/rendered_workspace_comment.ts | 25 +++++++++- core/contextmenu.ts | 52 +++++++++++++------- core/contextmenu_items.ts | 9 +++- core/contextmenu_registry.ts | 30 ++++++++---- core/menu.ts | 4 +- core/menuitem.ts | 12 +++-- core/workspace_svg.ts | 13 ++++- 8 files changed, 156 insertions(+), 42 deletions(-) diff --git a/core/block_svg.ts b/core/block_svg.ts index ca753fc0a..18751eb13 100644 --- a/core/block_svg.ts +++ b/core/block_svg.ts @@ -581,15 +581,16 @@ export class BlockSvg * * @returns Context menu options or null if no menu. */ - protected generateContextMenu(): Array< - ContextMenuOption | LegacyContextMenuOption - > | null { + protected generateContextMenu( + e: Event, + ): Array | null { if (this.workspace.isReadOnly() || !this.contextMenu) { return null; } const menuOptions = ContextMenuRegistry.registry.getContextMenuOptions( ContextMenuRegistry.ScopeType.BLOCK, {block: this}, + e, ); // Allow the block to add or modify menuOptions. @@ -600,17 +601,57 @@ export class BlockSvg return menuOptions; } + /** + * Gets the location in which to show the context menu for this block. + * Use the location of a click if the block was clicked, or a location + * based on the block's fields otherwise. + */ + protected calculateContextMenuLocation(e: Event): Coordinate { + // Open the menu where the user clicked, if they clicked + if (e instanceof PointerEvent) { + return new Coordinate(e.clientX, e.clientY); + } + + // Otherwise, calculate a location. + // Get the location of the top-left corner of the block in + // screen coordinates. + const blockCoords = svgMath.wsToScreenCoordinates( + this.workspace, + this.getRelativeToSurfaceXY(), + ); + + // Prefer a y position below the first field in the block. + const fieldBoundingClientRect = this.inputList + .filter((input) => input.isVisible()) + .flatMap((input) => input.fieldRow) + .find((f) => f.isVisible()) + ?.getSvgRoot() + ?.getBoundingClientRect(); + + const y = + fieldBoundingClientRect && fieldBoundingClientRect.height + ? fieldBoundingClientRect.y + fieldBoundingClientRect.height + : blockCoords.y + this.height; + + return new Coordinate( + this.RTL ? blockCoords.x - 5 : blockCoords.x + 5, + y + 5, + ); + } + /** * Show the context menu for this block. * * @param e Mouse event. * @internal */ - showContextMenu(e: PointerEvent) { - const menuOptions = this.generateContextMenu(); + showContextMenu(e: Event) { + const menuOptions = this.generateContextMenu(e); + + const location = this.calculateContextMenuLocation(e); if (menuOptions && menuOptions.length) { - ContextMenu.show(e, menuOptions, this.RTL, this.workspace); + ContextMenu.show(e, menuOptions, this.RTL, this.workspace, location); ContextMenu.setCurrentBlock(this); } } diff --git a/core/comments/rendered_workspace_comment.ts b/core/comments/rendered_workspace_comment.ts index 22525eb6a..9e48db0e4 100644 --- a/core/comments/rendered_workspace_comment.ts +++ b/core/comments/rendered_workspace_comment.ts @@ -22,6 +22,7 @@ import {IRenderedElement} from '../interfaces/i_rendered_element.js'; import {ISelectable} from '../interfaces/i_selectable.js'; import * as layers from '../layers.js'; import * as commentSerialization from '../serialization/workspace_comments.js'; +import {svgMath} from '../utils.js'; import {Coordinate} from '../utils/coordinate.js'; import * as dom from '../utils/dom.js'; import {Rect} from '../utils/rect.js'; @@ -283,12 +284,32 @@ export class RenderedWorkspaceComment } /** Show a context menu for this comment. */ - showContextMenu(e: PointerEvent): void { + showContextMenu(e: Event): void { const menuOptions = ContextMenuRegistry.registry.getContextMenuOptions( ContextMenuRegistry.ScopeType.COMMENT, {comment: this}, + e, + ); + + let location: Coordinate; + if (e instanceof PointerEvent) { + location = new Coordinate(e.clientX, e.clientY); + } else { + // Show the menu based on the location of the comment + const xy = svgMath.wsToScreenCoordinates( + this.workspace, + this.getRelativeToSurfaceXY(), + ); + location = xy.translate(10, 10); + } + + contextMenu.show( + e, + menuOptions, + this.workspace.RTL, + this.workspace, + location, ); - contextMenu.show(e, menuOptions, this.workspace.RTL, this.workspace); } /** Snap this comment to the nearest grid point. */ diff --git a/core/contextmenu.ts b/core/contextmenu.ts index 7123198c2..4a83b9dcc 100644 --- a/core/contextmenu.ts +++ b/core/contextmenu.ts @@ -21,6 +21,7 @@ import {Menu} from './menu.js'; import {MenuSeparator} from './menu_separator.js'; import {MenuItem} from './menuitem.js'; import * as serializationBlocks from './serialization/blocks.js'; +import {Coordinate} from './utils.js'; import * as aria from './utils/aria.js'; import * as dom from './utils/dom.js'; import {Rect} from './utils/rect.js'; @@ -38,6 +39,8 @@ const dummyOwner = {}; /** * Gets the block the context menu is currently attached to. + * It is not recommended that you use this function; instead, + * use the scope object passed to the context menu callback. * * @returns The block the context menu is attached to. */ @@ -62,26 +65,38 @@ let menu_: Menu | null = null; /** * Construct the menu based on the list of options and show the menu. * - * @param e Mouse event. + * @param menuOpenEvent Event that caused the menu to open. * @param options Array of menu options. * @param rtl True if RTL, false if LTR. * @param workspace The workspace associated with the context menu, if any. + * @param location The screen coordinates at which to show the menu. */ export function show( - e: PointerEvent, + menuOpenEvent: Event, options: (ContextMenuOption | LegacyContextMenuOption)[], rtl: boolean, workspace?: WorkspaceSvg, + location?: Coordinate, ) { WidgetDiv.show(dummyOwner, rtl, dispose, workspace); if (!options.length) { hide(); return; } - const menu = populate_(options, rtl, e); + + if (!location) { + if (menuOpenEvent instanceof PointerEvent) { + location = new Coordinate(menuOpenEvent.clientX, menuOpenEvent.clientY); + } else { + // We got a keyboard event that didn't tell us where to open the menu, so just guess + console.warn('Context menu opened with keyboard but no location given'); + location = new Coordinate(0, 0); + } + } + const menu = populate_(options, rtl, menuOpenEvent, location); menu_ = menu; - position_(menu, e, rtl); + position_(menu, rtl, location); // 1ms delay is required for focusing on context menus because some other // mouse event is still waiting in the queue and clears focus. setTimeout(function () { @@ -95,13 +110,15 @@ export function show( * * @param options Array of menu options. * @param rtl True if RTL, false if LTR. - * @param e The event that triggered the context menu to open. + * @param menuOpenEvent The event that triggered the context menu to open. + * @param location The screen coordinates at which to show the menu. * @returns The menu that will be shown on right click. */ function populate_( options: (ContextMenuOption | LegacyContextMenuOption)[], rtl: boolean, - e: PointerEvent, + menuOpenEvent: Event, + location: Coordinate, ): Menu { /* Here's what one option object looks like: {text: 'Make It So', @@ -123,7 +140,7 @@ function populate_( menu.addChild(menuItem); menuItem.setEnabled(option.enabled); if (option.enabled) { - const actionHandler = function () { + const actionHandler = function (p1: MenuItem, menuSelectEvent: Event) { hide(); requestAnimationFrame(() => { setTimeout(() => { @@ -131,7 +148,12 @@ function populate_( // will not be expecting a scope parameter, so there should be // no problems. Just assume it is a ContextMenuOption and we'll // pass undefined if it's not. - option.callback((option as ContextMenuOption).scope, e); + option.callback( + (option as ContextMenuOption).scope, + menuOpenEvent, + menuSelectEvent, + location, + ); }, 0); }); }; @@ -145,21 +167,19 @@ function populate_( * Add the menu to the page and position it correctly. * * @param menu The menu to add and position. - * @param e Mouse event for the right click that is making the context - * menu appear. * @param rtl True if RTL, false if LTR. + * @param location The location at which to anchor the menu. */ -function position_(menu: Menu, e: Event, rtl: boolean) { +function position_(menu: Menu, rtl: boolean, location: Coordinate) { // Record windowSize and scrollOffset before adding menu. const viewportBBox = svgMath.getViewportBBox(); - const mouseEvent = e as MouseEvent; // This one is just a point, but we'll pretend that it's a rect so we can use // some helper functions. const anchorBBox = new Rect( - mouseEvent.clientY + viewportBBox.top, - mouseEvent.clientY + viewportBBox.top, - mouseEvent.clientX + viewportBBox.left, - mouseEvent.clientX + viewportBBox.left, + location.y + viewportBBox.top, + location.y + viewportBBox.top, + location.x + viewportBBox.left, + location.x + viewportBBox.left, ); createWidget_(menu); diff --git a/core/contextmenu_items.ts b/core/contextmenu_items.ts index ebee8e3af..267305e21 100644 --- a/core/contextmenu_items.ts +++ b/core/contextmenu_items.ts @@ -614,7 +614,12 @@ export function registerCommentCreate() { preconditionFn: (scope: Scope) => { return scope.workspace?.isMutator ? 'hidden' : 'enabled'; }, - callback: (scope: Scope, e: PointerEvent) => { + callback: ( + scope: Scope, + menuOpenEvent: Event, + menuSelectEvent: Event, + location: Coordinate, + ) => { const workspace = scope.workspace; if (!workspace) return; eventUtils.setGroup(true); @@ -622,7 +627,7 @@ export function registerCommentCreate() { comment.setPlaceholderText(Msg['WORKSPACE_COMMENT_DEFAULT_TEXT']); comment.moveTo( pixelsToWorkspaceCoords( - new Coordinate(e.clientX, e.clientY), + new Coordinate(location.x, location.y), workspace, ), ); diff --git a/core/contextmenu_registry.ts b/core/contextmenu_registry.ts index 5bfb1eb63..f48fdfc67 100644 --- a/core/contextmenu_registry.ts +++ b/core/contextmenu_registry.ts @@ -13,6 +13,7 @@ import type {BlockSvg} from './block_svg.js'; import {RenderedWorkspaceComment} from './comments/rendered_workspace_comment.js'; +import {Coordinate} from './utils.js'; import type {WorkspaceSvg} from './workspace_svg.js'; /** @@ -83,6 +84,7 @@ export class ContextMenuRegistry { getContextMenuOptions( scopeType: ScopeType, scope: Scope, + menuOpenEvent: Event, ): ContextMenuOption[] { const menuOptions: ContextMenuOption[] = []; for (const item of this.registeredItems.values()) { @@ -102,7 +104,7 @@ export class ContextMenuRegistry { separator: true, }; } else { - const precondition = item.preconditionFn(scope); + const precondition = item.preconditionFn(scope, menuOpenEvent); if (precondition === 'hidden') continue; const displayText = @@ -165,12 +167,18 @@ export namespace ContextMenuRegistry { /** * @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. + * @param menuOpenEvent The original event that triggered the context menu to open. + * @param menuSelectEvent The event that triggered the option being selected. + * @param location The location in screen coordinates where the menu was opened. */ - callback: (scope: Scope, e: PointerEvent) => void; + callback: ( + scope: Scope, + menuOpenEvent: Event, + menuSelectEvent: Event, + location: Coordinate, + ) => void; displayText: ((p1: Scope) => string | HTMLElement) | string | HTMLElement; - preconditionFn: (p1: Scope) => string; + preconditionFn: (p1: Scope, menuOpenEvent: Event) => string; separator?: never; } @@ -206,10 +214,16 @@ export namespace ContextMenuRegistry { /** * @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. + * @param menuOpenEvent The original event that triggered the context menu to open. + * @param menuSelectEvent The event that triggered the option being selected. + * @param location The location in screen coordinates where the menu was opened. */ - callback: (scope: Scope, e: PointerEvent) => void; + callback: ( + scope: Scope, + menuOpenEvent: Event, + menuSelectEvent: Event, + location: Coordinate, + ) => void; separator?: never; } diff --git a/core/menu.ts b/core/menu.ts index 3af474ee7..664d3872d 100644 --- a/core/menu.ts +++ b/core/menu.ts @@ -379,7 +379,7 @@ export class Menu { const menuItem = this.getMenuItem(e.target as Element); if (menuItem) { - menuItem.performAction(); + menuItem.performAction(e); } } @@ -431,7 +431,7 @@ export class Menu { case 'Enter': case ' ': if (highlighted) { - highlighted.performAction(); + highlighted.performAction(e); } break; diff --git a/core/menuitem.ts b/core/menuitem.ts index ebeb9404b..b3ae33c5c 100644 --- a/core/menuitem.ts +++ b/core/menuitem.ts @@ -41,7 +41,8 @@ export class MenuItem { private highlight = false; /** Bound function to call when this menu item is clicked. */ - private actionHandler: ((obj: this) => void) | null = null; + private actionHandler: ((obj: this, menuSelectEvent: Event) => void) | null = + null; /** * @param content Text caption to display as the content of the item, or a @@ -220,11 +221,14 @@ export class MenuItem { * Performs the appropriate action when the menu item is activated * by the user. * + * @param menuSelectEvent the event that triggered the selection + * of the menu item. + * * @internal */ - performAction() { + performAction(menuSelectEvent: Event) { if (this.isEnabled() && this.actionHandler) { - this.actionHandler(this); + this.actionHandler(this, menuSelectEvent); } } @@ -236,7 +240,7 @@ export class MenuItem { * @param obj Used as the 'this' object in fn when called. * @internal */ - onAction(fn: (p1: MenuItem) => void, obj: object) { + onAction(fn: (p1: MenuItem, menuSelectEvent: Event) => void, obj: object) { this.actionHandler = fn.bind(obj); } } diff --git a/core/workspace_svg.ts b/core/workspace_svg.ts index 7dcd5b570..e9414dcde 100644 --- a/core/workspace_svg.ts +++ b/core/workspace_svg.ts @@ -1706,13 +1706,14 @@ export class WorkspaceSvg * @param e Mouse event. * @internal */ - showContextMenu(e: PointerEvent) { + showContextMenu(e: Event) { if (this.isReadOnly() || this.isFlyout) { return; } const menuOptions = ContextMenuRegistry.registry.getContextMenuOptions( ContextMenuRegistry.ScopeType.WORKSPACE, {workspace: this}, + e, ); // Allow the developer to add or modify menuOptions. @@ -1720,7 +1721,15 @@ export class WorkspaceSvg this.configureContextMenu(menuOptions, e); } - ContextMenu.show(e, menuOptions, this.RTL, this); + let location; + if (e instanceof PointerEvent) { + location = new Coordinate(e.clientX, e.clientY); + } else { + // TODO: Get the location based on the workspace cursor location + location = svgMath.wsToScreenCoordinates(this, new Coordinate(5, 5)); + } + + ContextMenu.show(e, menuOptions, this.RTL, this, location); } /**