mirror of
https://github.com/openfoodfoundation/openfoodnetwork
synced 2026-02-15 23:57:48 +00:00
When dropdown fields don't allow blank, but are blank, show as changed
They were already counted as changed by the javascript, but didn't have a 'changed' class to indicate it. The reason they are 'changed', is because the dropdown has no blank option, and is forced to select the first item in the list. This is purely to cover the case of invalid data, but should help a lot when debugging data issues. I don't think it's any less efficient, because the extra 'classList.toggle' calls don't do anything on unchanged fields.
This commit is contained in:
@@ -63,7 +63,7 @@ export default class BulkFormController extends Controller {
|
||||
// For each record, check if any fields are changed
|
||||
// TODO: optimise basd on current state. if field is changed, but form already changed, no need to update (and vice versa)
|
||||
const changedRecordCount = Object.values(this.recordElements).filter((elements) =>
|
||||
elements.some(this.#isChanged),
|
||||
elements.some(this.#checkIsChanged.bind(this)),
|
||||
).length;
|
||||
this.formChanged = changedRecordCount > 0 || this.errorValue;
|
||||
|
||||
@@ -131,11 +131,17 @@ export default class BulkFormController extends Controller {
|
||||
});
|
||||
}
|
||||
|
||||
#isChanged(element) {
|
||||
if(!element.isConnected) {
|
||||
return false;
|
||||
// Check if changed, and mark with class if it is.
|
||||
#checkIsChanged(element) {
|
||||
if(!element.isConnected) return false;
|
||||
|
||||
} else if (element.type == "checkbox") {
|
||||
const changed = this.#isChanged(element);
|
||||
element.classList.toggle("changed", changed);
|
||||
return changed;
|
||||
}
|
||||
|
||||
#isChanged(element) {
|
||||
if (element.type == "checkbox") {
|
||||
return element.defaultChecked !== undefined && element.checked != element.defaultChecked;
|
||||
|
||||
} else if (element.type == "select-one") {
|
||||
|
||||
@@ -99,6 +99,59 @@ describe("BulkFormController", () => {
|
||||
expect(input1b.classList).not.toContain('changed');
|
||||
expect(input2.classList).toContain('changed');
|
||||
});
|
||||
|
||||
describe("select not include_blank", () => {
|
||||
beforeEach(() => {
|
||||
document.body.innerHTML = `
|
||||
<form data-controller="bulk-form" data-bulk-form-disable-selector-value="#disable1,#disable2">
|
||||
<div data-record-id="1">
|
||||
<select id="select1">
|
||||
<option value="1">one</option>
|
||||
<option value="2">two</option>
|
||||
</select>
|
||||
</div>
|
||||
<input type="submit">
|
||||
</form>
|
||||
`;
|
||||
});
|
||||
|
||||
it("shows as changed", () => {
|
||||
// Expect select to show changed (select-one always has something selected)
|
||||
expect(select1.classList).toContain('changed');
|
||||
|
||||
// Change selection
|
||||
select1.options[0].selected = false;
|
||||
select1.options[1].selected = true;
|
||||
select1.dispatchEvent(new Event("input"));
|
||||
expect(select1.classList).toContain('changed');
|
||||
});
|
||||
});
|
||||
|
||||
describe("select-one with include_blank", () => {
|
||||
beforeEach(() => {
|
||||
document.body.innerHTML = `
|
||||
<form data-controller="bulk-form" data-bulk-form-disable-selector-value="#disable1,#disable2">
|
||||
<div data-record-id="1">
|
||||
<select id="select1">
|
||||
<option value="">blank</option>
|
||||
<option value="1">one</option>
|
||||
</select>
|
||||
</div>
|
||||
<input type="submit">
|
||||
</form>
|
||||
`;
|
||||
});
|
||||
|
||||
it("does not show as changed", () => {
|
||||
expect(select1.classList).not.toContain('changed');
|
||||
|
||||
// Change selection
|
||||
select1.options[0].selected = false;
|
||||
select1.options[1].selected = true;
|
||||
select1.dispatchEvent(new Event("input"));
|
||||
expect(select1.classList).toContain('changed');
|
||||
});
|
||||
});
|
||||
})
|
||||
|
||||
describe("activating sections, and showing a summary", () => {
|
||||
|
||||
Reference in New Issue
Block a user