Re-factor the loading-icons used in the viewer toolbars

Currently the SVG images for the loading-icons exist in two versions, for the light- respectively dark-theme, which nowadays are the only "duplicated" icons left. The reason for this is that these icons are being used in `input`-elements, where the regular `mask-image` approach used for all buttons don't work. This patch proposes that we "cheat" a little bit here, and instead display the loading-icons on neighbouring DOM-elements that support `mask-image`.
mozilla · Nov 17, 2023 · 7c870ef · 7c870ef
1 parent ffbfd68
commit 7c870ef
Show file tree

Hide file tree

Showing 4 changed files with 54 additions and 64 deletions.
diff --git a/web/images/loading-dark.svg b/web/images/loading-dark.svg
diff --git a/web/toolbar.js b/web/toolbar.js
@@ -22,8 +22,6 @@ import {
   toggleCheckedBtn,
 } from "./ui_utils.js";
 
-const PAGE_NUMBER_LOADING_INDICATOR = "visiblePageIsLoading";
-
 /**
  * @typedef {Object} ToolbarOptions
  * @property {HTMLDivElement} container - Container for the secondary toolbar.
@@ -300,7 +298,7 @@ class Toolbar {
   updateLoadingIndicatorState(loading = false) {
     const { pageNumber } = this.items;
 
-    pageNumber.classList.toggle(PAGE_NUMBER_LOADING_INDICATOR, loading);
+    pageNumber.classList.toggle("loading", loading);
   }
 }
 

diff --git a/web/viewer.css b/web/viewer.css
@@ -168,11 +168,6 @@
     --doorhanger-separator-color: rgb(92 92 97);
     --dialog-button-bg-color: rgb(92 92 97);
     --dialog-button-hover-bg-color: rgb(115 115 115);
-
-    /* This image is used in <input> elements, which unfortunately means that
-     * the `mask-image` approach used with all of the other images doesn't work
-     * here; hence why we still have two versions of this particular image. */
-    --loading-icon: url(images/loading-dark.svg);
   }
 }
 
@@ -494,22 +489,40 @@ body {
 
 #findInput {
   width: 200px;
+
+  /*#if !MOZCENTRAL*/
+  &::-webkit-input-placeholder {
+    color: rgb(191 191 191);
+  }
+  /*#endif*/
+  &::placeholder {
+    font-style: normal;
+  }
+  &[data-status="notFound"] {
+    background-color: rgb(255 102 102);
+  }
 }
-/*#if !MOZCENTRAL*/
-#findInput::-webkit-input-placeholder {
-  color: rgb(191 191 191);
-}
-/*#endif*/
-#findInput::placeholder {
-  font-style: normal;
-}
-#findInput[data-status="pending"] {
-  background-image: var(--loading-icon);
-  background-repeat: no-repeat;
-  background-position: calc(50% + 48% * var(--dir-factor));
+
+:is(#findPrevNext, #numPages)::before {
+  position: absolute;
+  visibility: hidden;
+  top: 6px;
+  width: 16px;
+  height: 16px;
+
+  content: "";
+  background-color: var(--toolbar-icon-bg-color);
+  mask-size: cover;
+  mask-image: var(--loading-icon);
 }
-#findInput[data-status="notFound"] {
-  background-color: rgb(255 102 102);
+
+#findPrevNext {
+  &::before {
+    inset-inline-start: -23px;
+  }
+  #findInput[data-status="pending"] + &::before {
+    visibility: visible;
+  }
 }
 
 .secondaryToolbar,
@@ -1100,22 +1113,13 @@ a:is(.toolbarButton, .secondaryToolbarButton)[href="#"] {
   width: 40px;
   background-size: 0 0;
   transition-property: none;
+
+  /*#if !MOZCENTRAL*/
+  &::-webkit-inner-spin-button {
+    -webkit-appearance: none;
+  }
+  /*#endif*/
 }
-#pageNumber.visiblePageIsLoading {
-  background-image: var(--loading-icon);
-  background-repeat: no-repeat;
-  background-position: calc(50% - 42% * var(--dir-factor));
-  background-size: 16px 16px;
-  /* Using a delay with background-image doesn't work,
-     consequently we use background-size. */
-  transition-property: background-size;
-  transition-delay: var(--loading-icon-delay);
-}
-/*#if !MOZCENTRAL*/
-#pageNumber::-webkit-inner-spin-button {
-  -webkit-appearance: none;
-}
-/*#endif*/
 
 .toolbarField:focus {
   border-color: #0a84ff;
@@ -1134,8 +1138,20 @@ a:is(.toolbarButton, .secondaryToolbarButton)[href="#"] {
   cursor: default;
 }
 
-#numPages.toolbarLabel {
-  padding-inline-start: 3px;
+#numPages {
+  &.toolbarLabel {
+    padding-inline-start: 3px;
+  }
+
+  &::before {
+    inset-inline-start: -54px;
+
+    transition-property: visibility;
+    transition-delay: var(--loading-icon-delay);
+  }
+  #pageNumber.loading + &::before {
+    visibility: visible;
+  }
 }
 
 #thumbnailView,

diff --git a/web/viewer.html b/web/viewer.html
@@ -139,7 +139,7 @@
         <div class="findbar hidden doorHanger" id="findbar">
           <div id="findbarInputContainer">
             <input id="findInput" class="toolbarField" title="Find" placeholder="Find in document…" tabindex="91" data-l10n-id="pdfjs-find-input" aria-invalid="false">
-            <div class="splitToolbarButton">
+            <div id="findPrevNext" class="splitToolbarButton">
               <button id="findPrevious" class="toolbarButton" title="Find the previous occurrence of the phrase" tabindex="92" data-l10n-id="pdfjs-find-previous-button">
                 <span data-l10n-id="pdfjs-find-previous-button-label">Previous</span>
               </button>