From 8ff67aca415a6e669540192c540bf510f0792b7f Mon Sep 17 00:00:00 2001 From: David Cook Date: Thu, 14 Sep 2023 11:12:13 +1000 Subject: [PATCH] Disable filters and sorting when form is modified Stimulus controllers aren't supposed to reach outside their own element (so we can't do this with targets). Perhaps the controller should be bigger to encompass more, but I wanted to see if I could avoid making a mega component that does everything. For now it seems appropriate just to pass a selector in. Another option is to publish events on other controllers using Outlets, but I don't know if we need to go there just yet. --- app/views/admin/products_v3/_table.html.haml | 2 +- .../controllers/bulk_form_controller.js | 25 +++++++++++- app/webpacker/css/admin/products_v3.scss | 21 +++++++++- .../stimulus/bulk_form_controller_test.js | 40 ++++++++++++++++++- 4 files changed, 83 insertions(+), 5 deletions(-) diff --git a/app/views/admin/products_v3/_table.html.haml b/app/views/admin/products_v3/_table.html.haml index 7e94f84c32..4d31694077 100644 --- a/app/views/admin/products_v3/_table.html.haml +++ b/app/views/admin/products_v3/_table.html.haml @@ -1,6 +1,6 @@ = form_with url: bulk_update_admin_products_v3_index_path, method: :patch, id: "products-form", html: {'data-reflex-serialize-form': true, 'data-reflex': 'submit->products#bulk_update', - 'data-controller': "bulk-form"} do |form| + 'data-controller': "bulk-form", 'data-bulk-form-disable-selector-value': "#sort,#filters"} do |form| %fieldset.form-actions.hidden{ 'data-bulk-form-target': "actions" } .container .status.ten.columns diff --git a/app/webpacker/controllers/bulk_form_controller.js b/app/webpacker/controllers/bulk_form_controller.js index d008ad7485..d5a70a9105 100644 --- a/app/webpacker/controllers/bulk_form_controller.js +++ b/app/webpacker/controllers/bulk_form_controller.js @@ -3,6 +3,9 @@ import { Controller } from "stimulus"; // Manages "modified" state for a form with multiple records export default class BulkFormController extends Controller { static targets = ["actions", "modifiedSummary"]; + static values = { + disableSelector: String, + }; recordElements = {}; connect() { @@ -24,6 +27,11 @@ export default class BulkFormController extends Controller { } } + disconnect() { + // Make sure to clean up anything that happened outside + this.#disableOtherElements(false); + } + toggleModified(e) { const element = e.target; const modified = element.value != element.defaultValue; @@ -39,8 +47,11 @@ export default class BulkFormController extends Controller { return element.value != element.defaultValue; }); }).length; + const formModified = modifiedRecordCount > 0; - this.actionsTarget.classList.toggle("hidden", modifiedRecordCount == 0); + // Show actions + this.actionsTarget.classList.toggle("hidden", !formModified); + this.#disableOtherElements(formModified); // like filters and sorting // Display number of records modified const key = this.modifiedSummaryTarget && this.modifiedSummaryTarget.dataset.translationKey; @@ -48,4 +59,16 @@ export default class BulkFormController extends Controller { this.modifiedSummaryTarget.textContent = I18n.t(key, { count: modifiedRecordCount }); } } + + // private + + #disableOtherElements(disable) { + this.disableElements ||= document.querySelectorAll(this.disableSelectorValue); + + if (this.disableElements) { + this.disableElements.forEach((element) => { + element.classList.toggle("disabled-section", disable); + }); + } + } } diff --git a/app/webpacker/css/admin/products_v3.scss b/app/webpacker/css/admin/products_v3.scss index fb8cb82300..434c20fce3 100644 --- a/app/webpacker/css/admin/products_v3.scss +++ b/app/webpacker/css/admin/products_v3.scss @@ -22,9 +22,10 @@ // Form actions floats over other controls when active .form-actions { position: absolute; - top: 1em; + top: -1em; left: 0; right: 0; + z-index: 1; // Ensure tom-select and .disabled-section are covered } // Hopefully these rules will be moved to component(s). @@ -234,4 +235,22 @@ } } } + + // Blurred and non-clickable + $disabled-blur: 1.5px; + .disabled-section { + position: relative; + + &::after { + content: ""; + position: absolute; + backdrop-filter: blur($disabled-blur); + // Stretch outside for a soft blur edge: + left: -$disabled-blur; + top: -$disabled-blur; + bottom: -$disabled-blur; + right: -$disabled-blur; + z-index: 1; // Ensure tom-select is covered + } + } } diff --git a/spec/javascripts/stimulus/bulk_form_controller_test.js b/spec/javascripts/stimulus/bulk_form_controller_test.js index a5ef778926..ce1d0bda0d 100644 --- a/spec/javascripts/stimulus/bulk_form_controller_test.js +++ b/spec/javascripts/stimulus/bulk_form_controller_test.js @@ -28,7 +28,9 @@ describe("BulkFormController", () => { beforeEach(() => { document.body.innerHTML = ` -
+
+
+
@@ -38,6 +40,7 @@ describe("BulkFormController", () => {
+ `; }); @@ -47,6 +50,8 @@ describe("BulkFormController", () => { // would be repeated if these were broken into multiple examples. So it seems impractical to // write individual unit tests. it("counts modified fields and records", () => { + const disable1 = document.getElementById("disable1"); + const disable2 = document.getElementById("disable2"); const actions = document.getElementById("actions"); const modified_summary = document.getElementById("modified_summary"); const input1a = document.getElementById("input1a"); @@ -56,12 +61,15 @@ describe("BulkFormController", () => { // Record 1: First field changed (we're not simulating a user in a browser here; we're testing DOM events directly) input1a.value = 'updated1a'; input1a.dispatchEvent(new Event("change")); - // Expect only first field to show modified, and show modified summary translation + // Expect only first field to show modified expect(input1a.classList).toContain('modified'); expect(input1b.classList).not.toContain('modified'); expect(input2.classList).not.toContain('modified'); + // Actions and modified summary are shown, with other sections disabled expect(actions.classList).not.toContain('hidden'); expect(modified_summary.textContent).toBe('modified_summary, {"count":1}'); + expect(disable1.classList).toContain('disabled-section'); + expect(disable2.classList).toContain('disabled-section'); // Record 1: Second field changed input1b.value = 'updated1b'; @@ -102,6 +110,8 @@ describe("BulkFormController", () => { expect(input2.classList).toContain('modified'); expect(actions.classList).not.toContain('hidden'); expect(modified_summary.textContent).toBe('modified_summary, {"count":1}'); + expect(disable1.classList).toContain('disabled-section'); + expect(disable2.classList).toContain('disabled-section'); // Record 2: Change back to original value input2.value = 'initial2'; @@ -110,8 +120,34 @@ describe("BulkFormController", () => { expect(input1a.classList).not.toContain('modified'); expect(input1b.classList).not.toContain('modified'); expect(input2.classList).not.toContain('modified'); + // Actions are hidden and other sections are now re-enabled expect(actions.classList).toContain('hidden'); expect(modified_summary.textContent).toBe('modified_summary, {"count":0}'); + expect(disable1.classList).not.toContain('disabled-section'); + expect(disable2.classList).not.toContain('disabled-section'); }); }); + + // unable to test disconnect at this stage + // describe("disconnect()", () => { + // it("resets other elements", () => { + // const disable1 = document.getElementById("disable1"); + // const disable2 = document.getElementById("disable2"); + // const controller = document.querySelector('[data-controller="bulk-form"]'); + // const form = document.querySelector('[data-controller="bulk-form"]'); + + // // Form is modified and other sections are disabled + // input1a.value = 'updated1a'; + // input1a.dispatchEvent(new Event("change")); + // expect(disable1.classList).toContain('disabled-section'); + // expect(disable2.classList).toContain('disabled-section'); + + // // form.submit(); //not implemented + // document.body.removeChild(controller); //doesn't trigger disconnect + // controller.innerHTML = ''; //doesn't trigger disconnect + + // expect(disable1.classList).not.toContain('disabled-section'); + // expect(disable2.classList).not.toContain('disabled-section'); + // }); + // }); });