From aa5e398e8be59e0f42060a67b35a8519c84e622e Mon Sep 17 00:00:00 2001 From: Miika Kuisma Date: Mon, 27 Apr 2026 22:18:04 +0300 Subject: [PATCH] Fixing issues with mobile contextMenu (#2848) * Fixing issues with mobile contextMenu in puter env app where user couldn't tap on menu items and there was checkmark in wrong place * context menu mobile version - handle submenus * context menu mobile icon placement tweak --- src/gui/src/UI/UIContextMenu.js | 44 +++++++++++----- src/gui/src/css/style.css | 15 +++++- .../src/ui/components/PuterContextMenu.js | 52 +++++++++++++++---- 3 files changed, 86 insertions(+), 25 deletions(-) diff --git a/src/gui/src/UI/UIContextMenu.js b/src/gui/src/UI/UIContextMenu.js index bc3ae06b7..7d051e39b 100644 --- a/src/gui/src/UI/UIContextMenu.js +++ b/src/gui/src/UI/UIContextMenu.js @@ -510,8 +510,9 @@ function UIContextMenu (options) { const contextMenu = document.getElementById(`context-menu-${menu_id}`); - // iOS-style action sheet on mobile (top-level menus only; submenus cascade). - const is_sheet = (isMobile.phone || isMobile.tablet) && !options.is_submenu; + // iOS-style action sheet on mobile. Both top-level menus and submenus use + // sheet styling; the submenu replaces (visually) the parent sheet. + const is_sheet = isMobile.phone || isMobile.tablet; let $sheet_backdrop = null; let x_pos = 0; let y_pos = 0; @@ -595,22 +596,35 @@ function UIContextMenu (options) { let cancel_options_ = null; const fade_remove = (item) => { - $(`#context-menu-${menu_id}, .context-menu[data-element-id="${$(item).closest('.context-menu').attr('data-parent-id')}"]`).fadeOut(200, function () { - $(contextMenu).remove(); - }); + const parent_data_id = $(item).closest('.context-menu').attr('data-parent-id'); + const $stack = $(`#context-menu-${menu_id}, .context-menu[data-element-id="${parent_data_id}"]`); + // Animate visible menus out. fadeOut's callback does not fire on + // already-hidden elements (e.g. a parent sheet hidden when its submenu + // opened), so force-remove the whole stack after the animation. + $stack.fadeOut(200); + setTimeout(() => $stack.remove(), 220); }; const remove = () => { $(contextMenu).remove(); }; - // Sheet-mode backdrop: tap outside to dismiss - if ( is_sheet ) { + // Sheet-mode backdrop: tap outside to dismiss. Only the top-level sheet + // owns a backdrop; submenu sheets reuse the parent's backdrop. + if ( is_sheet && !options.is_submenu ) { $sheet_backdrop = $('
'); $('body').append($sheet_backdrop); + // touchstart is registered as passive by jQuery (see initgui.js), so + // preventDefault would be a no-op and emit a console warning. stopPropagation + // is enough to keep the document-level dismiss handler from running twice. $sheet_backdrop.on('mousedown touchstart', (e) => { - e.preventDefault(); e.stopPropagation(); - fade_remove($sheet_backdrop); + // Close every sheet (parent + any open submenus) so a backdrop tap + // dismisses the whole stack at once. fadeOut's callback won't fire + // for elements already hidden, so force-remove after the animation. + const $sheets = $('.context-menu.context-menu-sheet'); + $sheets.fadeOut(200); + setTimeout(() => $sheets.remove(), 220); + $sheet_backdrop.remove(); }); } @@ -698,6 +712,13 @@ function UIContextMenu (options) { submenu_x_pos = x_pos + item_rect_box.width + 15; } + // On mobile sheet mode, hide the parent sheet so the + // submenu visually replaces it (still in DOM so the + // existing fade_remove cascade can clean it up). + if ( is_sheet ) { + $(contextMenu).hide(); + } + // open the new submenu UIContextMenu({ items: options.items[parseInt($(e).attr('data-action'))].items, @@ -738,11 +759,10 @@ function UIContextMenu (options) { }); // Useful in cases such as where a menu item is over a window, this prevents the mousedown event from - // reaching the window underneath + // reaching the window underneath. Note: do NOT call preventDefault here — on iOS Safari that + // cancels the synthesized click event, which breaks tapping items on mobile. $(`#context-menu-${menu_id} > li:not(.context-menu-item-disabled)`).on('mousedown', function (e) { - e.preventDefault(); e.stopPropagation(); - return false; }); // Disable parent scroll diff --git a/src/gui/src/css/style.css b/src/gui/src/css/style.css index 1a67b62a0..6e3ccef40 100644 --- a/src/gui/src/css/style.css +++ b/src/gui/src/css/style.css @@ -1827,11 +1827,17 @@ span.header-sort-icon img { .context-menu.context-menu-sheet .context-menu-item .ctx-item-icon, .context-menu.context-menu-sheet .context-menu-item .context-menu-item-icon, .context-menu.context-menu-sheet .context-menu-item .context-menu-item-icon-active { - left: 16px; + position: absolute; + left: 9px; top: 50%; transform: translateY(-50%); width: 20px; height: 20px; + margin: 0; + line-height: 1; + display: flex; + align-items: center; + justify-content: center; } .context-menu.context-menu-sheet .context-menu-item-active:not(.context-menu-divider) { @@ -1866,7 +1872,12 @@ span.header-sort-icon img { left: 0; right: 0; bottom: 0; - z-index: 9999999998; + /* Must be a value within the 32-bit signed int range. The .context-menu rule + above uses 9999999999 which browsers clamp to INT_MAX (2147483647); using + 9999999998 here would also clamp to the same value, leaving the backdrop + and menu at equal z-index — and since the backdrop is appended after the + menu, DOM order would put it on top and swallow every tap. */ + z-index: 2147483646; background: transparent; } diff --git a/src/puter-js/src/ui/components/PuterContextMenu.js b/src/puter-js/src/ui/components/PuterContextMenu.js index 4969e4530..59488fb5d 100644 --- a/src/puter-js/src/ui/components/PuterContextMenu.js +++ b/src/puter-js/src/ui/components/PuterContextMenu.js @@ -364,12 +364,14 @@ class PuterContextMenu extends PuterWebComponent { const menu = this.$('.context-menu'); if ( ! menu ) return; - // On mobile/touch devices, render as action sheet (bottom-anchored) - // Skip for submenus — they cascade as nested popovers, not sheets. - if ( this._isMobile() && !this.hasAttribute('data-submenu') ) { + // On mobile/touch devices, render as an iOS-style action sheet + // (bottom-anchored). Submenus also use sheet mode and visually + // replace the parent — only the root sheet owns the backdrop. + if ( this._isMobile() ) { this.classList.add('sheet-mode'); - // Add backdrop overlay for action sheet mode - this._showBackdrop(); + if ( ! this.hasAttribute('data-submenu') ) { + this._showBackdrop(); + } return; } @@ -738,6 +740,13 @@ class PuterContextMenu extends PuterWebComponent { this._closeAll(); }); + // On mobile sheet mode, hide self so the submenu visually replaces + // the parent sheet. We restore display in _hideActiveSubmenu. + if ( this.classList.contains('sheet-mode') ) { + this.style.display = 'none'; + this._sheetHidden = true; + } + document.body.appendChild(submenu); this.#activeSubmenu = { element: submenu, parentEl }; @@ -879,12 +888,24 @@ class PuterContextMenu extends PuterWebComponent { } } - _hideActiveSubmenu () { + _hideActiveSubmenu (restoreSelf = true) { if ( this.#activeSubmenu ) { - this.#activeSubmenu.element.remove(); + // If the submenu is animating itself out (e.g. its own _closeAll is + // running), don't yank it out of the DOM — let the animation finish. + if ( ! this.#activeSubmenu.element._closing ) { + this.#activeSubmenu.element.remove(); + } this.#activeSubmenu.parentEl.classList.remove('has-open-submenu'); this.#activeSubmenu = null; } + // Restore self if we were hidden for a sheet-mode submenu (e.g. user + // pressed ArrowLeft to return to the parent sheet). Skip restore when + // the parent is closing — restoring would briefly flash the parent + // visible before it animates out. + if ( restoreSelf && this._sheetHidden ) { + this.style.display = ''; + this._sheetHidden = false; + } // Apply deferred focus from safe-triangle hover if ( this.#pendingFocusIndex !== null ) { this._setFocusIndex(this.#pendingFocusIndex); @@ -898,7 +919,10 @@ class PuterContextMenu extends PuterWebComponent { this._cancelSubmenuClose(); clearTimeout(this.#submenuTimeout); clearTimeout(this.#typeaheadTimer); - this._hideActiveSubmenu(); + // Don't restore display when we're already closing — restoring would + // make the parent sheet briefly flash back before being removed. + const wasHidden = this._sheetHidden; + this._hideActiveSubmenu(false); if ( this._outsideClickHandler ) { document.removeEventListener('click', this._outsideClickHandler, true); } @@ -911,11 +935,17 @@ class PuterContextMenu extends PuterWebComponent { } this.emitEvent('close', {}); - // Sheet-mode close: animate down, then remove + // Sheet-mode close: animate down only if we're currently visible. A + // sheet that was hidden because a submenu replaced it has nothing + // visible to animate, so just clean up silently. if ( this.classList.contains('sheet-mode') ) { - this.classList.add('sheet-closing'); this._hideBackdrop(); - setTimeout(() => this.remove(), 250); + if ( wasHidden ) { + this.remove(); + } else { + this.classList.add('sheet-closing'); + setTimeout(() => this.remove(), 250); + } } else { this.remove(); }