Merge pull request #20923 from calixteman/bug2023138

Make sure the menuitems are readable when hovering them (bug 2023138)

web/menu.css

@@ -105,10 +105,10 @@ button.hasPopupMenu {
     --menu-mark-icon-size: 16px;
   }
 
-  > li {
+  > button {
     display: flex;
     align-items: center;
-    list-style: none;
+    position: relative;
     width: 100%;
     height: 26px;
     padding-inline: calc(var(--menu-mark-icon-size) + var(--menuitem-gap))
@@ -119,7 +119,7 @@ button.hasPopupMenu {
     border: 1px solid var(--menuitem-border-color);
     background: transparent;
 
-    &:has(button.selected)::before {
+    &.selected::after {
       content: "";
       display: inline-block;
       width: 11px;
@@ -129,30 +129,41 @@ button.hasPopupMenu {
       mask-image: var(--menuitem-checkmark-icon);
       background-color: var(--menu-text-color);
       position: absolute;
-      margin-inline-start: -16px;
+      inset-inline-start: var(--menuitem-gap);
+      top: 50%;
+      transform: translateY(-50%);
     }
 
-    &:has(button:disabled) {
+    &:disabled {
       opacity: var(--disabled-opacity);
-      pointer-events: none;
 
-      &:has(button.selected)::before {
+      &.selected::after {
         background-color: var(--menu-text-disabled-color);
       }
+
+      &:not(.noIcon)::before {
+        background-color: var(--menu-text-disabled-color);
       }
 
+      > span {
+        color: var(--menu-text-disabled-color);
+      }
+    }
+
+    &:not(:disabled) {
       &:hover {
         background: var(--menuitem-hover-bg);
         background-blend-mode: var(--menuitem-hover-background-blend-mode);
-      > button {
+
         &:not(.noIcon)::before {
           background-color: var(--menuitem-text-hover-fg);
         }
+
         > span {
           color: var(--menuitem-text-hover-fg);
         }
-      }
+
-      &:has(button.selected)::before {
+        &.selected::after {
           background-color: var(--menuitem-text-hover-fg);
         }
       }
@@ -160,28 +171,19 @@ button.hasPopupMenu {
       &:active {
         background-color: var(--menuitem-active-bg);
         border-color: var(--menuitem-active-border-color);
-      > button > span {
+
+        > span {
           color: var(--menuitem-text-active-fg);
         }
       }
 
-    &:has(> button:focus-visible) {
+      &:focus-visible {
         border-color: var(--menuitem-focus-border-color);
         background-color: var(--menuitem-focus-bg);
         outline: 2px solid var(--menuitem-focus-outline-color);
         outline-offset: 2px;
       }
-
+    }
-    > button {
-      display: flex;
-      flex-direction: row;
-      align-items: center;
-      width: 100%;
-      height: auto;
-      padding: var(--menuitem-gap);
-      gap: var(--menuitem-gap);
-      background: transparent;
-      border: none;
 
     &:not(.noIcon)::before {
       display: inline-block;
@@ -193,14 +195,12 @@ button.hasPopupMenu {
       background-color: var(--menu-text-color);
     }
 
-      &:focus-visible {
-        outline: none;
-      }
-
     > span {
-        display: inline-block;
+      display: flex;
-        width: max-content;
+      align-items: center;
-        height: auto;
+      align-self: stretch;
+      width: 100%;
+      min-width: max-content;
       text-align: start;
       color: var(--menu-text-color);
       user-select: none;
@@ -212,18 +212,5 @@ button.hasPopupMenu {
       font-weight: 510;
       line-height: normal;
     }
-
-      &:disabled {
-        pointer-events: none;
-
-        &:not(.noIcon)::before {
-          background-color: var(--menu-text-disabled-color);
-        }
-
-        > span {
-          color: var(--menu-text-disabled-color);
-        }
-      }
-    }
   }
 }

web/viewer.html

@@ -142,26 +142,18 @@ See https://github.com/adobe-type-tools/cmap-resources
                           <span data-l10n-id="pdfjs-views-manager-view-selector-button-label"></span>
                         </button>
                         <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>
                           </button>
-                          </li>
-                          <li>
                           <button id="outlinesViewMenu" role="option" type="button" tabindex="-1">
                             <span data-l10n-id="pdfjs-views-manager-outlines-option-label"></span>
                           </button>
-                          </li>
-                          <li>
                           <button id="attachmentsViewMenu" role="option" type="button" tabindex="-1">
                             <span data-l10n-id="pdfjs-views-manager-attachments-option-label"></span>
                           </button>
-                          </li>
-                          <li>
                           <button id="layersViewMenu" role="option" type="button" tabindex="-1">
                             <span data-l10n-id="pdfjs-views-manager-layers-option-label"></span>
                           </button>
-                          </li>
                         </menu>
                       </div>
                       <span id="viewsManagerHeaderLabel" class="viewsManagerLabel" role="heading" aria-level="2"></span>
@@ -217,26 +209,18 @@ See https://github.com/adobe-type-tools/cmap-resources
                             <span data-l10n-id="pdfjs-views-manager-pages-status-action-button-label"></span>
                           </button>
                           <menu id="viewsManagerStatusActionOptions" class="popupMenu">
-                            <li>
                             <button id="viewsManagerStatusActionCopy" class="noIcon" role="menuitem" type="button" tabindex="-1" disabled>
                               <span data-l10n-id="pdfjs-views-manager-pages-status-copy-button-label"></span>
                             </button>
-                            </li>
-                            <li>
                             <button id="viewsManagerStatusActionCut" class="noIcon" role="menuitem" type="button" tabindex="-1" disabled>
                               <span data-l10n-id="pdfjs-views-manager-pages-status-cut-button-label"></span>
                             </button>
-                            </li>
-                            <li>
                             <button id="viewsManagerStatusActionDelete" class="noIcon" role="menuitem" type="button" tabindex="-1" disabled>
                               <span data-l10n-id="pdfjs-views-manager-pages-status-delete-button-label"></span>
                             </button>
-                            </li>
-                            <li>
                             <button id="viewsManagerStatusActionExport" class="noIcon" role="menuitem" type="button" tabindex="-1" disabled>
                               <span data-l10n-id="pdfjs-views-manager-pages-status-export-selected-button-label"></span>
                             </button>
-                            </li>
                           </menu>
                         </div>
                       </div>

web/views_manager.css

@@ -337,7 +337,7 @@
           min-width: 182px;
           z-index: 1;
 
-          > li > button {
+          > button {
             &#thumbnailsViewMenu::before {
               mask-image: var(--menuitem-thumbnailsView-icon);
             }