From 99a85ff4ef859c0df78bbce495e639e6272b457a Mon Sep 17 00:00:00 2001 From: Kevin Heis Date: Mon, 25 Jan 2021 11:52:55 -0800 Subject: [PATCH] Push query string when searching (#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 --- javascripts/search.js | 102 +++++++++++++++++++++++++-------------- package.json | 1 - script/check-deps.js | 1 - tests/browser/browser.js | 21 ++++---- 4 files changed, 75 insertions(+), 50 deletions(-) diff --git a/javascripts/search.js b/javascripts/search.js index fb9c10e7de..f522fcb1c6 100644 --- 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 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 panel if input is clicked + $searchInput.addEventListener('focus', openSearch) - // Open modal if input is clicked - $input.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 +// When the user finishes typing, update the results +async function onSearch () { + const query = $searchInput.value - const url = new URL(location.origin) - url.pathname = '/search' - url.search = new URLSearchParams({ query, version, language }).toString() + // 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) - const response = await fetch(url, { - method: 'GET', - headers: { - 'Content-Type': 'application/json' - } - }) - const results = response.ok ? await response.json() : [] + // 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 index 59e14c3b3b..3212d69bc1 100644 --- 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 index c5da72b5dc..d38b8b1b1c 100644 --- 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 index cde07dfaed..aa4e15ba19 100644 --- 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() })