From 50bf999a1219b3d2e4e132d86d063e0b270605df Mon Sep 17 00:00:00 2001
From: Raymond Hill <rhill@raymondhill.net>
Date: Mon, 3 Aug 2020 08:55:02 -0400
Subject: [PATCH] Fine tune CodeMirror editor's search widget

Code review following latest changes.

Also, move the input field to the left so that it
renders properly on smaller displays and does not
jump around when the result position/count numbers
change.

This also makes it easier to add more functionality
to the editor's toolbar in the future.
---
 src/css/codemirror.css             | 39 ++++++++++++------------------
 src/js/codemirror/search-thread.js |  9 ++++---
 src/js/codemirror/search.js        | 36 +++++++++------------------
 3 files changed, 33 insertions(+), 51 deletions(-)

diff --git a/src/css/codemirror.css b/src/css/codemirror.css
index e17b5caa3..b7e598ecc 100644
--- a/src/css/codemirror.css
+++ b/src/css/codemirror.css
@@ -64,14 +64,17 @@ div.CodeMirror span.CodeMirror-matchingbracket {
     direction: ltr;
     display: flex;
     flex-shrink: 0;
-    font-size: 110%;
-    justify-content: center;
+    justify-content: space-between;
     padding: 0.5em;
     user-select: none;
       -moz-user-select: none;
       -webkit-user-select: none;
     z-index: 1000;
     }
+.cm-search-widget-input {
+    display: inline-flex;
+    flex-grow: 1;
+    }
 .cm-search-widget .fa-icon {
     fill: #888;
     font-size: 140%;
@@ -79,37 +82,25 @@ div.CodeMirror span.CodeMirror-matchingbracket {
 .cm-search-widget .fa-icon:not(.fa-icon-ro):hover {
     fill: #000;
     }
-.cm-search-widget-input {
-    border: 1px solid gray;
-    border-radius: 3px;
+.cm-search-widget-input input {
     display: inline-flex;
-    min-width: 14em;
-    width: 30vw;
-    }
-.cm-search-widget-input > input {
-    border: 0;
     flex-grow: 1;
-    width: 100%;
+    max-width: 16em;
     }
-.cm-search-widget-input > .cm-search-widget-count {
+.cm-search-widget-count {
     align-items: center;
-    display: none;
-    flex-grow: 0;
-    font-size: 80%;
-    padding: 0 0.4em;
-    }
-.cm-search-widget[data-query] .cm-search-widget-count {
     display: inline-flex;
+    flex-grow: 0;
+    font-size: 95%;
+    min-width: 6em;
+    visibility: hidden;
+    }
+.cm-search-widget[data-query] .cm-search-widget-count:not(:empty) {
+    visibility: visible;
     }
 .cm-search-widget .cm-search-widget-button:hover {
     color: #000;
     }
-.cm-search-widget .sourceURL {
-    padding-left: 0.5em;
-    padding-right: 0.5em;
-    position: absolute;
-    left: 0;
-    }
 .cm-search-widget .sourceURL[href=""] {
     display: none;
     }
diff --git a/src/js/codemirror/search-thread.js b/src/js/codemirror/search-thread.js
index c3eda6de8..7b33fb126 100644
--- a/src/js/codemirror/search-thread.js
+++ b/src/js/codemirror/search-thread.js
@@ -152,9 +152,6 @@ if (
             clearTimeout(workerTTLTimer);
         }
         workerTTLTimer = vAPI.setTimeout(shutdown, workerTTL);
-        self.addEventListener('beforeunload', ( ) => {
-            destroy();
-        }, { once: true });
     };
 
     const needHaystack = function() {
@@ -187,6 +184,12 @@ if (
         });
     };
 
+    self.addEventListener(
+        'beforeunload',
+        ( ) => { destroy(); },
+        { once: true }
+    );
+
     self.searchThread = { needHaystack, setHaystack, search, shutdown };
 }
 
diff --git a/src/js/codemirror/search.js b/src/js/codemirror/search.js
index 4c0f7e86c..9da1b401c 100644
--- a/src/js/codemirror/search.js
+++ b/src/js/codemirror/search.js
@@ -135,7 +135,7 @@
             }
         });
         cm.on('cursorActivity', cm => {
-            updateRank(cm);
+            updateCount(cm);
         });
     };
 
@@ -239,16 +239,6 @@
     };
 
     const updateCount = function(cm) {
-        const state = getSearchState(cm);
-        const count = state.lines.length;
-        const span = state.widget.querySelector(
-            '.cm-search-widget-count > span:nth-of-type(2)'
-        );
-        span.textContent = formatNumber(count);
-        span.title = count.toLocaleString();
-    };
-
-    const updateRank = function(cm) {
         const state = getSearchState(cm);
         const lines = state.lines;
         const current = cm.getCursor().line;
@@ -273,10 +263,11 @@
             }
             text = text + '\xA0/\xA0';
         }
-        const span = state.widget.querySelector(
-            '.cm-search-widget-count > span:nth-of-type(1)'
-        );
+        const count = lines.length;
+        text += formatNumber(count);
+        const span = state.widget.querySelector('.cm-search-widget-count');
         span.textContent = text;
+        span.title = count.toLocaleString();
     };
 
     const startSearch = function(cm, state) {
@@ -294,7 +285,6 @@
             if ( Array.isArray(lines) === false ) { return; }
             state.lines = lines;
             const count = lines.length;
-            updateRank(cm);
             updateCount(cm);
             if ( state.annotate !== undefined ) {
                 state.annotate.clear();
@@ -330,7 +320,7 @@
         });
         state.widget.setAttribute('data-query', state.queryText);
         // Ensure the caret is visible
-        let input = state.widget.querySelector('.cm-search-widget-input > input');
+        const input = state.widget.querySelector('.cm-search-widget-input input');
         input.selectionStart = input.selectionStart;
     };
 
@@ -432,15 +422,13 @@
         const searchWidgetTemplate =
             '<div class="cm-search-widget-template" style="display:none;">' +
               '<div class="cm-search-widget">' +
-                '<span class="fa-icon fa-icon-ro">search</span>&ensp;' +
                 '<span class="cm-search-widget-input">' +
-                  '<input type="search" spellcheck="false">' +
-                  '<span class="cm-search-widget-count">' +
-                    '<span></span><span>0</span>' +
-                  '</span>' +
-                '</span>&ensp;' +
-                '<span class="cm-search-widget-up cm-search-widget-button fa-icon">angle-up</span>&nbsp;' +
-                '<span class="cm-search-widget-down cm-search-widget-button fa-icon fa-icon-vflipped">angle-up</span>' +
+                  '<span class="fa-icon fa-icon-ro">search</span>&ensp;' +
+                  '<input type="search" spellcheck="false">&emsp;' +
+                  '<span class="cm-search-widget-up cm-search-widget-button fa-icon">angle-up</span>&nbsp;' +
+                  '<span class="cm-search-widget-down cm-search-widget-button fa-icon fa-icon-vflipped">angle-up</span>&emsp;' +
+                  '<span class="cm-search-widget-count"></span>' +
+                '</span>' +
                 '<a class="fa-icon sourceURL" href>external-link</a>' +
               '</div>' +
             '</div>';