From ba8a97527307948ccb7f2623e7ec82b82470b72a Mon Sep 17 00:00:00 2001 From: LB Johnston Date: Sun, 11 Jun 2023 13:51:47 +1000 Subject: [PATCH] SyncController - add better performance & enhance - Add support for a default 100ms debounce on apply calls so that multiple events (e.g. keyup) do not impact performance - Refine the delay for updating the field to not use setTimeout by default as this async behaviour is no longer needed for basic usage - Add unit tests for debounce functionality and clean up unit testing to ensure each test can run in isolation - Add explicit tests for non-inpu fields such as select (update code to ensure we use setter not setAttribute to support these) - Add ability to support an action param for a fixed value being synced to another field - Relates to #10517 --- client/src/controllers/SyncController.test.js | 131 +++++++++++++++++- client/src/controllers/SyncController.ts | 45 ++++-- 2 files changed, 157 insertions(+), 19 deletions(-) diff --git a/client/src/controllers/SyncController.test.js b/client/src/controllers/SyncController.test.js index db4c02958d..ea5b90524e 100644 --- a/client/src/controllers/SyncController.test.js +++ b/client/src/controllers/SyncController.test.js @@ -1,13 +1,15 @@ import { Application } from '@hotwired/stimulus'; import { SyncController } from './SyncController'; +import { range } from '../utils/range'; + jest.useFakeTimers(); describe('SyncController', () => { let application; describe('basic sync between two fields', () => { - beforeAll(() => { + beforeEach(() => { application?.stop(); document.body.innerHTML = ` @@ -19,7 +21,7 @@ describe('SyncController', () => { name="event-date" value="2025-07-22" data-controller="w-sync" - data-action="change->w-sync#apply cut->w-sync#clear custom:event->w-sync#ping" + data-action="change->w-sync#apply keyup->w-sync#apply cut->w-sync#clear custom:event->w-sync#ping" data-w-sync-target-value="#title" /> `; @@ -92,7 +94,7 @@ describe('SyncController', () => { expect(event.detail).toEqual({ element: document.getElementById('event-date'), - value: '2025-05-05', + value: '2025-07-22', }); }); @@ -102,7 +104,8 @@ describe('SyncController', () => { .getElementById('title') .addEventListener('change', changeListener); - expect(document.getElementById('title').value).toEqual('2025-05-05'); + const titleElement = document.getElementById('title'); + titleElement.setAttribute('value', 'initial title'); expect(changeListener).not.toHaveBeenCalled(); application.register('w-sync', SyncController); @@ -134,6 +137,52 @@ describe('SyncController', () => { expect(document.getElementById('title').value).toEqual(''); }); + + it('should debounce multiple consecutive calls to apply by default', () => { + const titleInput = document.getElementById('title'); + const dateInput = document.getElementById('event-date'); + + const changeListener = jest.fn(); + + titleInput.addEventListener('change', changeListener); + + dateInput.value = '2027-10-14'; + + application.register('w-sync', SyncController); + + range(0, 8).forEach(() => { + dateInput.dispatchEvent(new Event('keyup')); + jest.advanceTimersByTime(5); + }); + + expect(changeListener).not.toHaveBeenCalled(); + expect(titleInput.value).toEqual(''); + + jest.advanceTimersByTime(50); // not yet reaching the 100ms debounce value + + expect(changeListener).not.toHaveBeenCalled(); + expect(titleInput.value).toEqual(''); + + jest.advanceTimersByTime(50); // pass the 100ms debounce value + + // keyup run multiple times, only one change event should occur + expect(titleInput.value).toEqual('2027-10-14'); + expect(changeListener).toHaveBeenCalledTimes(1); + + // adjust the delay via a data attribute + dateInput.setAttribute('data-w-sync-delay-value', '500'); + + range(0, 8).forEach(() => { + dateInput.dispatchEvent(new Event('keyup')); + jest.advanceTimersByTime(5); + }); + + jest.advanceTimersByTime(300); // not yet reaching the custom debounce value + expect(changeListener).toHaveBeenCalledTimes(1); + + jest.advanceTimersByTime(295); // passing the custom debounce value + expect(changeListener).toHaveBeenCalledTimes(2); + }); }); describe('delayed sync between two fields', () => { @@ -169,9 +218,10 @@ describe('SyncController', () => { jest.advanceTimersByTime(500); expect(setTimeout).toHaveBeenLastCalledWith(expect.any(Function), 500); - expect(document.getElementById('title').value).toEqual(''); jest.runAllTimers(); + + expect(document.getElementById('title').value).toEqual(''); }); it('should delay the update on apply based on the set value', () => { @@ -193,10 +243,11 @@ describe('SyncController', () => { jest.advanceTimersByTime(500); expect(setTimeout).toHaveBeenLastCalledWith(expect.any(Function), 500); - expect(document.getElementById('title').value).toEqual('2025-05-05'); - expect(changeListener).toHaveBeenCalledTimes(1); jest.runAllTimers(); + + expect(document.getElementById('title').value).toEqual('2025-05-05'); + expect(changeListener).toHaveBeenCalledTimes(1); }); }); @@ -268,4 +319,70 @@ describe('SyncController', () => { expect(dateInput.getAttribute('data-w-sync-disabled-value')).toBeTruthy(); }); }); + + describe('ability to use sync for other field behaviour', () => { + beforeAll(() => { + application?.stop(); + }); + + it('should allow the sync clear method to be used on a button to clear target fields', async () => { + document.body.innerHTML = ` +
+ + +
`; + + application = Application.start(); + + application.register('w-sync', SyncController); + + await Promise.resolve(); + + expect(document.getElementById('title').value).toEqual('a title field'); + + document.getElementById('clear').click(); + + expect(document.getElementById('title').innerHTML).toEqual(''); + }); + + it('should allow the sync apply method to accept a param instead of the element value', async () => { + document.body.innerHTML = ` +
+ + +
`; + + application = Application.start(); + + application.register('w-sync', SyncController); + + await Promise.resolve(); + + expect(document.getElementById('pet-select').value).toEqual('dog'); + + document.getElementById('choose').dispatchEvent(new Event('click')); + + jest.runAllTimers(); + + expect(document.getElementById('pet-select').value).toEqual('pikachu'); + }); + }); }); diff --git a/client/src/controllers/SyncController.ts b/client/src/controllers/SyncController.ts index 4dd85e8300..9158062ab5 100644 --- a/client/src/controllers/SyncController.ts +++ b/client/src/controllers/SyncController.ts @@ -1,5 +1,7 @@ import { Controller } from '@hotwired/stimulus'; +import { debounce } from '../utils/debounce'; + /** * Adds ability to sync the value or interactions with one input with one * or more targeted other inputs. @@ -20,12 +22,14 @@ import { Controller } from '@hotwired/stimulus'; */ export class SyncController extends Controller { static values = { + debounce: { default: 100, type: Number }, delay: { default: 0, type: Number }, disabled: { default: false, type: Boolean }, quiet: { default: false, type: Boolean }, target: String, }; + declare debounceValue: number; declare delayValue: number; declare disabledValue: boolean; declare quietValue: boolean; @@ -38,6 +42,7 @@ export class SyncController extends Controller { */ connect() { this.processTargetElements('start', true); + this.apply = debounce(this.apply.bind(this), this.debounceValue); } /** @@ -50,20 +55,36 @@ export class SyncController extends Controller { /** * Applies a value from the controlled element to the targeted - * elements. + * elements. Calls to this method are debounced based on the + * controller's `debounceValue`. + * + * Applying of the value to the targets can be done with a delay, + * based on the controller's `delayValue`. */ - apply() { - this.processTargetElements('apply').forEach((target) => { - setTimeout(() => { - target.setAttribute('value', this.element.value); + apply(event?: Event & { params?: { apply?: string } }) { + const valueToApply = event?.params?.apply || this.element.value; - if (this.quietValue) return; - this.dispatch('change', { - cancelable: false, - prefix: '', - target: target as HTMLInputElement, - }); - }, this.delayValue); + const applyValue = (target) => { + /* use setter to correctly update value in non-inputs (e.g. select) */ // eslint-disable-next-line no-param-reassign + target.value = valueToApply; + + if (this.quietValue) return; + + this.dispatch('change', { + cancelable: false, + prefix: '', + target, + }); + }; + + this.processTargetElements('apply').forEach((target) => { + if (this.delayValue) { + setTimeout(() => { + applyValue(target); + }, this.delayValue); + } else { + applyValue(target); + } }); }