From 04d1e8162bc815e3bda1c18dc47f7ee8a9ef1718 Mon Sep 17 00:00:00 2001 From: LB Johnston Date: Sat, 21 Jan 2023 07:26:26 +1000 Subject: [PATCH] Migrate header search to a Stimulus controller (w-swap) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Removes the jQuery slide animation so content will be instantly replaced - Removes the autofocus behaviour on the search fields as this is not helpful for screen readers / keyboard control - Includes support for `window.header` if provided alongside dynamic adding of data-* attributes if not included - Base implementation for #9950 - Co-authored-by: sag᠎e --- CHANGELOG.txt | 2 + client/src/controllers/SwapController.test.js | 614 ++++++++++++++++++ client/src/controllers/SwapController.ts | 302 +++++++++ client/src/controllers/index.ts | 2 + client/src/entrypoints/admin/bulk-actions.js | 11 +- client/src/entrypoints/admin/core.js | 57 -- client/src/entrypoints/admin/wagtailadmin.js | 8 + docs/releases/5.1.md | 2 + 8 files changed, 931 insertions(+), 67 deletions(-) create mode 100644 client/src/controllers/SwapController.test.js create mode 100644 client/src/controllers/SwapController.ts diff --git a/CHANGELOG.txt b/CHANGELOG.txt index 5c31c88c10..3a5083f216 100644 --- a/CHANGELOG.txt +++ b/CHANGELOG.txt @@ -37,6 +37,7 @@ Changelog * Fix: Ensure that gettext_lazy works correctly when using verbose_name on a generic Settings models (Sébastien Corbin) * Fix: Remove unnecessary usage of `innerHTML` when modifying DOM content (LB (Ben) Johnston) * Fix: Avoid `ValueError` when extending `PagesAPIViewSet` and setting `meta_fields` to an empty list (Henry Harutyunyan, Alex Morega) + * Fix: Improve accessibility for header search, remove autofocus on page load, advise screen readers that content has changed when results update (LB (Ben) Johnston) * Docs: Document how to add non-ModelAdmin views to a `ModelAdminGroup` (Onno Timmerman) * Docs: Document how to add StructBlock data to a StreamField (Ramon Wenger) * Docs: Update ReadTheDocs settings to v2 to resolve urllib3 issue in linkcheck extension (Thibaud Colas) @@ -70,6 +71,7 @@ Changelog * Maintenance: Convert the CONTRIBUTORS file to Markdown (Dan Braghis) * Maintenance: Move `django-filter` version upper bound to v24 (Yuekui) * Maintenance: Update Pillow dependency to allow 10.x, only include support for >= 9.1.0 (Yuekui) + * Maintenance: Migrate header search behaviour to `w-swap`, a Stimulus controller (LB (Ben) Johnston) 5.0.2 (21.06.2023) diff --git a/client/src/controllers/SwapController.test.js b/client/src/controllers/SwapController.test.js new file mode 100644 index 0000000000..9d72a06a5f --- /dev/null +++ b/client/src/controllers/SwapController.test.js @@ -0,0 +1,614 @@ +import { Application } from '@hotwired/stimulus'; +import { SwapController } from './SwapController'; +import { range } from '../utils/range'; + +jest.useFakeTimers(); + +jest.spyOn(console, 'error').mockImplementation(() => {}); + +const flushPromises = () => new Promise(setImmediate); + +describe('SwapController', () => { + let application; + let handleError; + + const getMockResults = ( + { attrs = ['id="new-results"'], total = 3 } = {}, + arr = range(0, total), + ) => { + const items = arr.map((_) => `
  • RESULT ${_}
    • `).join(''); + return ``; + }; + + beforeEach(() => { + application = Application.start(); + application.register('w-swap', SwapController); + handleError = jest.fn(); + application.handleError = handleError; + }); + + afterEach(() => { + application.stop(); + document.body.innerHTML = '
    '; + jest.clearAllMocks(); + + if (window.headerSearch) { + delete window.headerSearch; + } + }); + + describe('when results element & src URL value is not available', () => { + it('should throw an error if no valid selector can be resolved', async () => { + expect(handleError).not.toHaveBeenCalled(); + + document.body.innerHTML = ` +
    + `; + + // trigger next browser render cycle + await Promise.resolve(); + + expect(handleError).toHaveBeenCalledWith( + expect.objectContaining({ message: "'' is not a valid selector" }), + 'Error connecting controller', + expect.objectContaining({ identifier: 'w-swap' }), + ); + }); + + it('should throw an error if target element selector cannot resolve a DOM element', async () => { + expect(handleError).not.toHaveBeenCalled(); + + document.body.innerHTML = ` +
    + `; + + // trigger next browser render cycle + await Promise.resolve(); + + expect(handleError).toHaveBeenCalledWith( + expect.objectContaining({ + message: 'Cannot find valid target element at "#resultX"', + }), + 'Error connecting controller', + expect.objectContaining({ identifier: 'w-swap' }), + ); + }); + + it('should throw an error if no valid src URL can be resolved', async () => { + expect(handleError).not.toHaveBeenCalled(); + + document.body.innerHTML = ` +
    + `; + + // trigger next browser render cycle + await Promise.resolve(); + + expect(handleError).toHaveBeenCalledWith( + expect.objectContaining({ message: 'Cannot find valid src URL value' }), + 'Error connecting controller', + expect.objectContaining({ identifier: 'w-swap' }), + ); + }); + }); + + describe('fallback on window.headerSearch values if not in HTML', () => { + it('should set the src & target value from the window.headerSearch if not present', async () => { + window.headerSearch = { + termInput: '#search', + url: 'path/to/page/search', + targetOutput: '#page-results', + }; + + document.body.innerHTML = ` +
    +
    + `; + + SwapController.afterLoad('w-swap'); + + // trigger next browser render cycle + await Promise.resolve(); + + // should not error + expect(handleError).not.toHaveBeenCalled(); + + expect({ ...document.getElementById('form').dataset }).toEqual({ + action: 'change->w-swap#searchLazy input->w-swap#searchLazy', + controller: 'w-swap', + wSwapSrcValue: 'path/to/page/search', // set from window.headerSearch + wSwapTargetValue: '#page-results', // set from window.headerSearch + }); + + expect({ ...document.getElementById('search').dataset }).toEqual({ + wSwapTarget: 'input', + }); + }); + }); + + describe('performing a location update via actions on a controlled input', () => { + beforeEach(() => { + document.body.innerHTML = ` + +
    + `; + + window.history.replaceState(null, '', '?'); + }); + + it('should not do a location based update if the URL query and the input query are equal', () => { + const input = document.getElementById('search'); + + // when values are empty + input.dispatchEvent(new CustomEvent('keyup', { bubbles: true })); + jest.runAllTimers(); // update is debounced + expect(handleError).not.toHaveBeenCalled(); + expect(global.fetch).not.toHaveBeenCalled(); + + // when input value only has whitespace + input.value = ' '; + input.dispatchEvent(new CustomEvent('keyup', { bubbles: true })); + jest.runAllTimers(); // update is debounced + expect(handleError).not.toHaveBeenCalled(); + expect(global.fetch).not.toHaveBeenCalled(); + + // when input value and URL query only have whitespace + window.history.replaceState(null, '', '?q=%20%20&p=foo'); // 2 spaces + input.value = ' '; // 4 spaces + input.dispatchEvent(new CustomEvent('keyup', { bubbles: true })); + jest.runAllTimers(); // update is debounced + expect(handleError).not.toHaveBeenCalled(); + expect(global.fetch).not.toHaveBeenCalled(); + + // when input value and URL query have the same value + window.history.replaceState(null, '', '?q=espresso'); + input.value = 'espresso'; + input.dispatchEvent(new CustomEvent('keyup', { bubbles: true })); + jest.runAllTimers(); // update is debounced + expect(handleError).not.toHaveBeenCalled(); + expect(global.fetch).not.toHaveBeenCalled(); + + // when input value and URL query have the same value (ignoring whitespace) + window.history.replaceState(null, '', '?q=%20espresso%20'); + input.value = ' espresso '; + input.dispatchEvent(new CustomEvent('keyup', { bubbles: true })); + jest.runAllTimers(); // update is debounced + expect(handleError).not.toHaveBeenCalled(); + expect(global.fetch).not.toHaveBeenCalled(); + }); + + it('should allow for updating via a declared action on input changes', async () => { + const input = document.getElementById('search'); + const icon = document.querySelector('.icon-search use'); + const targetElement = document.getElementById('results'); + + const results = getMockResults(); + + const onSuccess = new Promise((resolve) => { + document.addEventListener('w-swap:success', resolve); + }); + + fetch.mockImplementationOnce(() => + Promise.resolve({ + ok: true, + status: 200, + text: () => Promise.resolve(results), + }), + ); + + expect(window.location.search).toEqual(''); + expect(handleError).not.toHaveBeenCalled(); + expect(global.fetch).not.toHaveBeenCalled(); + expect(icon.getAttribute('href')).toEqual('#icon-search'); + expect(targetElement.getAttribute('aria-busy')).toBeNull(); + + input.value = 'alpha'; + input.dispatchEvent(new CustomEvent('keyup', { bubbles: true })); + + jest.runAllTimers(); // update is debounced + + // visual loading state should be active & content busy + await Promise.resolve(); // trigger next rendering + expect(targetElement.getAttribute('aria-busy')).toEqual('true'); + expect(icon.getAttribute('href')).toEqual('#icon-spinner'); + + expect(handleError).not.toHaveBeenCalled(); + expect(global.fetch).toHaveBeenCalledWith( + '/admin/images/results/?q=alpha', + expect.any(Object), + ); + + const successEvent = await onSuccess; + + // should dispatch success event + expect(successEvent.detail).toEqual({ + requestUrl: '/admin/images/results/?q=alpha', + results, + }); + + // should update HTML + expect(targetElement.querySelectorAll('li')).toHaveLength(3); + + await flushPromises(); + + // should update the current URL + expect(window.location.search).toEqual('?q=alpha'); + + // should reset the icon & busy state + expect(icon.getAttribute('href')).toEqual('#icon-search'); + expect(targetElement.getAttribute('aria-busy')).toBeNull(); + }); + + it('should correctly clear any params based on the action param value', async () => { + const MOCK_SEARCH = '?k=keep&q=alpha&r=remove-me&s=stay&x=exclude-me'; + window.history.replaceState(null, '', MOCK_SEARCH); + const input = document.getElementById('search'); + + // update clear param - check we can handle space separated values + input.setAttribute('data-w-swap-clear-param', 'r x'); + + fetch.mockImplementationOnce(() => + Promise.resolve({ + ok: true, + status: 200, + text: () => Promise.resolve(getMockResults()), + }), + ); + + expect(window.location.search).toEqual(MOCK_SEARCH); + + input.value = 'beta'; + input.dispatchEvent(new CustomEvent('keyup', { bubbles: true })); + + // run all timers & promises + await flushPromises(jest.runAllTimers()); + + // should update the current URL + expect(window.location.search).toEqual('?k=keep&q=beta&s=stay'); + }); + + it('should handle both clearing values in the URL and using a custom query param from input', async () => { + const MOCK_SEARCH = '?k=keep&query=alpha&r=remove-me'; + window.history.replaceState(null, '', MOCK_SEARCH); + const input = document.getElementById('search'); + input.setAttribute('name', 'query'); + + // update clear param value to a single (non-default) value + input.setAttribute('data-w-swap-clear-param', 'r'); + + fetch.mockImplementation(() => + Promise.resolve({ + ok: true, + status: 200, + text: () => Promise.resolve(getMockResults()), + }), + ); + + expect(window.location.search).toEqual(MOCK_SEARCH); + + input.value = 'a new search string!'; + input.dispatchEvent(new CustomEvent('keyup', { bubbles: true })); + + // run all timers & promises + await flushPromises(jest.runAllTimers()); + + // should update the current URL, removing any cleared params + expect(window.location.search).toEqual( + '?k=keep&query=a+new+search+string%21', + ); + + // should clear the location params if the input is updated to an empty value + input.value = ''; + input.dispatchEvent(new CustomEvent('keyup', { bubbles: true })); + + // run all timers & promises + await flushPromises(jest.runAllTimers()); + + // should update the current URL, removing the query param + expect(window.location.search).toEqual('?k=keep'); + }); + + it('should handle repeated input and correctly resolve the requested data', async () => { + window.history.replaceState(null, '', '?q=first&p=3'); + + const input = document.getElementById('search'); + + const onSuccess = new Promise((resolve) => { + document.addEventListener('w-swap:success', resolve); + }); + + const delays = [200, 20, 400]; // emulate changing results timings + + fetch.mockImplementation( + (query) => + new Promise((resolve) => { + const delay = delays.pop(); + setTimeout(() => { + resolve({ + ok: true, + status: 200, + text: () => + Promise.resolve( + getMockResults({ + attrs: [ + 'id="new-results"', + `data-query="${query}"`, + `data-delay="${delay}"`, + ], + }), + ), + }); + }, delay); + }), + ); + + expect(handleError).not.toHaveBeenCalled(); + expect(global.fetch).not.toHaveBeenCalled(); + + input.value = 'alpha'; + input.dispatchEvent(new CustomEvent('keyup', { bubbles: true })); + + setTimeout(() => { + input.value = 'beta'; + input.dispatchEvent(new CustomEvent('keyup', { bubbles: true })); + }, 210); + + setTimeout(() => { + input.value = 'delta'; + input.dispatchEvent(new CustomEvent('keyup', { bubbles: true })); + }, 420); + + jest.runAllTimers(); // update is debounced + + expect(handleError).not.toHaveBeenCalled(); + expect(global.fetch).toHaveBeenCalledTimes(3); + expect(global.fetch).toHaveBeenLastCalledWith( + '/admin/images/results/?q=delta', + expect.any(Object), + ); + + const successEvent = await onSuccess; + + // should dispatch success event + expect(successEvent.detail).toEqual({ + requestUrl: '/admin/images/results/?q=beta', + results: expect.any(String), + }); + + // should update HTML + const resultsElement = document.getElementById('results'); + expect(resultsElement.querySelectorAll('li')).toHaveLength(3); + expect( + resultsElement.querySelector('[data-query]').dataset.query, + ).toEqual('/admin/images/results/?q=delta'); + + await flushPromises(); + + // should update the current URL & clear the page param + expect(window.location.search).toEqual('?q=delta'); + }); + + it('should handle search results API failures gracefully', async () => { + const icon = document.querySelector('.icon-search use'); + const input = document.getElementById('search'); + + const onErrorEvent = jest.fn(); + document.addEventListener('w-swap:error', onErrorEvent); + + fetch.mockImplementationOnce(() => + Promise.resolve({ + ok: false, + status: 500, + }), + ); + + expect(window.location.search).toEqual(''); + expect(handleError).not.toHaveBeenCalled(); + expect(global.fetch).not.toHaveBeenCalled(); + + input.value = 'alpha'; + input.dispatchEvent(new CustomEvent('keyup', { bubbles: true })); + + jest.runAllTimers(); // update is debounced + + expect(handleError).not.toHaveBeenCalled(); + expect(global.fetch).toHaveBeenCalledWith( + '/admin/images/results/?q=alpha', + expect.any(Object), + ); + + expect(onErrorEvent).not.toHaveBeenCalled(); + + await Promise.resolve(); // trigger next rendering + expect(icon.getAttribute('href')).toEqual('#icon-spinner'); + + await flushPromises(); // resolve all promises + + // eslint-disable-next-line no-console + expect(console.error).toHaveBeenLastCalledWith( + 'Error fetching /admin/images/results/?q=alpha', + expect.any(Error), + ); + + // should not update any HTML + expect(document.getElementById('results').innerHTML).toEqual(''); + + // should have dispatched a custom event for the error + expect(onErrorEvent).toHaveBeenCalledTimes(1); + expect(onErrorEvent.mock.calls[0][0].detail).toEqual({ + error: expect.any(Error), + requestUrl: '/admin/images/results/?q=alpha', + }); + + await Promise.resolve(); // trigger next rendering + + // should reset the icon + expect(icon.getAttribute('href')).toEqual('#icon-search'); + }); + }); + + describe('performing a location update via actions on a controlled form', () => { + beforeEach(() => { + document.body.innerHTML = ` + +
    + `; + + window.history.replaceState(null, '', '?'); + }); + + it('should allow for searching via a declared action on input changes', async () => { + const input = document.getElementById('search'); + const icon = document.querySelector('.icon-search use'); + + const results = getMockResults(); + + const onBegin = jest.fn(); + document.addEventListener('w-swap:begin', onBegin); + + const onSuccess = new Promise((resolve) => { + document.addEventListener('w-swap:success', resolve); + }); + + fetch.mockImplementationOnce(() => + Promise.resolve({ + ok: true, + status: 200, + text: () => Promise.resolve(results), + }), + ); + + expect(window.location.search).toEqual(''); + expect(handleError).not.toHaveBeenCalled(); + expect(global.fetch).not.toHaveBeenCalled(); + expect(icon.getAttribute('href')).toEqual('#icon-search'); + + input.value = 'alpha'; + input.dispatchEvent(new CustomEvent('input', { bubbles: true })); + + jest.runAllTimers(); // update is debounced + + expect(onBegin).toHaveBeenCalledTimes(1); + + // visual loading state should be active + await Promise.resolve(); // trigger next rendering + expect(icon.getAttribute('href')).toEqual('#icon-spinner'); + + expect(handleError).not.toHaveBeenCalled(); + expect(global.fetch).toHaveBeenCalledWith( + '/path/to/form/action/?q=alpha', + expect.any(Object), + ); + + const successEvent = await onSuccess; + + // should dispatch success event + expect(successEvent.detail).toEqual({ + requestUrl: '/path/to/form/action/?q=alpha', + results, + }); + + // should update HTML + expect( + document.getElementById('other-results').querySelectorAll('li'), + ).toHaveLength(3); + + await flushPromises(); + + // should update the current URL + expect(window.location.search).toEqual('?q=alpha'); + + // should reset the icon + expect(icon.getAttribute('href')).toEqual('#icon-search'); + + expect(onBegin).toHaveBeenCalledTimes(1); + }); + + it('should allow for blocking the request with custom events', async () => { + const input = document.getElementById('search'); + + const results = getMockResults({ total: 5 }); + + const beginEventHandler = jest.fn((event) => { + event.preventDefault(); + }); + + document.addEventListener('w-swap:begin', beginEventHandler); + + fetch.mockImplementationOnce(() => + Promise.resolve({ + ok: true, + status: 200, + text: () => Promise.resolve(results), + }), + ); + + expect(window.location.search).toEqual(''); + expect(handleError).not.toHaveBeenCalled(); + expect(global.fetch).not.toHaveBeenCalled(); + + input.value = 'alpha'; + input.dispatchEvent(new CustomEvent('input', { bubbles: true })); + + expect(beginEventHandler).not.toHaveBeenCalled(); + + jest.runAllTimers(); // search is debounced + await Promise.resolve(requestAnimationFrame); + + // should fire a begin event before the request is made + expect(beginEventHandler).toHaveBeenCalledTimes(1); + expect(beginEventHandler.mock.calls[0][0].detail).toEqual({ + requestUrl: '/path/to/form/action/?q=alpha', + }); + + expect(global.fetch).not.toHaveBeenCalled(); + + document.removeEventListener('w-swap:begin', beginEventHandler); + }); + }); +}); diff --git a/client/src/controllers/SwapController.ts b/client/src/controllers/SwapController.ts new file mode 100644 index 0000000000..4bac441e5c --- /dev/null +++ b/client/src/controllers/SwapController.ts @@ -0,0 +1,302 @@ +import { Controller } from '@hotwired/stimulus'; +import { debounce } from '../utils/debounce'; +import { domReady } from '../utils/domReady'; + +declare global { + interface Window { + headerSearch?: { targetOutput: string; url: string }; + } +} + +/** + * Support legacy window global approach until header search + * can fully adopt data-attributes. + * + */ +const getGlobalHeaderSearchOptions = (): { + targetOutput?: string; + termInput?: string; + url?: string; +} => window.headerSearch || {}; + +/** + * Allow for an element to trigger an async query that will + * patch the results into a results DOM container. The query + * input can be the controlled element or the containing form. + * It supports the ability to update the URL with the query + * when processed. + * + * @example + *
    + * + */ +export class SwapController extends Controller< + HTMLFormElement | HTMLInputElement +> { + static defaultClearParam = 'p'; + + static targets = ['input']; + + static values = { + icon: { default: '', type: String }, + loading: { default: false, type: Boolean }, + src: { default: '', type: String }, + target: { default: '#results', type: String }, + wait: { default: 200, type: Number }, + }; + + declare readonly hasInputTarget: boolean; + declare readonly hasTargetValue: boolean; + declare readonly hasUrlValue: boolean; + declare readonly inputTarget: HTMLInputElement; + + declare iconValue: string; + declare loadingValue: boolean; + declare srcValue: string; + declare targetValue: string; + declare waitValue: number; + + /** Allow cancelling of in flight async request if disconnected */ + abortController?: AbortController; + /** The related icon element to attach the spinner to */ + iconElement?: SVGUseElement | null; + /** Debounced function to search results and then replace the DOM */ + searchLazy?: { (...args: any[]): void; cancel(): void }; + /** Element that receives the fetch result HTML output */ + targetElement?: HTMLElement; + + /** + * Ensure we have backwards compatibility with setting window.headerSearch + * and allowing for elements without a controller attached to be set up. + * + * Will be removed in a future release. + */ + static afterLoad(identifier: string) { + domReady().then(() => { + const { termInput, targetOutput, url } = getGlobalHeaderSearchOptions(); + + const input = termInput + ? (document.querySelector(termInput) as HTMLInputElement) + : null; + + const form = input?.form; + + if (!form) return; + + if (!input.hasAttribute(`data-${identifier}-target`)) { + input.setAttribute(`data-${identifier}-target`, 'input'); + } + + Object.entries({ + 'data-controller': identifier, + 'data-action': [ + `change->${identifier}#searchLazy`, + `input->${identifier}#searchLazy`, + ].join(' '), + [`data-${identifier}-src-value`]: url, + [`data-${identifier}-target-value`]: targetOutput, + }).forEach(([key, value]) => { + if (!form.hasAttribute(key)) { + form.setAttribute(key, value as string); + } + }); + }); + } + + connect() { + const formContainer = this.hasInputTarget + ? this.inputTarget.form + : this.element; + this.srcValue = + this.srcValue || formContainer?.getAttribute('action') || ''; + this.targetElement = this.getTarget(this.targetValue); + + // set up icons + this.iconElement = null; + const iconContainer = ( + this.hasInputTarget ? this.inputTarget : this.element + ).parentElement; + + this.iconElement = iconContainer?.querySelector('use') || null; + this.iconValue = this.iconElement?.getAttribute('href') || ''; + + // set up initial loading state (if set originally in the HTML) + this.loadingValue = false; + + // set up debounced method + this.searchLazy = debounce(this.search.bind(this), this.waitValue); + } + + getTarget(targetValue = this.targetValue) { + const targetElement = document.querySelector(targetValue); + + const foundTarget = targetElement && targetElement instanceof HTMLElement; + const hasValidUrlValue = !!this.srcValue; + + const errors: string[] = []; + + if (!foundTarget) { + errors.push(`Cannot find valid target element at "${targetValue}"`); + } + + if (!hasValidUrlValue) { + errors.push(`Cannot find valid src URL value`); + } + + if (errors.length) { + throw new Error(errors.join(', ')); + } + + return targetElement as HTMLElement; + } + + /** + * Toggle the visual spinner icon if available and ensure content about + * to be replaced is flagged as busy. + */ + loadingValueChanged(isLoading: boolean) { + if (isLoading) { + this.targetElement?.setAttribute('aria-busy', 'true'); + this.iconElement?.setAttribute('href', '#icon-spinner'); + } else { + this.targetElement?.removeAttribute('aria-busy'); + this.iconElement?.setAttribute('href', this.iconValue); + } + } + + /** + * Perform a search based on a single input query, and only if that query's value + * differs from the current matching URL param. Once complete, update the URL param. + * Additionally, clear the `'p'` pagination param in the URL if present, can be overridden + * via action params if needed. + */ + search( + data?: CustomEvent<{ clear: string }> & { + params?: { clear?: string }; + }, + ) { + /** Params to be cleared when updating the location (e.g. ['p'] for page). */ + const clearParams = ( + data?.detail?.clear || + data?.params?.clear || + (this.constructor as typeof SwapController).defaultClearParam + ).split(' '); + + const searchInput = this.hasInputTarget ? this.inputTarget : this.element; + const queryParam = searchInput.name; + const searchParams = new URLSearchParams(window.location.search); + const currentQuery = searchParams.get(queryParam) || ''; + const newQuery = searchInput.value || ''; + + // only do the query if it has changed for trimmed queries + // for example - " " === "" and "first word " ==== "first word" + if (currentQuery.trim() === newQuery.trim()) return; + + // Update search query param ('q') to the new value or remove if empty + if (newQuery) { + searchParams.set(queryParam, newQuery); + } else { + searchParams.delete(queryParam); + } + + // clear any params (e.g. page/p) if needed + clearParams.forEach((param) => { + searchParams.delete(param); + }); + + const queryString = '?' + searchParams.toString(); + const url = this.srcValue; + + this.replace(url + queryString).then(() => { + window.history.replaceState(null, '', queryString); + }); + } + + /** + * Abort any existing requests & set up new abort controller, then fetch and replace + * the HTML target with the new results. + * Cancel any in progress results request using the AbortController so that + * a faster response does not replace an in flight request. + */ + async replace( + data: + | string + | (CustomEvent<{ url: string }> & { params?: { url?: string } }), + ) { + /** Parse a request URL from the supplied param, as a string or inside a custom event */ + const requestUrl = + typeof data === 'string' + ? data + : data.detail.url || data.params?.url || ''; + + if (this.abortController) this.abortController.abort(); + this.abortController = new AbortController(); + const { signal } = this.abortController; + + this.loadingValue = true; + + const beginEvent = this.dispatch('begin', { + cancelable: true, + detail: { requestUrl }, + // Stimulus dispatch target element type issue https://github.com/hotwired/stimulus/issues/642 + target: this.targetElement as HTMLInputElement, + }) as CustomEvent<{ requestUrl: string }>; + + if (beginEvent.defaultPrevented) return Promise.resolve(); + + return fetch(requestUrl, { + headers: { 'x-requested-with': 'XMLHttpRequest' }, + signal, + }) + .then((response) => { + if (!response.ok) { + throw new Error(`HTTP error! Status: ${response.status}`); + } + return response.text(); + }) + .then((results) => { + const targetElement = this.targetElement as HTMLElement; + targetElement.innerHTML = results; + this.dispatch('success', { + cancelable: false, + detail: { requestUrl, results }, + // Stimulus dispatch target element type issue https://github.com/hotwired/stimulus/issues/642 + target: targetElement as HTMLInputElement, + }); + return results; + }) + .catch((error) => { + if (signal.aborted) return; + this.dispatch('error', { + cancelable: false, + detail: { error, requestUrl }, + // Stimulus dispatch target element type issue https://github.com/hotwired/stimulus/issues/642 + target: this.targetElement as HTMLInputElement, + }); + // eslint-disable-next-line no-console + console.error(`Error fetching ${requestUrl}`, error); + }) + .finally(() => { + if (signal === this.abortController?.signal) { + this.loadingValue = false; + } + }); + } + + /** + * When disconnecting, ensure we reset any visual related state values and + * cancel any in-flight requests. + */ + disconnect() { + this.loadingValue = false; + this.searchLazy?.cancel(); + } +} diff --git a/client/src/controllers/index.ts b/client/src/controllers/index.ts index e10ebab0e0..c89c330bde 100644 --- a/client/src/controllers/index.ts +++ b/client/src/controllers/index.ts @@ -12,6 +12,7 @@ import { ProgressController } from './ProgressController'; import { SkipLinkController } from './SkipLinkController'; import { SlugController } from './SlugController'; import { SubmitController } from './SubmitController'; +import { SwapController } from './SwapController'; import { SyncController } from './SyncController'; import { TagController } from './TagController'; import { UpgradeController } from './UpgradeController'; @@ -32,6 +33,7 @@ export const coreControllerDefinitions: Definition[] = [ { controllerConstructor: SkipLinkController, identifier: 'w-skip-link' }, { controllerConstructor: SlugController, identifier: 'w-slug' }, { controllerConstructor: SubmitController, identifier: 'w-submit' }, + { controllerConstructor: SwapController, identifier: 'w-swap' }, { controllerConstructor: SyncController, identifier: 'w-sync' }, { controllerConstructor: TagController, identifier: 'w-tag' }, { controllerConstructor: UpgradeController, identifier: 'w-upgrade' }, diff --git a/client/src/entrypoints/admin/bulk-actions.js b/client/src/entrypoints/admin/bulk-actions.js index a32c88ddd0..e0651e49ed 100644 --- a/client/src/entrypoints/admin/bulk-actions.js +++ b/client/src/entrypoints/admin/bulk-actions.js @@ -4,13 +4,4 @@ import { } from '../../includes/bulk-actions'; document.addEventListener('DOMContentLoaded', addBulkActionListeners); - -if (window.headerSearch) { - const termInput = document.querySelector(window.headerSearch.termInput); - if (termInput) { - termInput.addEventListener( - 'search-success', - rebindBulkActionsEventListeners, - ); - } -} +document.addEventListener('w-swap:success', rebindBulkActionsEventListeners); diff --git a/client/src/entrypoints/admin/core.js b/client/src/entrypoints/admin/core.js index 95faf73590..dd6baececb 100644 --- a/client/src/entrypoints/admin/core.js +++ b/client/src/entrypoints/admin/core.js @@ -3,7 +3,6 @@ import $ from 'jquery'; import { coreControllerDefinitions } from '../../controllers'; import { escapeHtml } from '../../utils/text'; import { initStimulus } from '../../includes/initStimulus'; -import { initTooltips } from '../../includes/initTooltips'; /** initialise Wagtail Stimulus application with core controller definitions */ window.Stimulus = initStimulus({ definitions: coreControllerDefinitions }); @@ -216,62 +215,6 @@ $(() => { .on('dragleave dragend drop', function onDragLeave() { $(this).removeClass('hovered'); }); - - /* Header search behaviour */ - if (window.headerSearch) { - let searchCurrentIndex = 0; - let searchNextIndex = 0; - const $input = $(window.headerSearch.termInput); - const $inputContainer = $input.parent(); - const $icon = $inputContainer.find('use'); - const baseIcon = $icon.attr('href'); - - $input.on('keyup cut paste change', () => { - clearTimeout($input.data('timer')); - // eslint-disable-next-line @typescript-eslint/no-use-before-define - $input.data('timer', setTimeout(search, 200)); - }); - - // auto focus on search box - $input.trigger('focus'); - - // eslint-disable-next-line func-names - const search = function () { - const newQuery = $input.val(); - const searchParams = new URLSearchParams(window.location.search); - const currentQuery = searchParams.get('q') || ''; - // only do the query if it has changed for trimmed queries - // for example - " " === "" and "firstword " ==== "firstword" - if (currentQuery.trim() !== newQuery.trim()) { - $icon.attr('href', '#icon-spinner'); - searchNextIndex += 1; - const index = searchNextIndex; - - // Update q, reset to first page, and keep other query params - searchParams.set('q', newQuery); - searchParams.delete('p'); - const queryString = searchParams.toString(); - - $.ajax({ - url: window.headerSearch.url, - data: queryString, - success(data) { - if (index > searchCurrentIndex) { - searchCurrentIndex = index; - $(window.headerSearch.targetOutput).html(data).slideDown(800); - window.history.replaceState(null, null, '?' + queryString); - $input[0].dispatchEvent(new Event('search-success')); - } - }, - complete() { - // Reinitialise any tooltips - initTooltips(); - $icon.attr('href', baseIcon); - }, - }); - } - }; - } }); // ============================================================================= diff --git a/client/src/entrypoints/admin/wagtailadmin.js b/client/src/entrypoints/admin/wagtailadmin.js index 4390e087a6..f076689c11 100644 --- a/client/src/entrypoints/admin/wagtailadmin.js +++ b/client/src/entrypoints/admin/wagtailadmin.js @@ -36,3 +36,11 @@ window.addEventListener('load', () => { initAnchoredPanels(); initMinimap(); }); + +/** + * When search results are successful, reinitialise widgets + * that could be inside the newly injected DOM. + */ +window.addEventListener('w-swap:success', () => { + initTooltips(); // reinitialise any tooltips +}); diff --git a/docs/releases/5.1.md b/docs/releases/5.1.md index b661dc4938..a49be23897 100644 --- a/docs/releases/5.1.md +++ b/docs/releases/5.1.md @@ -68,6 +68,7 @@ Thank you to Damilola for his work, and to Google for sponsoring this project. * Ensure that `gettext_lazy` works correctly when using `verbose_name` on a generic Settings models (Sébastien Corbin) * Remove unnecessary usage of `innerHTML` when modifying DOM content (LB (Ben) Johnston) * Avoid `ValueError` when extending `PagesAPIViewSet` and setting `meta_fields` to an empty list (Henry Harutyunyan, Alex Morega) + * Improve accessibility for header search, remove autofocus on page load, advise screen readers that content has changed when results update (LB (Ben) Johnston) ### Documentation @@ -104,6 +105,7 @@ Thank you to Damilola for his work, and to Google for sponsoring this project. * Convert the `CONTRIBUTORS` file to Markdown (Dan Braghis) * Move `django-filter` version upper bound to v24 (Yuekui) * Update Pillow dependency to allow 10.x, only include support for >= 9.1.0 (Yuekui) + * Migrate header search behaviour to `w-swap`, a Stimulus controller (LB (Ben) Johnston) ## Upgrade considerations