raggedstaff/openfoodnetwork
1
0
Fork 0
mirror of https://github.com/openfoodfoundation/openfoodnetwork synced 2026-02-02 21:57:17 +00:00
Code Issues Packages Projects Releases Wiki Activity

Rename 'modified' state to 'changed'

Browse Source 
This conveniently matches the terminology used in both JavaScript and ActiveModel::Dirty.
This commit is contained in:
David Cook
2023-10-24 14:49:04 +11:00
parent ceb9d9af92
commit fdad45bb46
7 changed files with 60 additions and 60 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
app/views/admin/products_v3/_table.html.haml
View File

@@ -4,7 +4,7 @@
%fieldset.form-actions.hidden{ 'data-bulk-form-target': "actions" }
.container
.status.ten.columns
.modified_summary{ 'data-bulk-form-target': "modifiedSummary", 'data-translation-key': 'admin.products_v3.table.modified_summary'}
.changed_summary{ 'data-bulk-form-target': "changedSummary", 'data-translation-key': 'admin.products_v3.table.changed_summary'}
- if defined?(error_msg) && error_msg.present?
.error
= error_msg

40
app/webpacker/controllers/bulk_form_controller.js
View File

@@ -1,8 +1,8 @@
import { Controller } from "stimulus";
// Manages "modified" state for a form with multiple records
// Manages "changed" state for a form with multiple records
export default class BulkFormController extends Controller {
static targets = ["actions", "modifiedSummary"];
static targets = ["actions", "changedSummary"];
static values = {
disableSelector: String,
};
@@ -12,10 +12,10 @@ export default class BulkFormController extends Controller {
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
// 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.toggleModified.bind(this)); // instant response
element.addEventListener("change", this.toggleModified.bind(this)); // just in case (eg right-click paste)
element.addEventListener("keyup", this.toggleChanged.bind(this)); // instant response
element.addEventListener("change", this.toggleChanged.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.
@@ -33,32 +33,32 @@ export default class BulkFormController extends Controller {
window.removeEventListener("beforeunload", this.preventLeavingBulkForm);
}
toggleModified(e) {
toggleChanged(e) {
const element = e.target;
element.classList.toggle("modified", this.#isModified(element));
element.classList.toggle("changed", this.#isChanged(element));
this.toggleFormModified();
this.toggleFormChanged();
}
toggleFormModified() {
// For each record, check if any fields are modified
const modifiedRecordCount = Object.values(this.recordElements).filter((elements) =>
elements.some(this.#isModified)
toggleFormChanged() {
// For each record, check if any fields are changed
const changedRecordCount = Object.values(this.recordElements).filter((elements) =>
elements.some(this.#isChanged)
).length;
const formModified = modifiedRecordCount > 0;
const formChanged = changedRecordCount > 0;
// Show actions
this.actionsTarget.classList.toggle("hidden", !formModified);
this.#disableOtherElements(formModified); // like filters and sorting
this.actionsTarget.classList.toggle("hidden", !formChanged);
this.#disableOtherElements(formChanged); // like filters and sorting
// Display number of records modified
const key = this.modifiedSummaryTarget && this.modifiedSummaryTarget.dataset.translationKey;
// Display number of records changed
const key = this.changedSummaryTarget && this.changedSummaryTarget.dataset.translationKey;
if (key) {
this.modifiedSummaryTarget.textContent = I18n.t(key, { count: modifiedRecordCount });
this.changedSummaryTarget.textContent = I18n.t(key, { count: changedRecordCount });
}
// Prevent accidental data loss
if (formModified) {
if (formChanged) {
window.addEventListener("beforeunload", this.preventLeavingBulkForm);
} else {
window.removeEventListener("beforeunload", this.preventLeavingBulkForm);
@@ -85,7 +85,7 @@ export default class BulkFormController extends Controller {
}
}
#isModified(element) {
#isChanged(element) {
return element.value != element.defaultValue;
}
}

4
app/webpacker/css/admin/products_v3.scss
View File

@@ -126,8 +126,8 @@
border-color: $color-txt-hover-brd;
}
&.modified {
border-color: $color-txt-modified-brd;
&.changed {
border-color: $color-txt-changed-brd;
}
}

2
app/webpacker/css/admin_v3/globals/variables.scss
View File

@@ -74,7 +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;
$color-txt-changed-brd: $bright-orange !default;
$vpadding-txt: 5px;
$hpadding-txt: 8px;

4
app/webpacker/css/admin_v3/shared/forms.scss
View File

@@ -31,8 +31,8 @@ fieldset {
opacity: 0.7;
}
&.modified {
border-color: $color-txt-modified-brd;
&.changed {
border-color: $color-txt-changed-brd;
}
}

2
config/locales/en.yml
View File

@@ -833,7 +833,7 @@ en:
import_products: Import multiple products
no_products_found_for_search: No products found for your search criteria
table:
modified_summary:
changed_summary:
zero: ""
one: "%{count} product modified."
other: "%{count} products modified."

66
spec/javascripts/stimulus/bulk_form_controller_test.js
View File

@@ -32,7 +32,7 @@ describe("BulkFormController", () => {
<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 id="changed_summary" data-bulk-form-target="changedSummary" data-translation-key="changed_summary"></div>
<div data-record-id="1">
<input id="input1a" type="text" value="initial1a">
<input id="input1b" type="text" value="initial1b">
@@ -50,7 +50,7 @@ describe("BulkFormController", () => {
const disable1 = document.getElementById("disable1");
const disable2 = document.getElementById("disable2");
const actions = document.getElementById("actions");
const modified_summary = document.getElementById("modified_summary");
const changed_summary = document.getElementById("changed_summary");
const input1a = document.getElementById("input1a");
const input1b = document.getElementById("input1b");
const input2 = document.getElementById("input2");
@@ -60,30 +60,30 @@ describe("BulkFormController", () => {
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');
// 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"));
expect(input1a.classList).not.toContain('modified');
expect(input1a.classList).not.toContain('changed');
});
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');
// 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('modified');
expect(input1a.classList).not.toContain('changed');
});
it("multiple fields", () => {
@@ -91,29 +91,29 @@ describe("BulkFormController", () => {
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');
// 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"));
expect(input1a.classList).not.toContain('modified');
expect(input1b.classList).not.toContain('modified');
expect(input2.classList).toContain('modified');
expect(input1a.classList).not.toContain('changed');
expect(input1b.classList).not.toContain('changed');
expect(input2.classList).toContain('changed');
});
})
describe("activating sections, and showing a summary", () => {
// This scenario should probably be broken up into smaller units.
it("counts modified records ", () => {
it("counts changed records ", () => {
// Record 1: First field changed
input1a.value = 'updated1a';
input1a.dispatchEvent(new Event("change"));
// Actions and modified summary are shown, with other sections disabled
// Actions and changed summary are shown, with other sections disabled
expect(actions.classList).not.toContain('hidden');
expect(modified_summary.textContent).toBe('modified_summary, {"count":1}');
expect(changed_summary.textContent).toBe('changed_summary, {"count":1}');
expect(disable1.classList).toContain('disabled-section');
expect(disable2.classList).toContain('disabled-section');
@@ -122,31 +122,31 @@ describe("BulkFormController", () => {
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}');
expect(changed_summary.textContent).toBe('changed_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}');
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"));
// Both records are still modified.
expect(input1a.classList).not.toContain('modified');
expect(input1b.classList).toContain('modified');
expect(input2.classList).toContain('modified');
// Both records are still changed.
expect(input1a.classList).not.toContain('changed');
expect(input1b.classList).toContain('changed');
expect(input2.classList).toContain('changed');
expect(actions.classList).not.toContain('hidden');
expect(modified_summary.textContent).toBe('modified_summary, {"count":2}');
expect(changed_summary.textContent).toBe('changed_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
// Both fields for record 1 show unchanged, but second record is still changed
expect(actions.classList).not.toContain('hidden');
expect(modified_summary.textContent).toBe('modified_summary, {"count":1}');
expect(changed_summary.textContent).toBe('changed_summary, {"count":1}');
expect(disable1.classList).toContain('disabled-section');
expect(disable2.classList).toContain('disabled-section');
@@ -155,7 +155,7 @@ describe("BulkFormController", () => {
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(changed_summary.textContent).toBe('changed_summary, {"count":0}');
expect(disable1.classList).not.toContain('disabled-section');
expect(disable2.classList).not.toContain('disabled-section');
});
@@ -170,7 +170,7 @@ describe("BulkFormController", () => {
// const controller = document.querySelector('[data-controller="bulk-form"]');
// const form = document.querySelector('[data-controller="bulk-form"]');
// // Form is modified and other sections are disabled
// // Form is changed and other sections are disabled
// input1a.value = 'updated1a';
// input1a.dispatchEvent(new Event("change"));
// expect(disable1.classList).toContain('disabled-section');