Push query string when searching (github#17417)

* Push query string when searching * Update search.js * Fix browser test, remove querystring dependency (new shiny!) * Remove language and version from visible URL * Avoid casting event interface * Update search.js * Update browser.js
dotam99 · Jan 25, 2021 · 99a85ff · 99a85ff
1 parent 7dd6c93
commit 99a85ff
Show file tree

Hide file tree

Showing 4 changed files with 76 additions and 51 deletions.
diff --git a/javascripts/search.js b/javascripts/search.js
@@ -14,16 +14,18 @@ let $searchResultsContainer
 let $searchOverlay
 let $searchInput
 
+// This is our default placeholder, but it can be localized with a <meta> tag
 let placeholder = 'Search topics, products...'
 let version
 let language
 
 export default function search () {
+  // First, only initialize search if the elements are on the page
   $searchInputContainer = document.getElementById('search-input-container')
   $searchResultsContainer = document.getElementById('search-results-container')
-
   if (!$searchInputContainer || !$searchResultsContainer) return
 
+  // This overlay exists so if you click off the search, it closes
   $searchOverlay = document.querySelector('.search-overlay-desktop')
 
   // There's an index for every version/language combination
@@ -36,15 +38,25 @@ export default function search () {
     placeholder = $placeholderMeta.content
   }
 
+  // Write the search form into its container
   $searchInputContainer.append(tmplSearchInput())
   $searchInput = $searchInputContainer.querySelector('input')
 
-  searchWithYourKeyboard('#search-input-container input', '.ais-Hits-item')
-  toggleSearchDisplay()
-
+  // Prevent 'enter' from refreshing the page
   $searchInputContainer.querySelector('form')
     .addEventListener('submit', evt => evt.preventDefault())
+
+  // Search when the user finished typing
   $searchInput.addEventListener('keyup', debounce(onSearch))
+
+  // Adds ability to navigate search results with keyboard (up, down, enter, esc)
+  searchWithYourKeyboard('#search-input-container input', '.ais-Hits-item')
+
+  // If the user already has a query in the URL, parse it and search away
+  parseExistingSearch()
+
+  // If not on home page, decide if search panel should be open
+  toggleSearchDisplay() // must come after parseExistingSearch
 }
 
 // The home page and 404 pages have a standalone search
@@ -64,43 +76,37 @@ function toggleSearchDisplay () {
   // If not on homepage...
   if (hasStandaloneSearch()) return
 
-  const $input = $searchInput
-
-  // Open modal if input is clicked
-  $input.addEventListener('focus', () => {
-    openSearch()
-  })
+  // Open panel if input is clicked
+  $searchInput.addEventListener('focus', openSearch)
 
-  // Close modal if overlay is clicked
+  // Close panel if overlay is clicked
   if ($searchOverlay) {
-    $searchOverlay.addEventListener('click', () => {
-      closeSearch()
-    })
+    $searchOverlay.addEventListener('click', closeSearch)
   }
 
-  // Open modal if page loads with query in the params/input
-  if ($input.value) {
+  // Open panel if page loads with query in the params/input
+  if ($searchInput.value) {
     openSearch()
   }
 }
 
+// On most pages, opens the search panel
 function openSearch () {
   $searchInput.classList.add('js-open')
   $searchResultsContainer.classList.add('js-open')
   $searchOverlay.classList.add('js-open')
 }
 
+// Close panel if not on homepage
 function closeSearch () {
-  // Close modal if not on homepage
   if (!hasStandaloneSearch()) {
     $searchInput.classList.remove('js-open')
     $searchResultsContainer.classList.remove('js-open')
     $searchOverlay.classList.remove('js-open')
   }
 
-  const $hits = $searchResultsContainer.querySelector('.ais-Hits')
-  if ($hits) $hits.style.display = 'none'
   $searchInput.value = ''
+  onSearch()
 }
 
 function deriveLanguageCodeFromPath () {
@@ -122,6 +128,7 @@ function deriveVersionFromPath () {
     : versionObject.miscBaseName
 }
 
+// Wait for the event to stop triggering for X milliseconds before responding
 function debounce (fn, delay = 300) {
   let timer
   return (...args) => {
@@ -130,34 +137,47 @@ function debounce (fn, delay = 300) {
   }
 }
 
-async function onSearch (evt) {
-  const query = evt.target.value
-
-  const url = new URL(location.origin)
-  url.pathname = '/search'
-  url.search = new URLSearchParams({ query, version, language }).toString()
+// When the user finishes typing, update the results
+async function onSearch () {
+  const query = $searchInput.value
 
-  const response = await fetch(url, {
-    method: 'GET',
-    headers: {
-      'Content-Type': 'application/json'
-    }
-  })
-  const results = response.ok ? await response.json() : []
+  // Update the URL with the search parameters in the query string
+  const pushUrl = new URL(location)
+  pushUrl.search = query ? new URLSearchParams({ query }) : ''
+  history.pushState({}, '', pushUrl)
+
+  // If there's a query, call the endpoint
+  // Otherwise, there's no results by default
+  let results = []
+  if (query.trim()) {
+    const endpointUrl = new URL(location.origin)
+    endpointUrl.pathname = '/search'
+    endpointUrl.search = new URLSearchParams({ language, version, query })
+
+    const response = await fetch(endpointUrl, {
+      method: 'GET',
+      headers: {
+        'Content-Type': 'application/json'
+      }
+    })
+    results = response.ok ? await response.json() : []
+  }
 
+  // Either way, update the display
   $searchResultsContainer.querySelectorAll('*').forEach(el => el.remove())
   $searchResultsContainer.append(
     tmplSearchResults(results)
   )
-
   toggleStandaloneSearch()
 
   // Analytics tracking
-  sendEvent({
-    type: 'search',
-    search_query: query
-    // search_context
-  })
+  if (query.trim()) {
+    sendEvent({
+      type: 'search',
+      search_query: query
+      // search_context
+    })
+  }
 }
 
 // If on homepage, toggle results container if query is present
@@ -189,6 +209,14 @@ function toggleStandaloneSearch () {
   if (queryPresent && $results) $results.style.display = 'block'
 }
 
+// If the user shows up with a query in the URL, go ahead and search for it
+function parseExistingSearch () {
+  const params = new URLSearchParams(location.search)
+  if (!params.has('query')) return
+  $searchInput.value = params.get('query')
+  onSearch()
+}
+
 /** * Template functions ***/
 
 function tmplSearchInput () {

diff --git a/package.json b/package.json
@@ -72,7 +72,6 @@
     "parse5": "^6.0.1",
     "platform-utils": "^1.2.0",
     "port-used": "^2.0.8",
-    "querystring": "^0.2.0",
     "rate-limit-redis": "^2.0.0",
     "react": "^17.0.1",
     "react-dom": "^17.0.1",

diff --git a/script/check-deps.js b/script/check-deps.js
@@ -32,7 +32,6 @@ const main = async () => {
       '@babel/*',
       'babel-preset-env',
       '@primer/*',
-      'querystring',
       'pa11y-ci',
       'sass',
       'babel-loader',

diff --git a/tests/browser/browser.js b/tests/browser/browser.js
@@ -1,6 +1,5 @@
 /* global page, browser */
 const sleep = require('await-sleep')
-const querystring = require('querystring')
 const { latest } = require('../../lib/enterprise-server-releases')
 
 describe('homepage', () => {
@@ -12,7 +11,7 @@ describe('homepage', () => {
   })
 })
 
-describe('algolia browser search', () => {
+describe('browser search', () => {
   jest.setTimeout(60 * 1000)
 
   it('works on the homepage', async () => {
@@ -42,18 +41,18 @@ describe('algolia browser search', () => {
     expect(hits.length).toBeGreaterThan(5)
   })
 
-  it('sends the correct data to algolia for Enterprise Server', async () => {
+  it('sends the correct data to search for Enterprise Server', async () => {
     expect.assertions(2)
 
     const newPage = await browser.newPage()
-    await newPage.goto('http://localhost:4001/ja/enterprise/2.22/admin/installation')
+    await newPage.goto('http://localhost:4001/ja/enterprise-server@2.22/admin/installation')
 
     await newPage.setRequestInterception(true)
     newPage.on('request', interceptedRequest => {
       if (interceptedRequest.method() === 'GET' && /search/i.test(interceptedRequest.url())) {
-        const { version, language } = querystring.parse(interceptedRequest.url())
-        expect(version).toBe('2.22')
-        expect(language).toBe('ja')
+        const { searchParams } = new URL(interceptedRequest.url())
+        expect(searchParams.get('version')).toBe('2.22')
+        expect(searchParams.get('language')).toBe('ja')
       }
       interceptedRequest.continue()
     })
@@ -63,7 +62,7 @@ describe('algolia browser search', () => {
     await newPage.waitForSelector('.search-result')
   })
 
-  it('sends the correct data to algolia for GHAE', async () => {
+  it('sends the correct data to search for GHAE', async () => {
     expect.assertions(2)
 
     const newPage = await browser.newPage()
@@ -72,9 +71,9 @@ describe('algolia browser search', () => {
     await newPage.setRequestInterception(true)
     newPage.on('request', interceptedRequest => {
       if (interceptedRequest.method() === 'GET' && /search/i.test(interceptedRequest.url())) {
-        const { version, language } = querystring.parse(interceptedRequest.url())
-        expect(version).toBe('ghae')
-        expect(language).toBe('en')
+        const { searchParams } = new URL(interceptedRequest.url())
+        expect(searchParams.get('version')).toBe('ghae')
+        expect(searchParams.get('language')).toBe('en')
       }
       interceptedRequest.continue()
     })