From 1edac26805806ebe5ef53dd79cd257bb41a20124 Mon Sep 17 00:00:00 2001 From: Nick O'Leary Date: Tue, 10 Mar 2026 15:02:10 +0000 Subject: [PATCH] Apply suggestions from code review --- .../editor-client/src/js/ui/common/menu.js | 24 +++++++++++- .../editor-client/src/js/ui/contextMenu.js | 39 ++++++++----------- 2 files changed, 40 insertions(+), 23 deletions(-) diff --git a/packages/node_modules/@node-red/editor-client/src/js/ui/common/menu.js b/packages/node_modules/@node-red/editor-client/src/js/ui/common/menu.js index 3b3b1a0b9..3909b0696 100644 --- a/packages/node_modules/@node-red/editor-client/src/js/ui/common/menu.js +++ b/packages/node_modules/@node-red/editor-client/src/js/ui/common/menu.js @@ -275,8 +275,30 @@ RED.menu = (function() { var {x, y} = getSubmenuPosition(item[0], submenu[0], direction); submenu.css({ 'top': y + 'px', - 'left': x + 'px' + 'left': x + 'px', + maxHeight: "" }); + const windowHeight = $(window).height(); + const menuHeight = submenu.outerHeight(); + const spaceAbove = y; + const bottomOverlap = menuHeight - (windowHeight - y); + if (bottomOverlap > 0) { + if (spaceAbove > bottomOverlap + 5) { + // There is room to move the menu up and still show it all + submenu.css({ + top: (menuHeight - bottomOverlap - 5) + "px" + }) + } else { + // There is not room for the whole menu. + // 1. move it to the top + // 2. enable overflow/scrolling + submenu.css({ + top: "5px", + maxHeight: (windowHeight - 25) + "px", + overflowY: "auto" + }); + } + } } function cleanUpSubmenu() { diff --git a/packages/node_modules/@node-red/editor-client/src/js/ui/contextMenu.js b/packages/node_modules/@node-red/editor-client/src/js/ui/contextMenu.js index 0bcceadaa..7369af721 100644 --- a/packages/node_modules/@node-red/editor-client/src/js/ui/contextMenu.js +++ b/packages/node_modules/@node-red/editor-client/src/js/ui/contextMenu.js @@ -275,10 +275,10 @@ RED.contextMenu = (function () { }); menu.show(); - var menuHeight = menu.outerHeight(); - var menuWidth = menu.outerWidth(); - var spaceBelow = windowHeight - top; - var spaceAbove = top; + const menuHeight = menu.outerHeight(); + const menuWidth = menu.outerWidth(); + const spaceAbove = top; + const bottomOverlap = menuHeight - (windowHeight - top); // Adjust horizontal position if menu would overflow right edge if (left + menuWidth > windowWidth - 10) { @@ -287,26 +287,21 @@ RED.contextMenu = (function () { } // Check if menu overflows below viewport - if (menuHeight > spaceBelow - 10) { - // Try repositioning above the click point first - if (spaceAbove > spaceBelow && menuHeight <= spaceAbove - 10) { + if (bottomOverlap > 0) { + if (spaceAbove > bottomOverlap + 5) { + // There is room to move the menu up and still show it all menu.css({ - top: (top - menuHeight) + "px" - }); + top: (top - bottomOverlap - 5) + "px" + }) } else { - // Enable scrolling with max available space above or below - if (spaceAbove > spaceBelow) { - menu.css({ - top: "10px", - maxHeight: (top - 20) + "px", - overflowY: "auto" - }); - } else { - menu.css({ - maxHeight: (spaceBelow - 10) + "px", - overflowY: "auto" - }); - } + // There is not room for the whole menu. + // 1. move it to the top + // 2. enable overflow/scrolling + menu.css({ + top: "5px", + maxHeight: (windowHeight - 25) + "px", + overflowY: "auto" + }); } } // set focus to first item so that pressing escape key closes the menu