Merge pull request #1861 from TriliumNext/content-menu-target

Make it show which node triggered the event when right-clicking on tree

Author: eliandoran

apps/client/src/menus/context_menu.ts

@@ -10,6 +10,7 @@ interface ContextMenuOptions<T> {
     items: MenuItem<T>[];
     /** On mobile, if set to `true` then the context menu is shown near the element. If `false` (default), then the context menu is shown at the bottom of the screen. */
     forcePositionOnMobile?: boolean;
+    onHide?: () => void;
 }
 
 interface MenuSeparatorItem {
@@ -36,15 +37,13 @@ export type ContextMenuEvent = PointerEvent | MouseEvent | JQuery.ContextMenuEve
 class ContextMenu {
     private $widget: JQuery<HTMLElement>;
     private $cover: JQuery<HTMLElement>;
-    private dateContextMenuOpenedMs: number;
     private options?: ContextMenuOptions<any>;
     private isMobile: boolean;
 
     constructor() {
         this.$widget = $("#context-menu-container");
         this.$cover = $("#context-menu-cover");
         this.$widget.addClass("dropend");
-        this.dateContextMenuOpenedMs = 0;
         this.isMobile = utils.isMobile();
 
         if (this.isMobile) {
@@ -76,8 +75,6 @@ class ContextMenu {
         keyboardActionService.updateDisplayedShortcuts(this.$widget);
 
         this.positionMenu();
-
-        this.dateContextMenuOpenedMs = Date.now();
     }
 
     positionMenu() {
@@ -186,8 +183,6 @@ class ContextMenu {
                             return false;
                         }
 
-                        this.hide();
-
                         if ("handler" in item && item.handler) {
                             item.handler(item, e);
                         }
@@ -197,6 +192,12 @@ class ContextMenu {
                         // it's important to stop the propagation especially for sub-menus, otherwise the event
                         // might be handled again by top-level menu
                         return false;
+                    })
+                    .on("mouseup", (e) =>{
+                        e.stopPropagation();
+                        // Hide the content menu on mouse up to prevent the mouse event from propagating to the elements below.
+                        this.hide();
+                        return false;
                     });
 
                 if ("enabled" in item && item.enabled !== undefined && !item.enabled) {
@@ -220,27 +221,14 @@ class ContextMenu {
     }
 
     async hide() {
-        // this date checking comes from change in FF66 - https://github.com/zadam/trilium/issues/468
-        // "contextmenu" event also triggers "click" event which depending on the timing can close the just opened context menu
-        // we might filter out right clicks, but then it's better if even right clicks close the context menu
-        if (Date.now() - this.dateContextMenuOpenedMs > 300) {
-            // seems like if we hide the menu immediately, some clicks can get propagated to the underlying component
-            // see https://github.com/zadam/trilium/pull/3805 for details
-            await timeout(100);
-            this.$widget.removeClass("show");
-            this.$cover.removeClass("show");
-            $("body").removeClass("context-menu-shown");
-            this.$widget.hide();
-        }
+        this.options?.onHide?.();
+        this.$widget.removeClass("show");
+        this.$cover.removeClass("show");
+        $("body").removeClass("context-menu-shown");
+        this.$widget.hide();
     }
 }
 
-function timeout(ms: number) {
-    return new Promise((accept, reject) => {
-        setTimeout(accept, ms);
-    });
-}
-
 const contextMenu = new ContextMenu();
 
 export default contextMenu;

apps/client/src/menus/tree_context_menu.ts

@@ -19,6 +19,8 @@ interface ConvertToAttachmentResponse {
     attachment?: FAttachment;
 }
 
+let lastTargetNode: HTMLElement | null = null;
+
 // This will include all commands that implement ContextMenuCommandData, but it will not work if it additional options are added via the `|` operator,
 // so they need to be added manually.
 export type TreeCommandNames = FilteredCommandNames<ContextMenuCommandData> | "openBulkActionsDialog";
@@ -33,12 +35,19 @@ export default class TreeContextMenu implements SelectMenuItemEventListener<Tree
     }
 
     async show(e: PointerEvent | JQuery.TouchStartEvent | JQuery.ContextMenuEvent) {
-        contextMenu.show({
+        await contextMenu.show({
             x: e.pageX ?? 0,
             y: e.pageY ?? 0,
             items: await this.getMenuItems(),
-            selectMenuItemHandler: (item, e) => this.selectMenuItemHandler(item)
+            selectMenuItemHandler: (item, e) => this.selectMenuItemHandler(item),
+            onHide: () => {
+                lastTargetNode?.classList.remove('fancytree-menu-target');
+            }
         });
+        // It's placed after show to ensure the old target is cleared before showing the context menu again on repeated right-clicks.
+        lastTargetNode?.classList.remove('fancytree-menu-target');
+        lastTargetNode = this.node.span;
+        lastTargetNode.classList.add('fancytree-menu-target');
     }
 
     async getMenuItems(): Promise<MenuItem<TreeCommandNames>[]> {

apps/client/src/stylesheets/tree.css

@@ -208,6 +208,10 @@ span.fancytree-node:hover {
     border: 1px solid var(--main-border-color);
 }
 
+span.fancytree-node.fancytree-menu-target {
+    box-shadow: inset 0 0 0 1px var(--main-border-color);
+}
+
 .fancytree-title:hover,
 span.fancytree-node:hover .fancytree-title {
     border: 0;