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

@@ -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 = $('<div class="context-menu-sheet-backdrop"></div>');
         $('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

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

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