From fd9eed97d793c7decda25db7ee405f87827e9ba8 Mon Sep 17 00:00:00 2001 From: Lovelyfin00 Date: Tue, 24 Jan 2023 12:34:13 +0100 Subject: [PATCH] Converted button-longrunning to a Stimulus Controller - implemented afterLoad in Stimulus button-longrunning to support non-adopted data attributes - Partial completed #9910 --- CHANGELOG.txt | 1 + .../controllers/ProgressController.test.js | 107 +++++++++++++ client/src/controllers/ProgressController.ts | 151 ++++++++++++++++++ client/src/controllers/index.ts | 2 + client/src/entrypoints/admin/core.js | 65 -------- docs/releases/5.0.md | 1 + .../wagtailadmin/workflows/edit_task.html | 11 +- 7 files changed, 271 insertions(+), 67 deletions(-) create mode 100644 client/src/controllers/ProgressController.test.js create mode 100644 client/src/controllers/ProgressController.ts diff --git a/CHANGELOG.txt b/CHANGELOG.txt index 08a3b6efb8..56010647c3 100644 --- a/CHANGELOG.txt +++ b/CHANGELOG.txt @@ -6,6 +6,7 @@ Changelog * Add `WAGTAILIMAGES_EXTENSIONS` setting to restrict image uploads to specific file types (Aman Pandey, Ananjan-R) * Update user list column level to `Access level` to be easier to understand (Vallabh Tiwari) + * Migrate `.button-longrunning` behaviour to a Stimulus controller with support for custom label element & duration (Loveth Omokaro) * Fix: Ensure `label_format` on StructBlock gracefully handles missing variables (Aadi jindal) * Fix: Adopt a no-JavaScript and more accessible solution for the 'Reset to default' switch to Gravatar when editing user profile (Loveth Omokaro) * Docs: Add code block to make it easier to understand contribution docs (Suyash Singh) diff --git a/client/src/controllers/ProgressController.test.js b/client/src/controllers/ProgressController.test.js new file mode 100644 index 0000000000..1ca794f482 --- /dev/null +++ b/client/src/controllers/ProgressController.test.js @@ -0,0 +1,107 @@ +import { Application } from '@hotwired/stimulus'; +import { ProgressController } from './ProgressController'; + +jest.useFakeTimers(); + +const flushPromises = () => new Promise(setImmediate); +describe('ProgressController', () => { + // form submit is not implemented in jsdom + const mockSubmit = jest.fn((e) => e.preventDefault()); + + beforeEach(() => { + document.body.innerHTML = ` +
+ +
+ `; + + document.getElementById('form').addEventListener('submit', mockSubmit); + + Application.start().register('w-progress', ProgressController); + }); + + afterEach(() => { + document.body.innerHTML = ''; + jest.clearAllMocks(); + jest.clearAllTimers(); + }); + + it('should not change the text of the button to Loading if the form is not valid', async () => { + const form = document.querySelector('form'); + const button = document.querySelector('.button-longrunning'); + expect(mockSubmit).not.toHaveBeenCalled(); + + form.noValidate = false; + form.checkValidity = jest.fn().mockReturnValue(false); + const onClick = jest.fn(); + button.addEventListener('click', onClick); + button.dispatchEvent(new CustomEvent('click')); + + jest.advanceTimersByTime(10); + await flushPromises(); + + expect(mockSubmit).not.toHaveBeenCalled(); + expect(button.disabled).toEqual(false); + expect(onClick).toHaveBeenCalledTimes(1); + + jest.runAllTimers(); + await flushPromises(); + + expect(mockSubmit).not.toHaveBeenCalled(); + }); + + it('should trigger a timeout based on the value attribute', () => { + const form = document.querySelector('form'); + const button = document.querySelector('.button-longrunning'); + jest.spyOn(global, 'setTimeout'); + + button.click(); + + jest.runAllTimers(); + + // default timer 30 seconds + expect(setTimeout).toHaveBeenLastCalledWith(expect.any(Function), 30_000); + + // change to 4 seconds + document + .getElementById('button') + .setAttribute('data-w-progress-duration-seconds-value', '4'); + + button.click(); + jest.runAllTimers(); + + expect(setTimeout).toHaveBeenLastCalledWith(expect.any(Function), 4_000); + }); + + it('should change the the text of the button and sets disabled attribute on click', async () => { + const button = document.querySelector('.button-longrunning'); + const label = document.querySelector('#em-el'); + expect(mockSubmit).not.toHaveBeenCalled(); + + button.click(); + jest.advanceTimersByTime(10); + await flushPromises(); + + expect(label.textContent).toBe('Loading'); + expect(button.getAttribute('disabled')).toEqual(''); + expect(button.classList.contains('button-longrunning-active')).toBe(true); + + jest.runAllTimers(); + await flushPromises(); + + expect(mockSubmit).toHaveBeenCalled(); + expect(label.textContent).toBe('Sign in'); + expect(button.getAttribute('disabled')).toBeNull(); + expect(button.classList.contains('button-longrunning-active')).toBe(false); + }); +}); diff --git a/client/src/controllers/ProgressController.ts b/client/src/controllers/ProgressController.ts new file mode 100644 index 0000000000..87cc271017 --- /dev/null +++ b/client/src/controllers/ProgressController.ts @@ -0,0 +1,151 @@ +import { Application, Controller } from '@hotwired/stimulus'; + +const DEFAULT_CLASS = 'button-longrunning'; + +/** + * Adds the ability for a button to be clicked and then not allow any further clicks + * until the duration has elapsed. Will also update the button's label while in progress. + * + * @example + * + */ +export class ProgressController extends Controller { + static classes = ['active']; + static targets = ['label']; + static values = { + active: { default: '', type: String }, + durationSeconds: { default: 30, type: Number }, + label: { default: '', type: String }, + loading: { default: false, type: Boolean }, + }; + + declare activeClass: string; + /** Label to use when loading */ + declare activeValue: string; + declare durationSecondsValue: number; + /** Label to store the original text on the button */ + declare labelValue: string; + declare loadingValue: boolean; + declare readonly hasActiveClass: boolean; + declare readonly hasLabelTarget: boolean; + declare readonly labelTarget: HTMLElement; + timer?: number; + + /** + * Ensure we have backwards compatibility with buttons that have + * not yet adopted the new data attribute syntax. + * Will warn and advise in release notes that this support + * will be removed in a future version. + * @deprecated - RemovedInWagtail60 + */ + static afterLoad(identifier: string, application: Application) { + const { controllerAttribute } = application.schema; + const { actionAttribute } = application.schema; + + document.addEventListener( + 'DOMContentLoaded', + () => { + document + .querySelectorAll( + `.${DEFAULT_CLASS}:not([${controllerAttribute}~='${identifier}'])`, + ) + .forEach((button) => { + button.setAttribute(controllerAttribute, identifier); + button.setAttribute(actionAttribute, `${identifier}#activate`); + + const activeText = button.getAttribute('data-clicked-text'); + if (activeText) { + button.setAttribute( + `data-${identifier}-active-value`, + activeText, + ); + button.removeAttribute('data-clicked-text'); + } + + const labelElement = button.querySelector('em'); + if (labelElement) { + labelElement.setAttribute(`data-${identifier}-target`, 'label'); + } + + button.setAttribute( + `data-${identifier}-duration-seconds-value`, + '30', + ); + }); + }, + { once: true, passive: true }, + ); + } + + activate() { + // If client-side validation is active on this form, and is going to block submission of the + // form, don't activate the spinner + const form = this.element.closest('form'); + + if ( + form && + form.checkValidity && + !form.noValidate && + !form.checkValidity() + ) { + return; + } + + window.setTimeout(() => { + this.loadingValue = true; + + const durationMs = this.durationSecondsValue * 1000; + + this.timer = window.setTimeout(() => { + this.loadingValue = false; + }, durationMs); + }); + } + + loadingValueChanged(isLoading: boolean) { + const activeClass = this.hasActiveClass + ? this.activeClass + : `${DEFAULT_CLASS}-active`; + + this.element.classList.toggle(activeClass, isLoading); + + if (!this.labelValue) { + this.labelValue = this.hasLabelTarget + ? (this.labelTarget.textContent as string) + : (this.element.textContent as string); + } + + if (isLoading) { + // Disabling button must be done last: disabled buttons can't be + // modified in the normal way, it would seem. + this.element.setAttribute('disabled', ''); + + if (this.activeValue && this.hasLabelTarget) { + this.labelTarget.textContent = this.activeValue; + } + } else { + this.element.removeAttribute('disabled'); + + if (this.labelValue && this.hasLabelTarget) { + this.labelTarget.textContent = this.labelValue; + } + } + } + + disconnect(): void { + if (this.timer) { + clearTimeout(this.timer); + } + } +} diff --git a/client/src/controllers/index.ts b/client/src/controllers/index.ts index 51fca0b0bc..a552961d3e 100644 --- a/client/src/controllers/index.ts +++ b/client/src/controllers/index.ts @@ -3,6 +3,7 @@ import type { Definition } from '@hotwired/stimulus'; // Order controller imports alphabetically. import { ActionController } from './ActionController'; import { AutoFieldController } from './AutoFieldController'; +import { ProgressController } from './ProgressController'; import { SkipLinkController } from './SkipLinkController'; import { UpgradeController } from './UpgradeController'; @@ -13,6 +14,7 @@ export const coreControllerDefinitions: Definition[] = [ // Keep this list in alphabetical order { controllerConstructor: ActionController, identifier: 'w-action' }, { controllerConstructor: AutoFieldController, identifier: 'w-auto-field' }, + { controllerConstructor: ProgressController, identifier: 'w-progress' }, { controllerConstructor: SkipLinkController, identifier: 'w-skip-link' }, { controllerConstructor: UpgradeController, identifier: 'w-upgrade' }, ]; diff --git a/client/src/entrypoints/admin/core.js b/client/src/entrypoints/admin/core.js index 9f0561a40d..26de983d91 100644 --- a/client/src/entrypoints/admin/core.js +++ b/client/src/entrypoints/admin/core.js @@ -304,71 +304,6 @@ $(() => { } }; } - - /* Debounce submission of long-running forms and add spinner to give sense of activity */ - // eslint-disable-next-line func-names - $(document).on('click', 'button.button-longrunning', function () { - const $self = $(this); - const $replacementElem = $('em', $self); - const reEnableAfter = 30; - const dataName = 'disabledtimeout'; - - // eslint-disable-next-line func-names - window.cancelSpinner = function () { - $self - .prop('disabled', '') - .removeData(dataName) - .removeClass('button-longrunning-active'); - - if ($self.data('clicked-text')) { - $replacementElem.text($self.data('original-text')); - } - }; - - // If client-side validation is active on this form, and is going to block submission of the - // form, don't activate the spinner - const form = $self.closest('form').get(0); - if ( - form && - form.checkValidity && - !form.noValidate && - !form.checkValidity() - ) { - return; - } - - // Disabling a button prevents it submitting the form, so disabling - // must occur on a brief timeout only after this function returns. - - const timeout = setTimeout(() => { - if (!$self.data(dataName)) { - // Button re-enables after a timeout to prevent button becoming - // permanently un-usable - $self.data( - dataName, - setTimeout(() => { - clearTimeout($self.data(dataName)); - - // eslint-disable-next-line no-undef - cancelSpinner(); - }, reEnableAfter * 1000), - ); - - if ($self.data('clicked-text') && $replacementElem.length) { - // Save current button text - $self.data('original-text', $replacementElem.text()); - - $replacementElem.text($self.data('clicked-text')); - } - - // Disabling button must be done last: disabled buttons can't be - // modified in the normal way, it would seem. - $self.addClass('button-longrunning-active').prop('disabled', 'true'); - } - - clearTimeout(timeout); - }, 10); - }); }); // ============================================================================= diff --git a/docs/releases/5.0.md b/docs/releases/5.0.md index 84ff1558e0..ce3aebcc74 100644 --- a/docs/releases/5.0.md +++ b/docs/releases/5.0.md @@ -17,6 +17,7 @@ depth: 1 * Add `WAGTAILIMAGES_EXTENSIONS` setting to restrict image uploads to specific file types (Aman Pandey, Ananjan-R) * Update user list column level to `Access level` to be easier to understand (Vallabh Tiwari) + * Migrate `.button-longrunning` behaviour to a Stimulus controller with support for custom label element & duration (Loveth Omokaro) ### Bug fixes diff --git a/wagtail/admin/templates/wagtailadmin/workflows/edit_task.html b/wagtail/admin/templates/wagtailadmin/workflows/edit_task.html index 4406f4e32b..9d94d71200 100644 --- a/wagtail/admin/templates/wagtailadmin/workflows/edit_task.html +++ b/wagtail/admin/templates/wagtailadmin/workflows/edit_task.html @@ -41,8 +41,15 @@
  • {% block form_actions %}
    -
    {% icon name="arrow-up" %}