From 373743f96da8749c2cf4547feb488661712e94ae Mon Sep 17 00:00:00 2001 From: David Cook Date: Thu, 23 Nov 2023 13:35:15 +1100 Subject: [PATCH] Simplify event handlers The new 'input' event is for this exact use case. --- .../controllers/bulk_form_controller.js | 3 +- .../stimulus/bulk_form_controller_test.js | 42 +++++++------------ 2 files changed, 15 insertions(+), 30 deletions(-) diff --git a/app/webpacker/controllers/bulk_form_controller.js b/app/webpacker/controllers/bulk_form_controller.js index cb0cd64159..91af33e9e0 100644 --- a/app/webpacker/controllers/bulk_form_controller.js +++ b/app/webpacker/controllers/bulk_form_controller.js @@ -15,8 +15,7 @@ export default class BulkFormController extends Controller { // Start listening for any changes within the form // this.element.addEventListener('change', this.toggleChanged.bind(this)); // dunno why this doesn't work for (const element of this.form.elements) { - element.addEventListener("keyup", this.toggleChanged.bind(this)); // instant response - element.addEventListener("change", this.toggleChanged.bind(this)); // just in case (eg right-click paste) + element.addEventListener("input", this.toggleChanged.bind(this)); // immediately respond to any change // Set up a tree of fields according to their associated record const recordContainer = element.closest("[data-record-id]"); // The JS could be more efficient if this data was added to each element. But I didn't want to pollute the HTML too much. diff --git a/spec/javascripts/stimulus/bulk_form_controller_test.js b/spec/javascripts/stimulus/bulk_form_controller_test.js index fdbfcde2be..a1f0ee6250 100644 --- a/spec/javascripts/stimulus/bulk_form_controller_test.js +++ b/spec/javascripts/stimulus/bulk_form_controller_test.js @@ -57,9 +57,9 @@ describe("BulkFormController", () => { }); describe("marking changed fields", () => { - it("onChange", () => { + it("onInput", () => { input1a.value = 'updated1a'; - input1a.dispatchEvent(new Event("change")); + input1a.dispatchEvent(new Event("input")); // Expect only first field to show changed expect(input1a.classList).toContain('changed'); expect(input1b.classList).not.toContain('changed'); @@ -67,30 +67,16 @@ describe("BulkFormController", () => { // Change back to original value input1a.value = 'initial1a'; - input1a.dispatchEvent(new Event("change")); + input1a.dispatchEvent(new Event("input")); expect(input1a.classList).not.toContain('changed'); }); - it("onKeyup", () => { - input1a.value = 'u1a'; - input1a.dispatchEvent(new Event("keyup")); - // Expect only first field to show changed - expect(input1a.classList).toContain('changed'); - expect(input1b.classList).not.toContain('changed'); - expect(input2.classList).not.toContain('changed'); - - // Change back to original value - input1a.value = 'initial1a'; - input1a.dispatchEvent(new Event("keyup")); - expect(input1a.classList).not.toContain('changed'); - }); - it("multiple fields", () => { input1a.value = 'updated1a'; - input1a.dispatchEvent(new Event("change")); + input1a.dispatchEvent(new Event("input")); input2.value = 'updated2'; - input2.dispatchEvent(new Event("change")); + input2.dispatchEvent(new Event("input")); // Expect only first field to show changed expect(input1a.classList).toContain('changed'); expect(input1b.classList).not.toContain('changed'); @@ -98,7 +84,7 @@ describe("BulkFormController", () => { // Change only one back to original value input1a.value = 'initial1a'; - input1a.dispatchEvent(new Event("change")); + input1a.dispatchEvent(new Event("input")); expect(input1a.classList).not.toContain('changed'); expect(input1b.classList).not.toContain('changed'); expect(input2.classList).toContain('changed'); @@ -110,7 +96,7 @@ describe("BulkFormController", () => { it("counts changed records ", () => { // Record 1: First field changed input1a.value = 'updated1a'; - input1a.dispatchEvent(new Event("change")); + input1a.dispatchEvent(new Event("input")); // Actions and changed summary are shown, with other sections disabled expect(actions.classList).not.toContain('hidden'); expect(changed_summary.textContent).toBe('changed_summary, {"count":1}'); @@ -121,21 +107,21 @@ describe("BulkFormController", () => { // Record 1: Second field changed input1b.value = 'updated1b'; - input1b.dispatchEvent(new Event("change")); + input1b.dispatchEvent(new Event("input")); // Expect to show same summary translation expect(actions.classList).not.toContain('hidden'); expect(changed_summary.textContent).toBe('changed_summary, {"count":1}'); // Record 2: has been changed input2.value = 'updated2'; - input2.dispatchEvent(new Event("change")); + input2.dispatchEvent(new Event("input")); // Expect summary to count both records expect(actions.classList).not.toContain('hidden'); expect(changed_summary.textContent).toBe('changed_summary, {"count":2}'); // Record 1: Change first field back to original value input1a.value = 'initial1a'; - input1a.dispatchEvent(new Event("change")); + input1a.dispatchEvent(new Event("input")); // Both records are still changed. expect(input1a.classList).not.toContain('changed'); expect(input1b.classList).toContain('changed'); @@ -145,7 +131,7 @@ describe("BulkFormController", () => { // Record 1: Change second field back to original value input1b.value = 'initial1b'; - input1b.dispatchEvent(new Event("change")); + input1b.dispatchEvent(new Event("input")); // Both fields for record 1 show unchanged, but second record is still changed expect(actions.classList).not.toContain('hidden'); expect(changed_summary.textContent).toBe('changed_summary, {"count":1}'); @@ -156,7 +142,7 @@ describe("BulkFormController", () => { // Record 2: Change back to original value input2.value = 'initial2'; - input2.dispatchEvent(new Event("change")); + input2.dispatchEvent(new Event("input")); // Actions are hidden and other sections are now re-enabled expect(actions.classList).toContain('hidden'); expect(changed_summary.textContent).toBe('changed_summary, {"count":0}'); @@ -193,13 +179,13 @@ describe("BulkFormController", () => { // Record 1: First field changed input1a.value = 'updated1a'; - input1a.dispatchEvent(new Event("change")); + input1a.dispatchEvent(new Event("input")); // Expect actions to remain visible expect(actions.classList).not.toContain('hidden'); // Change back to original value input1a.value = 'initial1a'; - input1a.dispatchEvent(new Event("change")); + input1a.dispatchEvent(new Event("input")); // Expect actions to remain visible expect(actions.classList).not.toContain('hidden'); });