From 4afe01104b7c06d92b34e2db8a2fac72c7b25ea7 Mon Sep 17 00:00:00 2001 From: LB Johnston Date: Sat, 4 Feb 2023 11:06:43 +1000 Subject: [PATCH] Adopt Stimulus SwapController for task-chooser-modal use case - Builds on #9952 - Create a new method `submit` and `submitLazy` to serialise a form's inputs and submit (GET) async to replace content - Create a lazy version of `replace` and add unit tests for it - Partial progress on #9950 --- client/src/controllers/SwapController.test.js | 387 ++++++++++++++++++ client/src/controllers/SwapController.ts | 69 +++- .../entrypoints/admin/task-chooser-modal.js | 43 +- .../workflows/task_chooser/chooser.html | 11 +- 4 files changed, 473 insertions(+), 37 deletions(-) diff --git a/client/src/controllers/SwapController.test.js b/client/src/controllers/SwapController.test.js index 76bd6c3ce8ea..5f263762ac95 100644 --- a/client/src/controllers/SwapController.test.js +++ b/client/src/controllers/SwapController.test.js @@ -612,4 +612,391 @@ describe('SwapController', () => { document.removeEventListener('w-swap:begin', beginEventHandler); }); }); + + describe('performing a content update via actions on a controlled form using form values', () => { + let beginEventHandler; + let formElement; + let onSuccess; + const results = getMockResults({ total: 2 }); + + beforeEach(() => { + document.body.innerHTML = ` +
+
+
+ `; + + window.history.replaceState(null, '', '?'); + + formElement = document.getElementById('form'); + + onSuccess = new Promise((resolve) => { + document.addEventListener('w-swap:success', resolve); + }); + + beginEventHandler = jest.fn(); + document.addEventListener('w-swap:begin', beginEventHandler); + + fetch.mockImplementationOnce(() => + Promise.resolve({ + ok: true, + status: 200, + text: () => Promise.resolve(results), + }), + ); + }); + + it('should allow for actions to call the replace method directly, defaulting to the form action url', async () => { + const expectedRequestUrl = '/path/to/form/action/'; + + expect(window.location.search).toEqual(''); + expect(handleError).not.toHaveBeenCalled(); + expect(global.fetch).not.toHaveBeenCalled(); + + formElement.dispatchEvent( + new CustomEvent('custom:event', { bubbles: false }), + ); + + expect(beginEventHandler).not.toHaveBeenCalled(); + + jest.runAllTimers(); // search is debounced + + // should fire a begin event before the request is made + expect(beginEventHandler).toHaveBeenCalledTimes(1); + expect(beginEventHandler.mock.calls[0][0].detail).toEqual({ + requestUrl: expectedRequestUrl, + }); + + // visual loading state should be active + await Promise.resolve(); // trigger next rendering + + expect(handleError).not.toHaveBeenCalled(); + expect(global.fetch).toHaveBeenCalledWith( + expectedRequestUrl, + expect.any(Object), + ); + + const successEvent = await onSuccess; + + // should dispatch success event + expect(successEvent.detail).toEqual({ + requestUrl: expectedRequestUrl, + results: expect.any(String), + }); + + // should update HTML + expect( + document.getElementById('content').querySelectorAll('li'), + ).toHaveLength(5); + + await flushPromises(); + + // should NOT update the current URL + expect(window.location.search).toEqual(''); + }); + + it('should support replace with a src value', async () => { + const expectedRequestUrl = '/path/to-src-value/'; + + expect(window.location.search).toEqual(''); + expect(handleError).not.toHaveBeenCalled(); + expect(global.fetch).not.toHaveBeenCalled(); + + formElement.setAttribute('data-w-swap-src-value', expectedRequestUrl); + + formElement.dispatchEvent( + new CustomEvent('custom:event', { bubbles: false }), + ); + + expect(beginEventHandler).not.toHaveBeenCalled(); + + jest.runAllTimers(); // search is debounced + + // should fire a begin event before the request is made + expect(beginEventHandler).toHaveBeenCalledTimes(1); + expect(beginEventHandler.mock.calls[0][0].detail).toEqual({ + requestUrl: expectedRequestUrl, + }); + + // visual loading state should be active + await Promise.resolve(); // trigger next rendering + + expect(handleError).not.toHaveBeenCalled(); + expect(global.fetch).toHaveBeenCalledWith( + expectedRequestUrl, + expect.any(Object), + ); + + const successEvent = await onSuccess; + + // should dispatch success event + expect(successEvent.detail).toEqual({ + requestUrl: expectedRequestUrl, + results: expect.any(String), + }); + + // should update HTML + expect( + document.getElementById('content').querySelectorAll('li'), + ).toHaveLength(2); + + await flushPromises(); + + // should NOT update the current URL + expect(window.location.search).toEqual(''); + }); + + it('should support replace with a url value provided via the Custom event detail', async () => { + const expectedRequestUrl = '/path/to/url-in-event-detail/?q=alpha'; + + expect(window.location.search).toEqual(''); + expect(handleError).not.toHaveBeenCalled(); + expect(global.fetch).not.toHaveBeenCalled(); + + formElement.dispatchEvent( + new CustomEvent('custom:event', { + bubbles: false, + detail: { url: expectedRequestUrl }, + }), + ); + + expect(beginEventHandler).not.toHaveBeenCalled(); + + jest.runAllTimers(); // search is debounced + + // should fire a begin event before the request is made + expect(beginEventHandler).toHaveBeenCalledTimes(1); + expect(beginEventHandler.mock.calls[0][0].detail).toEqual({ + requestUrl: expectedRequestUrl, + }); + + // visual loading state should be active + await Promise.resolve(); // trigger next rendering + + expect(handleError).not.toHaveBeenCalled(); + expect(global.fetch).toHaveBeenCalledWith( + expectedRequestUrl, + expect.any(Object), + ); + + const successEvent = await onSuccess; + + // should dispatch success event + expect(successEvent.detail).toEqual({ + requestUrl: expectedRequestUrl, + results: expect.any(String), + }); + + // should update HTML + expect( + document.getElementById('content').querySelectorAll('li'), + ).toHaveLength(2); + + await flushPromises(); + + // should NOT update the current URL + expect(window.location.search).toEqual(''); + }); + + it('should support replace with a url value provided via an action param', async () => { + const expectedRequestUrl = '/path/to/url-in-action-param/#hash'; + + expect(window.location.search).toEqual(''); + expect(handleError).not.toHaveBeenCalled(); + expect(global.fetch).not.toHaveBeenCalled(); + + formElement.setAttribute('data-w-swap-url-param', expectedRequestUrl); + + formElement.dispatchEvent( + new CustomEvent('custom:event', { bubbles: false }), + ); + + expect(beginEventHandler).not.toHaveBeenCalled(); + + jest.runAllTimers(); // search is debounced + + // should fire a begin event before the request is made + expect(beginEventHandler).toHaveBeenCalledTimes(1); + expect(beginEventHandler.mock.calls[0][0].detail).toEqual({ + requestUrl: expectedRequestUrl, + }); + + // visual loading state should be active + await Promise.resolve(); // trigger next rendering + + expect(handleError).not.toHaveBeenCalled(); + expect(global.fetch).toHaveBeenCalledWith( + expectedRequestUrl, + expect.any(Object), + ); + + const successEvent = await onSuccess; + + // should dispatch success event + expect(successEvent.detail).toEqual({ + requestUrl: expectedRequestUrl, + results: expect.any(String), + }); + + // should update HTML + expect( + document.getElementById('content').querySelectorAll('li'), + ).toHaveLength(2); + + await flushPromises(); + + // should NOT update the current URL + expect(window.location.search).toEqual(''); + }); + }); + + describe('performing a content update via actions on a controlled form using form values', () => { + beforeEach(() => { + // intentionally testing without target input (icon not needed & should work without this) + + 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 results = getMockResults({ total: 5 }); + + const onSuccess = new Promise((resolve) => { + document.addEventListener('w-swap:success', resolve); + }); + + const beginEventHandler = jest.fn(); + 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'; + document.querySelector('[name="other"]').value = 'something on other'; + input.dispatchEvent(new CustomEvent('change', { bubbles: true })); + + expect(beginEventHandler).not.toHaveBeenCalled(); + + jest.runAllTimers(); // search is debounced + + // 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&type=some-type&other=something+on+other', + }); + + // visual loading state should be active + await Promise.resolve(); // trigger next rendering + + expect(handleError).not.toHaveBeenCalled(); + expect(global.fetch).toHaveBeenCalledWith( + '/path/to/form/action/?q=alpha&type=some-type&other=something+on+other', + expect.any(Object), + ); + + const successEvent = await onSuccess; + + // should dispatch success event + expect(successEvent.detail).toEqual({ + requestUrl: + '/path/to/form/action/?q=alpha&type=some-type&other=something+on+other', + results: expect.any(String), + }); + + // should update HTML + expect( + document.getElementById('task-results').querySelectorAll('li').length, + ).toBeTruthy(); + + await flushPromises(); + + // should NOT update the current URL + expect(window.location.search).toEqual(''); + }); + + 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'; + document.querySelector('[name="other"]').value = 'something on other'; + input.dispatchEvent(new CustomEvent('change', { 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&type=some-type&other=something+on+other', + }); + + 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 index 4bac441e5c9d..fa3661eed8d2 100644 --- a/client/src/controllers/SwapController.ts +++ b/client/src/controllers/SwapController.ts @@ -24,9 +24,22 @@ const getGlobalHeaderSearchOptions = (): { * 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. + * when processed or simply make a query based on a form's + * values. * - * @example + * @example - A form that will update the results based on the form's input + *
+ *
+ * + * + *
+ * + * @example - A single input that will update the results & the URL *
* + * */ export class SwapController extends Controller< HTMLFormElement | HTMLInputElement @@ -68,8 +82,12 @@ export class SwapController extends Controller< abortController?: AbortController; /** The related icon element to attach the spinner to */ iconElement?: SVGUseElement | null; + /** Debounced function to request a URL and then replace the DOM with the results */ + replaceLazy?: { (...args: any[]): void; cancel(): void }; /** Debounced function to search results and then replace the DOM */ searchLazy?: { (...args: any[]): void; cancel(): void }; + /** Debounced function to submit the serialised form and then replace the DOM */ + submitLazy?: { (...args: any[]): void; cancel(): void }; /** Element that receives the fetch result HTML output */ targetElement?: HTMLElement; @@ -131,8 +149,13 @@ export class SwapController extends Controller< // set up initial loading state (if set originally in the HTML) this.loadingValue = false; - // set up debounced method + // set up debounced methods + this.replaceLazy = debounce(this.replace.bind(this), this.waitValue); this.searchLazy = debounce(this.search.bind(this), this.waitValue); + this.submitLazy = debounce(this.submit.bind(this), this.waitValue); + + // dispatch event for any initial action usage + this.dispatch('ready', { cancelable: false }); } getTarget(targetValue = this.targetValue) { @@ -173,10 +196,14 @@ export class SwapController extends Controller< } /** - * 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. + * Perform a URL search param update based on the input's value with a comparison against the + * matching URL search params. Will replace the target element's content with the results + * of the async search request based on the query. + * + * Search will only be performed with the URL param value is different to the input value. + * Cleared params will be removed from the URL if present. + * + * `clear` can be provided as Event detail or action param to override the default of 'p'. */ search( data?: CustomEvent<{ clear: string }> & { @@ -220,6 +247,26 @@ export class SwapController extends Controller< }); } + /** + * Update the target element's content with the response from a request based on the input's form + * values serialised. Do not account for anything in the main location/URL, simply replace the content within + * the target element. + */ + submit() { + const form = ( + this.hasInputTarget ? this.inputTarget.form : this.element + ) as HTMLFormElement; + + // serialise the form to a query string + // https://github.com/microsoft/TypeScript/issues/43797 + const searchParams = new URLSearchParams(new FormData(form) as any); + + const queryString = '?' + searchParams.toString(); + const url = this.srcValue; + + this.replace(url + queryString); + } + /** * Abort any existing requests & set up new abort controller, then fetch and replace * the HTML target with the new results. @@ -227,15 +274,15 @@ export class SwapController extends Controller< * a faster response does not replace an in flight request. */ async replace( - data: + 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' + (typeof data === 'string' ? data - : data.detail.url || data.params?.url || ''; + : data?.detail?.url || data?.params?.url || '') || this.srcValue; if (this.abortController) this.abortController.abort(); this.abortController = new AbortController(); @@ -297,6 +344,8 @@ export class SwapController extends Controller< */ disconnect() { this.loadingValue = false; + this.replaceLazy?.cancel(); this.searchLazy?.cancel(); + this.submitLazy?.cancel(); } } diff --git a/client/src/entrypoints/admin/task-chooser-modal.js b/client/src/entrypoints/admin/task-chooser-modal.js index d28d60853baa..747f5e60c7cd 100644 --- a/client/src/entrypoints/admin/task-chooser-modal.js +++ b/client/src/entrypoints/admin/task-chooser-modal.js @@ -1,9 +1,6 @@ import $ from 'jquery'; import { initTabs } from '../../includes/tabs'; -import { - submitCreationForm, - SearchController, -} from '../../includes/chooserModal'; +import { submitCreationForm } from '../../includes/chooserModal'; const ajaxifyTaskCreateTab = (modal) => { $( @@ -24,36 +21,30 @@ const ajaxifyTaskCreateTab = (modal) => { const TASK_CHOOSER_MODAL_ONLOAD_HANDLERS = { chooser(modal, jsonData) { + const form = $('form.task-search', modal.body)[0]; + function ajaxifyLinks(context) { - $('a.task-choice', context) - // eslint-disable-next-line func-names - .on('click', function () { - modal.loadUrl(this.href); - return false; - }); + $('a.task-choice', context).on('click', function handleClick() { + modal.loadUrl(this.href); + return false; + }); - // eslint-disable-next-line func-names - $('.pagination a', context).on('click', function () { - // eslint-disable-next-line @typescript-eslint/no-use-before-define - searchController.fetchResults(this.href); + $('.pagination a', context).on('click', function handleClick() { + const url = this.href; + form.dispatchEvent(new CustomEvent('navigate', { detail: { url } })); return false; }); // Reinitialize tabs to hook up tab event listeners in the modal initTabs(); - } - const searchController = new SearchController({ - form: $('form.task-search', modal.body), - containerElement: modal.body, - resultsContainerSelector: '#search-results', - onLoadResults: (context) => { - ajaxifyLinks(context); - }, - inputDelay: 50, - }); - searchController.attachSearchInput('#id_q'); - searchController.attachSearchFilter('#id_task_type'); + // set up success handling when new results are returned for next search + modal.body[0].addEventListener( + 'w-swap:success', + ({ srcElement }) => ajaxifyLinks($(srcElement)), + { once: true }, + ); + } ajaxifyLinks(modal.body); ajaxifyTaskCreateTab(modal, jsonData); diff --git a/wagtail/admin/templates/wagtailadmin/workflows/task_chooser/chooser.html b/wagtail/admin/templates/wagtailadmin/workflows/task_chooser/chooser.html index 0b987262f13c..fa813a458b93 100644 --- a/wagtail/admin/templates/wagtailadmin/workflows/task_chooser/chooser.html +++ b/wagtail/admin/templates/wagtailadmin/workflows/task_chooser/chooser.html @@ -30,7 +30,16 @@ aria-labelledby="tab-label-existing" hidden > -
+