Merge pull request #20583 from calixteman/simplify_menu

Hide the menu container in changing it's visibility

Author: timvandermeij

web/menu.css

@@ -13,6 +13,16 @@
  * limitations under the License.
  */
 
+button.hasPopupMenu {
+  &[aria-expanded="true"] + menu {
+    visibility: visible;
+  }
+
+  &[aria-expanded="false"] + menu {
+    visibility: hidden;
+  }
+}
+
 .popupMenu {
   --menuitem-checkmark-icon: url(images/checkmark.svg);
   --menu-mark-icon-size: 0;
@@ -75,6 +85,7 @@
   top: 1px;
   margin: 0;
   padding: 5px;
+  box-sizing: border-box;
 
   background: var(--menu-bg);
   background-blend-mode: var(--menu-background-blend-mode);

web/menu.js

@@ -56,7 +56,6 @@ class Menu {
       return;
     }
     const menu = this.#menu;
-    menu.classList.toggle("hidden", true);
     this.#triggeringButton.ariaExpanded = "false";
     this.#openMenuAC.abort();
     this.#openMenuAC = null;
@@ -82,7 +81,6 @@ class Menu {
       }
 
       const menu = this.#menu;
-      menu.classList.toggle("hidden", false);
       this.#triggeringButton.ariaExpanded = "true";
       this.#openMenuAC = new AbortController();
       const signal = AbortSignal.any([
@@ -137,6 +135,13 @@ class Menu {
               .focus();
             stopEvent(e);
             break;
+          default:
+            const char = e.key.toLocaleLowerCase();
+            this.#goToNextItem(e.target, true, item =>
+              item.textContent.trim().toLowerCase().startsWith(char)
+            );
+            stopEvent(e);
+            break;
         }
       },
       { signal, capture: true }
@@ -148,32 +153,38 @@ class Menu {
     });
     this.#triggeringButton.addEventListener(
       "keydown",
-      ev => {
-        if (!this.#openMenuAC) {
-          return;
-        }
-        switch (ev.key) {
+      e => {
+        switch (e.key) {
+          case " ":
+          case "Enter":
           case "ArrowDown":
           case "Home":
+            if (!this.#openMenuAC) {
+              this.#triggeringButton.click();
+            }
             this.#menuItems
               .find(
                 item => !item.disabled && !item.classList.contains("hidden")
               )
               .focus();
-            stopEvent(ev);
+            stopEvent(e);
             break;
           case "ArrowUp":
           case "End":
+            if (!this.#openMenuAC) {
+              this.#triggeringButton.click();
+            }
             this.#menuItems
               .findLast(
                 item => !item.disabled && !item.classList.contains("hidden")
               )
               .focus();
-            stopEvent(ev);
+            stopEvent(e);
             break;
           case "Escape":
             this.#closeMenu();
-            stopEvent(ev);
+            stopEvent(e);
+            break;
         }
       },
       { signal }
@@ -185,7 +196,7 @@ class Menu {
    * @param {HTMLElement} element
    * @param {boolean} forward
    */
-  #goToNextItem(element, forward) {
+  #goToNextItem(element, forward, check = () => true) {
     const index =
       this.#lastIndex === -1
         ? this.#menuItems.indexOf(element)
@@ -198,7 +209,11 @@ class Menu {
       i = (i + increment) % len
     ) {
       const menuItem = this.#menuItems[i];
-      if (!menuItem.disabled && !menuItem.classList.contains("hidden")) {
+      if (
+        !menuItem.disabled &&
+        !menuItem.classList.contains("hidden") &&
+        check(menuItem)
+      ) {
         menuItem.focus();
         this.#lastIndex = i;
         break;

web/viewer.html

@@ -130,7 +130,7 @@
                     <div id="viewsManagerTitle">
                       <div id="viewsManagerSelector">
                         <button
-                          class="toolbarButton viewsManagerButton"
+                          class="toolbarButton viewsManagerButton hasPopupMenu"
                           type="button"
                           id="viewsManagerSelectorButton"
                           tabindex="0"
@@ -141,7 +141,7 @@
                         >
                           <span data-l10n-id="pdfjs-views-manager-view-selector-button-label"></span>
                         </button>
-                        <menu id="viewsManagerSelectorOptions" role="listbox" class="popupMenu hidden withMark">
+                        <menu id="viewsManagerSelectorOptions" role="listbox" class="popupMenu withMark">
                           <li>
                             <button id="thumbnailsViewMenu" role="option" type="button" tabindex="-1">
                               <span data-l10n-id="pdfjs-views-manager-pages-option-label"></span>
@@ -196,15 +196,16 @@
                         <div id="actionSelector">
                           <button
                             id="viewsManagerStatusActionButton"
-                            class="viewsManagerButton"
+                            class="viewsManagerButton hasPopupMenu"
                             type="button"
                             tabindex="0"
                             aria-haspopup="menu"
                             aria-controls="viewsManagerStatusActionOptions"
+                            aria-expanded="false"
                           >
                             <span data-l10n-id="pdfjs-views-manager-pages-status-action-button-label"></span>
                           </button>
-                          <menu id="viewsManagerStatusActionOptions" class="popupMenu hidden">
+                          <menu id="viewsManagerStatusActionOptions" class="popupMenu">
                             <li>
                               <button id="viewsManagerStatusActionCopy" class="noIcon" role="menuitem" type="button" tabindex="0">
                                 <span data-l10n-id="pdfjs-views-manager-pages-status-copy-button-label"></span>

web/views_manager.css

@@ -394,6 +394,7 @@
 
         #actionSelector {
           height: 32px;
+          min-width: 115px;
           width: auto;
           display: block;
 
@@ -425,8 +426,9 @@
             }
           }
 
-          > .contextMenu {
-            min-width: 115px;
+          > .popupMenu {
+            width: auto;
+            z-index: 1;
           }
         }
       }