Merge pull request #11509 from dacook/buu-editing-part3-11059

[BUU] Bulk form editing features

app/views/admin/products_v3/_content.html.haml

@@ -7,10 +7,10 @@
                                               category_options: category_options,
                                               category_id: category_id }
   - if products.any?
-    .container
+    .container.results
       .sixteen.columns
         = render partial: 'sort', locals: { pagy: pagy, search_term: search_term, producer_id: producer_id, category_id: category_id }
-    = render partial: 'table', locals: { products: products }
+        = render partial: 'table', locals: { products: products }
     - if pagy.pages > 1
       = render partial: 'admin/shared/v3/pagy', locals: { pagy: pagy, reflex: "click->Products#fetch" }
   - else

app/views/admin/products_v3/_table.html.haml

@@ -1,15 +1,15 @@
 = 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'} do |form|
-  %fieldset.form-actions
+            html: {'data-reflex-serialize-form': true, 'data-reflex': 'submit->products#bulk_update',
+                   '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
-        / = t('.products_modified', count: 'X')
+        .modified_summary{ 'data-bulk-form-target': "modifiedSummary", 'data-translation-key': 'admin.products_v3.table.modified_summary'}
         - if defined?(error_msg) && error_msg.present?
           .error
             = error_msg
       .form-buttons.six.columns
-        = form.submit t('.reset'), type: :reset, class: "medium"
+        = form.submit t('.reset'), type: :reset, class: "medium", 'data-reflex': 'click->products#fetch'
         = form.submit t('.save'), class: "medium"
   %table.products
     %col{ width:"15%" }
@@ -34,7 +34,7 @@
         %th.align-left= t('admin.products_page.columns.inherits_properties')
     - products.each do |product|
       = form.fields_for("products", product, index: nil) do |product_form|
-        %tbody.relaxed
+        %tbody.relaxed{ 'data-record-id': product_form.object.id }
           %tr
             %td.align-left.header
               = product_form.hidden_field :id

app/webpacker/controllers/bulk_form_controller.js

@@ -0,0 +1,90 @@
+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() {
+    this.form = this.element;
+
+    // Start listening for any changes within the form
+    // this.element.addEventListener('change', this.toggleModified.bind(this)); // dunno why this doesn't work
+    for (const element of this.form.elements) {
+      element.addEventListener("keyup", this.toggleModified.bind(this)); // instant response
+      element.addEventListener("change", this.toggleModified.bind(this)); // just in case (eg right-click paste)
+
+      // 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.
+      const recordId = recordContainer && recordContainer.dataset.recordId;
+      if (recordId) {
+        this.recordElements[recordId] ||= [];
+        this.recordElements[recordId].push(element);
+      }
+    }
+  }
+
+  disconnect() {
+    // Make sure to clean up anything that happened outside
+    this.#disableOtherElements(false);
+    window.removeEventListener("beforeunload", this.preventLeavingBulkForm);
+  }
+
+  toggleModified(e) {
+    const element = e.target;
+    const modified = element.value != element.defaultValue;
+    element.classList.toggle("modified", modified);
+
+    this.toggleFormModified();
+  }
+
+  toggleFormModified() {
+    // For each record, check if any fields are modified
+    const modifiedRecordCount = Object.keys(this.recordElements).filter((recordId) => {
+      return this.recordElements[recordId].some((element) => {
+        return element.value != element.defaultValue;
+      });
+    }).length;
+    const formModified = 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;
+    if (key) {
+      this.modifiedSummaryTarget.textContent = I18n.t(key, { count: modifiedRecordCount });
+    }
+
+    // Prevent accidental data loss
+    if (formModified) {
+      window.addEventListener("beforeunload", this.preventLeavingBulkForm);
+    } else {
+      window.removeEventListener("beforeunload", this.preventLeavingBulkForm);
+    }
+  }
+
+  preventLeavingBulkForm(e) {
+    // Cancel the event
+    e.preventDefault();
+    // Chrome requires returnValue to be set. Other browsers may display this if provided, but let's
+    // not create a new translation key, and keep the behaviour consistent.
+    e.returnValue = "";
+  }
+
+  // private
+
+  #disableOtherElements(disable) {
+    this.disableElements ||= document.querySelectorAll(this.disableSelectorValue);
+
+    if (this.disableElements) {
+      this.disableElements.forEach((element) => {
+        element.classList.toggle("disabled-section", disable);
+      });
+    }
+  }
+}

app/webpacker/css/admin/products_v3.scss

@@ -15,6 +15,19 @@
     padding: 0;
   }
 
+  .results {
+    position: relative;
+  }
+
+  // Form actions floats over other controls when active
+  .form-actions {
+    position: absolute;
+    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).
   table.products {
     table-layout: fixed; // Column widths are based solely on col definitions (not content). This allows more efficient rendering.
@@ -100,6 +113,10 @@
       &:focus {
         border-color: $color-txt-hover-brd;
       }
+
+      &.modified {
+        border-color: $color-txt-modified-brd;
+      }
     }
 
     .field_with_errors {
@@ -218,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
+    }
+  }
 }

app/webpacker/css/admin_v3/globals/variables.scss

@@ -74,6 +74,7 @@ $color-sel-hover-bg:             $lighter-grey !default;
 $color-txt-brd:                  $color-border !default;
 $color-txt-text:                 $near-black !default;
 $color-txt-hover-brd:            $teal !default;
+$color-txt-modified-brd:         $bright-orange !default;
 $vpadding-txt:                   5px;
 $hpadding-txt:                   8px;
 

app/webpacker/css/admin_v3/shared/forms.scss

@@ -30,6 +30,10 @@ fieldset {
   &[disabled] {
     opacity: 0.7;
   }
+
+  &.modified {
+    border-color: $color-txt-modified-brd;
+  }
 }
 
 textarea {

config/locales/en.yml

@@ -818,6 +818,10 @@ en:
         import_products: Import multiple products
         no_products_found_for_search: No products found for your search criteria
       table:
+        modified_summary:
+          zero: ""
+          one: "%{count} product modified."
+          other: "%{count} products modified."
         save: Save changes
         reset: Discard changes
       bulk_update: # TODO: fix these

spec/javascripts/stimulus/bulk_form_controller_test.js

@@ -0,0 +1,188 @@
+/**
+ * @jest-environment jsdom
+ */
+
+import { Application } from "stimulus";
+import bulk_form_controller from "../../../app/webpacker/controllers/bulk_form_controller";
+
+describe("BulkFormController", () => {
+  beforeAll(() => {
+    const application = Application.start();
+    application.register("bulk-form", bulk_form_controller);
+  });
+
+  // Mock I18n. TODO: moved to a shared helper
+  beforeAll(() => {
+    const mockedT = jest.fn();
+    mockedT.mockImplementation((string, opts) => (string + ', ' + JSON.stringify(opts)));
+
+    global.I18n =  {
+      t: mockedT
+    };
+  })
+
+  // (jest still doesn't have aroundEach https://github.com/jestjs/jest/issues/4543 )
+  afterAll(() => {
+    delete global.I18n;
+  })
+
+  beforeEach(() => {
+    document.body.innerHTML = `
+      <div id="disable1"></div>
+      <div id="disable2"></div>
+      <form data-controller="bulk-form" data-bulk-form-disable-selector-value="#disable1,#disable2">
+        <div id="actions" data-bulk-form-target="actions" class="hidden"></div>
+        <div id="modified_summary" data-bulk-form-target="modifiedSummary" data-translation-key="modified_summary"></div>
+        <div data-record-id="1">
+          <input id="input1a" type="text" value="initial1a">
+          <input id="input1b" type="text" value="initial1b">
+        </div>
+        <div data-record-id="2">
+          <input id="input2" type="text" value="initial2">
+        </div>
+        <input type="submit">
+      </form>
+    `;
+  });
+
+  describe("Modifying input values", () => {
+    beforeEach(() => {
+      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");
+      const input1b = document.getElementById("input1b");
+      const input2 = document.getElementById("input2");
+    });
+
+    describe("marking changed fields", () => {
+      it("onChange", () => {
+        input1a.value = 'updated1a';
+        input1a.dispatchEvent(new Event("change"));
+        // Expect only first field to show modified
+        expect(input1a.classList).toContain('modified');
+        expect(input1b.classList).not.toContain('modified');
+        expect(input2.classList).not.toContain('modified');
+
+        // Change back to original value
+        input1a.value = 'initial1a';
+        input1a.dispatchEvent(new Event("change"));
+        expect(input1a.classList).not.toContain('modified');
+
+      });
+
+      it("onKeyup", () => {
+        input1a.value = 'u1a';
+        input1a.dispatchEvent(new Event("keyup"));
+        // Expect only first field to show modified
+        expect(input1a.classList).toContain('modified');
+        expect(input1b.classList).not.toContain('modified');
+        expect(input2.classList).not.toContain('modified');
+
+        // Change back to original value
+        input1a.value = 'initial1a';
+        input1a.dispatchEvent(new Event("keyup"));
+        expect(input1a.classList).not.toContain('modified');
+      });
+
+      it("multiple fields", () => {
+        input1a.value = 'updated1a';
+        input1a.dispatchEvent(new Event("change"));
+        input2.value = 'updated2';
+        input2.dispatchEvent(new Event("change"));
+        // Expect only first field to show modified
+        expect(input1a.classList).toContain('modified');
+        expect(input1b.classList).not.toContain('modified');
+        expect(input2.classList).toContain('modified');
+
+        // Change only one back to original value
+        input1a.value = 'initial1a';
+        input1a.dispatchEvent(new Event("change"));
+        expect(input1a.classList).not.toContain('modified');
+        expect(input1b.classList).not.toContain('modified');
+        expect(input2.classList).toContain('modified');
+      });
+    })
+
+    describe("activating sections, and showing a summary", () => {
+      // This scenario should probably be broken up into smaller units.
+      it("counts modified records ", () => {
+        // Record 1: First field changed
+        input1a.value = 'updated1a';
+        input1a.dispatchEvent(new Event("change"));
+        // 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';
+        input1b.dispatchEvent(new Event("change"));
+        // Expect to show same summary translation
+        expect(actions.classList).not.toContain('hidden');
+        expect(modified_summary.textContent).toBe('modified_summary, {"count":1}');
+
+        // Record 2: has been changed
+        input2.value = 'updated2';
+        input2.dispatchEvent(new Event("change"));
+        // Expect summary to count both records
+        expect(actions.classList).not.toContain('hidden');
+        expect(modified_summary.textContent).toBe('modified_summary, {"count":2}');
+
+        // Record 1: Change first field back to original value
+        input1a.value = 'initial1a';
+        input1a.dispatchEvent(new Event("change"));
+        // Both records are still modified.
+        expect(input1a.classList).not.toContain('modified');
+        expect(input1b.classList).toContain('modified');
+        expect(input2.classList).toContain('modified');
+        expect(actions.classList).not.toContain('hidden');
+        expect(modified_summary.textContent).toBe('modified_summary, {"count":2}');
+
+        // Record 1: Change second field back to original value
+        input1b.value = 'initial1b';
+        input1b.dispatchEvent(new Event("change"));
+        // Both fields for record 1 show unmodified, but second record is still 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';
+        input2.dispatchEvent(new Event("change"));
+        // 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');
+  //     //TODO: expect window to have no beforeunload event listener
+  //   });
+  // });
+});

spec/system/admin/products_v3/products_spec.rb

@@ -205,6 +205,33 @@ describe 'As an admin, I can see the new product page' do
       pending
       expect(page).to have_content "Changes saved"
     end
+
+    it "can discard changes and reload latest data" do
+      within row_containing_name("Apples") do
+        fill_in "Name", with: "Pommes"
+      end
+
+      # Expect to be alerted when attempting to navigate away. Cancel.
+      dismiss_confirm do
+        click_link "Dashboard"
+      end
+      within row_containing_name("Apples") do
+        expect(page).to have_field "Name", with: "Pommes" # Changed value wasn't lost
+      end
+
+      # Meanwhile, the SKU was updated
+      product_a.update! sku: "APL-10"
+
+      expect {
+        click_button "Discard changes"
+        product_a.reload
+      }.to_not change { product_a.name }
+
+      within row_containing_name("Apples") do
+        expect(page).to have_field "Name", with: "Apples" # Changed value wasn't saved
+        expect(page).to have_field "SKU", with: "APL-10" # Updated value shown
+      end
+    end
   end
 
   def expect_page_to_be(page_number)