From 0161df5af0ca5c33c9688070a9cb3865540203d2 Mon Sep 17 00:00:00 2001
From: derv82 <derv82@gmail.com>
Date: Wed, 6 Dec 2017 01:22:33 -0800
Subject: [PATCH] Addressed PR feedback: i18n, result.onlick,
 description-expander, loader fix

* i18n text and titles for all labels.
* Clicking a search result opens the page for that style.
* Descriptions are truncated with (i) info button; expands description when clicked
* Simplified/fixed setLoader() + lds-spinner functionality.
---
 _locales/en/messages.json | 76 +++++++++++++++++++++++----------------
 popup.html                | 22 ++++++------
 popup/search-results.css  |  6 ++--
 popup/search-results.js   | 53 ++++++++++++++-------------
 4 files changed, 87 insertions(+), 70 deletions(-)

diff --git a/_locales/en/messages.json b/_locales/en/messages.json
index d57ea719..c1a7b3b9 100644
--- a/_locales/en/messages.json
+++ b/_locales/en/messages.json
@@ -349,43 +349,23 @@
       }
     }
   },
-  "openSearchWebsite": {
-    "message": "Search on userstyles.org",
-    "description": "Text for a link that searches for styles for this current site on userstyles.org"
-  },
-  "searchResultsHeader": {
-    "message": "Results for",
-    "description": "Text for label above search results"
-  },
-  "searchResultRating": {
-    "message": "Rating",
-    "description": "Text for label that shows the search result's rating"
-  },
-  "searchResultInstallCount": {
-    "message": "Installs",
-    "description": "Text for label that shows the number of times a search result was installed"
-  },
-  "searchResultInstall": {
-    "message": "Install",
-    "description": "Text for a button that installs a search result"
-  },
-  "searchResultCustomize": {
-    "message": "Customize",
-    "description": "Text for a button that customizes a search result before installing"
-  },
-  "searchResultCustomizeTooltip": {
-    "message": "Open this style on userstyles.org to customize via 'Advanced Style Settings'",
-    "description": "Tooltip for a button that opens style on userstyles.org for customizing"
-  },
   "filteredStylesAllHidden": {
     "message": "Currently applied filters match no styles",
     "description": "Text shown when no styles match currently applied filter in the style manager"
   },
-  "findStylesForSite": {
+  "findStyles": {
     "message": "Find styles",
     "description": "Text for a link that gets a list of styles for the current site"
   },
-  "findStylesForSiteInline": {
+  "findStylesInlineTooltip": {
+    "message": "Display search results inside this window.",
+    "description": "Text for a checkbox that displays search results within the Stylus popup."
+  },
+  "findStylesTooltip": {
+    "message": "Find more styles for this site",
+    "description": "Tooltip for a link that finds styles"
+  },
+  "findStylesInline": {
     "message": "Inline",
     "description": "Text for a checkbox that opens search results 'inline' (within the Stylus popup window)"
   },
@@ -661,6 +641,22 @@
     "message": "More Options",
     "description": "Subheading for options section on manage page."
   },
+  "paginationCurrent": {
+    "message": "Current page",
+    "description": "Tooltip for the current page index in search results"
+  },
+  "paginationNext": {
+    "message": "Next page",
+    "description": "Tooltip for the '->' (next page) button in search results"
+  },
+  "paginationPrevious": {
+    "message": "Previous page",
+    "description": "Tooltip for the '<-' button in search results"
+  },
+  "paginationTotal": {
+    "message": "Total pages",
+    "description": "Tooltip for the total page count in search results"
+  },
   "parseUsercssError": {
     "message": "Stylus failed to parse usercss:",
     "description": "The error message to show when stylus failed to parse usercss"
@@ -719,6 +715,26 @@
     "message": "Use /re/ syntax for regexp search",
     "description": "Label after the search input field in the editor shown on Ctrl-F"
   },
+  "searchResultsHeader": {
+    "message": "Results for",
+    "description": "Text for label above search results"
+  },
+  "searchResultRating": {
+    "message": "Rating",
+    "description": "Text for label that shows the search result's rating"
+  },
+  "searchResultInstallCount": {
+    "message": "Installs",
+    "description": "Text for label that shows the number of times a search result was installed"
+  },
+  "searchResultCustomize": {
+    "message": "Customize",
+    "description": "Text for a button that opens style on userstyles.org for customizing"
+  },
+  "searchResultCustomizeTooltip": {
+    "message": "Open this style on userstyles.org to customize via 'Advanced Style Settings'",
+    "description": "Tooltip for a button that opens style on userstyles.org for customizing"
+  },
   "searchStyles": {
     "message": "Search contents",
     "description": "Label for the search filter textbox on the Manage styles page"
diff --git a/popup.html b/popup.html
index 76ce9af6..129209ee 100644
--- a/popup.html
+++ b/popup.html
@@ -85,7 +85,7 @@
   <template data-id="searchResult">
     <div class="searchResult">
       <img class="searchResult-screenshot" />
-      <a class="searchResult-title" target="_blank"></a>
+      <h3 class="searchResult-title"></h3>
       <div class="searchResult-description-group">
         <div class="searchResult-description"></div>
         <div class="searchResult-description-info">
@@ -153,33 +153,35 @@
         </label>
       </div>
     </div>
+  </div>
+  <div class="actions">
     <div id="searchResults" class="hidden">
-      <h3>
+      <h2>
         <span i18n-text="searchResultsHeader"></span>
         <span id="searchResults-terms">...</span>
-      </h3>
+      </h2>
       <div id="searchResults-list"></div>
       <div id="searchResultsNav">
-        <button id="searchResultsNav-prev" title="Previous page" disabled>&#8592;</button>
+        <button id="searchResultsNav-prev" i18n-title="paginationPrevious" disabled>&#8592;</button>
         <label>
-          <span id="searchResultsNav-currentPage" title="Current page">-</span>
+          <span id="searchResultsNav-currentPage" i18n-title="paginationCurrent">-</span>
           /
-          <span id="searchResultsNav-totalPages" title="Total pages of search results">-</span>
+          <span id="searchResultsNav-totalPages" i18n-title="paginationTotal">-</span>
         </label>
-        <button id="searchResultsNav-next" title="Next page" disabled>&#8594;</button>
+        <button id="searchResultsNav-next" i18n-title="paginationNext" disabled>&#8594;</button>
       </div>
     </div>
     <div class="left-gutter"></div>
     <div class="main-controls">
       <div id="find-styles">
-        <a id="find-styles-link" i18n-text="findStylesForSite"></a>
+        <a id="find-styles-link" i18n-text="findStyles" i18n-title="findStylesTooltip"></a>
         <span id="find-styles-inline-group">
-          <label>
+          <label i18n-title="findStylesInlineTooltip">
             <input id="find-styles-inline" class="checker" type="checkbox" checked />
             <svg class="svg-icon checked" viewBox="0 0 1000 1000">
               <path fill-rule="evenodd" d="M983.2,184.3L853,69.8c-4-3.5-9.3-5.3-14.5-5c-5.3,0.4-10.3,2.8-13.8,6.8L352.3,609.2L184.4,386.9c-3.2-4.2-8-7-13.2-7.8c-5.3-0.8-10.6,0.6-14.9,3.9L18,487.5c-8.8,6.7-10.6,19.3-3.9,28.1L325,927.2c3.6,4.8,9.3,7.7,15.3,8c0.2,0,0.5,0,0.7,0c5.8,0,11.3-2.5,15.1-6.8L985,212.6C992.3,204.3,991.5,191.6,983.2,184.3z"/>
             </svg>
-            <span i18n-text="findStylesForSiteInline"></span>
+            <span i18n-text="findStylesInline"></span>
           </label>
         </span>
       </div>
diff --git a/popup/search-results.css b/popup/search-results.css
index 84a011ba..da109def 100755
--- a/popup/search-results.css
+++ b/popup/search-results.css
@@ -39,8 +39,8 @@ body.search-results-shown {
 }
 
 .searchResult-title {
-  display: block;
-  font-weight: bold;
+  padding: 3px;
+  margin: 0;
 }
 
 .searchResult-description {
@@ -53,7 +53,7 @@ body.search-results-shown {
 
 .searchResult-description.expanded {
   width: 100%;
-  white-space: normal;
+  white-space: pre-wrap;
   overflow: auto;
   text-overflow: unset;
   display: block;
diff --git a/popup/search-results.js b/popup/search-results.js
index 1bd34229..e4618b74 100755
--- a/popup/search-results.js
+++ b/popup/search-results.js
@@ -40,6 +40,7 @@
     const processedResults = []; // Search results that are not installed and apply ot the page (includes 'json' field with full style).
     const BLANK_PIXEL_DATA = 'data:image/gif;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAA' +
                              'C1HAwCAAAAC0lEQVR42mOcXQ8AAbsBHLLDr5MAAAAASUVORK5CYII=';
+    let loading = false;
     let category; // Category for the active tab's URL.
     let currentDisplayedPage = 1; // Current page number in popup.html
 
@@ -53,25 +54,24 @@
     }
 
     /**
-     * Adds spinner to node.
-     * @param {DOMNode} parentNode The parent node to append/remove the spinner to/from.
-     * @param {Boolean} isLoading If the element is loading or stopped loading.
+     * Sets loading status of search results.
+     * @param {Boolean} isLoading If search results are idle (false) or still loading (true).
      */
-    function setLoading(parentNode, isLoading) {
-      if (isLoading) {
-        if ($('.lds-spinner', parentNode) === null) {
-          parentNode.appendChild(
+    function setLoading(isLoading) {
+      if (loading !== isLoading) {
+        loading = isLoading;
+        if (loading) {
+          $('#searchResults').appendChild(
             $create(
               '.lds-spinner',
               new Array(12).fill($create('div')).map(e => e.cloneNode()))
           );
+        } else {
+          $.remove('#searchResults > .lds-spinner');
         }
-      } else {
-        $.remove('.lds-spinner', parentNode);
       }
     }
 
-
     function render() {
       $('#searchResults-list').textContent = ''; // Clear search results
 
@@ -104,9 +104,10 @@
 
     function loadMoreIfNeeded() {
       if (shouldLoadMore()) {
+        setLoading(true);
         setTimeout(load, DELAY_BEFORE_SEARCHING_STYLES);
       } else {
-        setLoading($('#searchResults'), false);
+        setLoading(false);
       }
     }
 
@@ -121,8 +122,8 @@
     /** Decrements currentPage and loads results. */
     function prev() {
       currentDisplayedPage = Math.max(1, currentDisplayedPage - 1);
-      render();
       window.scrollTo(0, 0);
+      render();
     }
 
     /**
@@ -151,9 +152,9 @@
         processNextResult();
       } else if (searchAPI.isExhausted()) {
         // Stop if no more search results.
-        setLoading($('#searchResults'), false);
+        setLoading(false);
       } else {
-        setLoading($('#searchResults'), true);
+        setLoading(true);
         // Search for more results.
         $('#searchResults').classList.remove('hidden');
         $('#searchResults-error').classList.add('hidden');
@@ -183,7 +184,7 @@
      */
     function processNextResult() {
       if (!shouldLoadMore()) {
-        setLoading($('#searchResults'), false);
+        setLoading(false);
         return;
       }
 
@@ -284,17 +285,16 @@
 
       const entry = template.searchResult.cloneNode(true);
       Object.assign(entry, {
-        id: 'searchResult-' + userstyleSearchResult.id
+        id: 'searchResult-' + userstyleSearchResult.id,
+        onclick: handleEvent.openURLandHide
       });
+      entry.dataset.href = searchAPI.BASE_URL + userstyleSearchResult.url;
       $('#searchResults-list').appendChild(entry);
 
       const searchResultName = userstyleSearchResult.name;
       const title = $('.searchResult-title', entry);
       Object.assign(title, {
-        textContent: searchResultName,
-        title: searchResultName,
-        href: searchAPI.BASE_URL + userstyleSearchResult.url,
-        onclick: handleEvent.openURLandHide
+        textContent: searchResultName
       });
 
       const screenshot = $('.searchResult-screenshot', entry);
@@ -312,14 +312,14 @@
 
       const description = $('.searchResult-description', entry);
       Object.assign(description, {
-        textContent: userstyleSearchResult.description.replace(/<.*?>/g, ''),
+        textContent: userstyleSearchResult.description.replace(/<.*?>/g, '').replace(/(\r\n?)\r\n?/g, '$1')
       });
       const descriptionExpand = $('.searchResult-description-info', entry);
       Object.assign(descriptionExpand, {
         onclick: e => {
+          e.stopPropagation();
           descriptionExpand.classList.add('hidden');
           description.classList.add('expanded');
-          description.innerHTML = userstyleSearchResult.description.replace(/<br\/?>\s*<br\/?>/, '<br/>');
         }
       });
 
@@ -370,14 +370,14 @@
       }
 
       /** Installs the current userstyleSearchResult into stylus. */
-      function install() {
+      function install(event) {
+        if (event) {
+          event.stopPropagation();
+        }
         const styleId = userstyleSearchResult.id;
         const url = searchAPI.BASE_URL + '/styles/chrome/' + styleId + '.json';
-        setLoading(entry, true);
         saveStyleSafe(userstyleSearchResult.json)
           .then(() => {
-            setLoading(entry, false);
-
             // Remove search result after installing
             let matchingIndex = -1;
             processedResults.forEach((processedResult, index) => {
@@ -394,7 +394,6 @@
             processNextResult();
           })
           .catch(reason => {
-            setLoading(entry, false);
             console.log('install:saveStyleSafe(', url, ') => [ERROR]: ', reason);
             alert('Error while downloading ' + url + '\nReason: ' + reason);
           });