Skip to content

Commit

Permalink
Simplify event handlers
Browse files Browse the repository at this point in the history 
The new 'input' event is for this exact use case.
  • Loading branch information
@dacook
dacook committed Nov 23, 2023
1 parent 10886a4 commit 1abedae
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 30 deletions.
3 changes: 1 addition & 2 deletions app/webpacker/controllers/bulk_form_controller.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
42 changes: 14 additions & 28 deletions spec/javascripts/stimulus/bulk_form_controller_test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,48 +57,34 @@ 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');
expect(input2.classList).not.toContain('changed');

// 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');
expect(input2.classList).toContain('changed');

// 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');
Expand All @@ -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}');
Expand All @@ -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');
Expand All @@ -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}');
Expand All @@ -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}');
Expand Down Expand Up @@ -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');
});
Expand Down

0 comments on commit 1abedae

Please sign in to comment.