bug #221 [LiveComponent] Fix expanded choices and checkboxes in forms (Lustmored)

This PR was squashed before being merged into the 2.x branch.

Discussion
----------

[LiveComponent] Fix expanded choices and checkboxes in forms

| Q             | A
| ------------- | ---
| Bug fix?      | yes
| New feature?  | no
| Tickets       |
| License       | MIT

When using live component with forms that contains expanded choices list (rendered as a list of radio buttons) extracting form values returns array of nulls (one per choice), while frontend expects single value.

Therefore after calling first `update` and then trying to call update on expanded field controller throws (`The property used in data-model="${propertyPath}" was never initialized. Did you forget to add exposed={"${lastPart}"} to its LiveProp?`).

This simple change fixes the issue. I have also added simple test case for a few basic form types.

Commits
-------

7605821 Code Style
2de0cbf Rebuild dist file
d96d82f Adapt to changes in component
2f735a2 Simplify form functional test
eb92ebd Use $initialData instead of 'data' in FormType to make tests easier to follow
33dabc0 fetchMock.done() is not needed anymore
857ff1f Return early and throw on unsupported element
bd05418 Renames and comments
f7dbdc1 Add description to updateArrayDataFromChangedElement
d195ee9 Rename getArrayValue
914bcd5 Spacing issues
f13a498 Merge branch '2.x' into fix-expanded-choices
a636a71 Add simple select multiple test to functional test
4c8336b Add select multiple to backend form test
d00cb41 Rebuild dist/
3204490 getArrayValues should return empty array instead of null to mimic backend behavior
f87796d Rebuild `dist/`
56caf14 Fix last tests
e51f202 Add tests for checkbox and select multiple cases
f342a7a Introduce getArrayValue with unit tests
7580a73 Add test for normalizeModelName and explain change within
a194f0c After merge cleanup and tests fixing
18741bd Merge branch '2.x' into fix-expanded-choices
a5f56f2 Other attempt at tests
ffd6b72 Code style
296f9a4 misstype
6d41d90 Try to make JS tests work
1e31c30 Move array handling into _updateModelFromElement
d183645 Add backend web test case for checkboxes
66f85e8 Rework FormComponent1 to use form type and register it in test kernel.
546c5a7 Don't pass values to extractFormValues
71f49ae Rebuild dist assets
0f4e70e Merge branch '2.x' into fix-expanded-choices
a17870f Resolve conflicts
2f1d0ee Handle collections of checkboxes as data arrays
255d9f9 Add choice_multiple to test and fix uninitialized value
cfbbf9d Set data to null on unchecked elements (checkboxes)
f5e34fa Always propagate form null values to formValues
cab4c4a Rename test component to avoid possible conflicts
841e07f Simplify if clause and fix assert arguments order
fc953dd Code style
4a37d10 Fix form values for ChoiceType with expanded: true
76de2a2 Add basic test for formValues()
e76bdb9 Add symfony/form to dev dependencies

Author: weaverryan

src/LiveComponent/assets/dist/live_controller.js

@@ -898,14 +898,23 @@ function combineSpacedArray(parts) {
     return finalParts;
 }
 
-function setDeepData(data, propertyPath, value) {
+function parseDeepData(data, propertyPath) {
     const finalData = JSON.parse(JSON.stringify(data));
     let currentLevelData = finalData;
     const parts = propertyPath.split('.');
     for (let i = 0; i < parts.length - 1; i++) {
         currentLevelData = currentLevelData[parts[i]];
     }
     const finalKey = parts[parts.length - 1];
+    return {
+        currentLevelData,
+        finalData,
+        finalKey,
+        parts
+    };
+}
+function setDeepData(data, propertyPath, value) {
+    const { currentLevelData, finalData, finalKey, parts } = parseDeepData(data, propertyPath);
     if (typeof currentLevelData !== 'object') {
         const lastPart = parts.pop();
         throw new Error(`Cannot set data-model="${propertyPath}". The parent "${parts.join('.')}" data does not appear to be an object (it's "${currentLevelData}"). Did you forget to add exposed={"${lastPart}"} to its LiveProp?`);
@@ -928,10 +937,12 @@ function doesDeepPropertyExist(data, propertyPath) {
 }
 function normalizeModelName(model) {
     return model
+        .replace(/\[]$/, '')
         .split('[')
         .map(function (s) {
         return s.replace(']', '');
-    }).join('.');
+    })
+        .join('.');
 }
 
 function haveRenderedValuesChanged(originalDataJson, currentDataJson, newDataJson) {
@@ -979,6 +990,30 @@ function cloneHTMLElement(element) {
     return newElement;
 }
 
+function updateArrayDataFromChangedElement(element, value, currentValues) {
+    if (!(currentValues instanceof Array)) {
+        currentValues = [];
+    }
+    if (element instanceof HTMLInputElement && element.type === 'checkbox') {
+        const index = currentValues.indexOf(value);
+        if (element.checked) {
+            if (index === -1) {
+                currentValues.push(value);
+            }
+            return currentValues;
+        }
+        if (index > -1) {
+            currentValues.splice(index, 1);
+        }
+        return currentValues;
+    }
+    if (element instanceof HTMLSelectElement) {
+        currentValues = Array.from(element.selectedOptions).map(el => el.value);
+        return currentValues;
+    }
+    throw new Error(`The element used to determine array data from is unsupported (${element.tagName} provided)`);
+}
+
 const DEFAULT_DEBOUNCE = 150;
 class default_1 extends Controller {
     constructor() {
@@ -1022,12 +1057,10 @@ class default_1 extends Controller {
         window.removeEventListener('beforeunload', this.markAsWindowUnloaded);
     }
     update(event) {
-        const value = this._getValueFromElement(event.target);
-        this._updateModelFromElement(event.target, value, true);
+        this._updateModelFromElement(event.target, this._getValueFromElement(event.target), true);
     }
     updateDefer(event) {
-        const value = this._getValueFromElement(event.target);
-        this._updateModelFromElement(event.target, value, false);
+        this._updateModelFromElement(event.target, this._getValueFromElement(event.target), false);
     }
     action(event) {
         const rawAction = event.currentTarget.dataset.actionName;
@@ -1088,7 +1121,17 @@ class default_1 extends Controller {
             const clonedElement = cloneHTMLElement(element);
             throw new Error(`The update() method could not be called for "${clonedElement.outerHTML}": the element must either have a "data-model" or "name" attribute set to the model name.`);
         }
-        this.$updateModel(model, value, shouldRender, element.hasAttribute('name') ? element.getAttribute('name') : null);
+        if (/\[]$/.test(model)) {
+            const { currentLevelData, finalKey } = parseDeepData(this.dataValue, normalizeModelName(model));
+            const currentValue = currentLevelData[finalKey];
+            value = updateArrayDataFromChangedElement(element, value, currentValue);
+        }
+        else if (element instanceof HTMLInputElement
+            && element.type === 'checkbox'
+            && !element.checked) {
+            value = null;
+        }
+        this.$updateModel(model, value, shouldRender, element.hasAttribute('name') ? element.getAttribute('name') : null, {});
     }
     $updateModel(model, value, shouldRender = true, extraModelName = null, options = {}) {
         const directives = parseDirectives(model);
@@ -1112,7 +1155,7 @@ class default_1 extends Controller {
             this._dispatchEvent('live:update-model', {
                 modelName,
                 extraModelName: normalizedExtraModelName,
-                value,
+                value
             });
         }
         this.dataValue = setDeepData(this.dataValue, modelName, value);

src/LiveComponent/assets/src/live_controller.ts

@@ -2,10 +2,11 @@ import { Controller } from '@hotwired/stimulus';
 import morphdom from 'morphdom';
 import { parseDirectives, Directive } from './directives_parser';
 import { combineSpacedArray } from './string_utils';
-import { setDeepData, doesDeepPropertyExist, normalizeModelName } from './set_deep_data';
+import { setDeepData, doesDeepPropertyExist, normalizeModelName, parseDeepData } from './set_deep_data';
 import { haveRenderedValuesChanged } from './have_rendered_values_changed';
 import { normalizeAttributesForComparison } from './normalize_attributes_for_comparison';
 import { cloneHTMLElement } from './clone_html_element';
+import { updateArrayDataFromChangedElement } from "./update_array_data";
 
 interface ElementLoadingDirectives {
     element: HTMLElement|SVGElement,
@@ -105,15 +106,11 @@ export default class extends Controller {
      * Called to update one piece of the model
      */
     update(event: any) {
-        const value = this._getValueFromElement(event.target);
-
-        this._updateModelFromElement(event.target, value, true);
+        this._updateModelFromElement(event.target, this._getValueFromElement(event.target), true);
     }
 
     updateDefer(event: any) {
-        const value = this._getValueFromElement(event.target);
-
-        this._updateModelFromElement(event.target, value, false);
+        this._updateModelFromElement(event.target, this._getValueFromElement(event.target), false);
     }
 
     action(event: any) {
@@ -194,11 +191,10 @@ export default class extends Controller {
         return element.dataset.value || (element as any).value;
     }
 
-    _updateModelFromElement(element: Element, value: string, shouldRender: boolean) {
+    _updateModelFromElement(element: Element, value: string|null, shouldRender: boolean) {
         if (!(element instanceof HTMLElement)) {
             throw new Error('Could not update model for non HTMLElement');
         }
-
         const model = element.dataset.model || element.getAttribute('name');
 
         if (!model) {
@@ -207,7 +203,25 @@ export default class extends Controller {
             throw new Error(`The update() method could not be called for "${clonedElement.outerHTML}": the element must either have a "data-model" or "name" attribute set to the model name.`);
         }
 
-        this.$updateModel(model, value, shouldRender, element.hasAttribute('name') ? element.getAttribute('name') : null);
+        // HTML form elements with name ending with [] require array as data
+        // we need to handle addition and removal of values from it to send
+        // back only required data
+        if (/\[]$/.test(model)) {
+            // Get current value from data
+            const { currentLevelData, finalKey } = parseDeepData(this.dataValue, normalizeModelName(model))
+            const currentValue = currentLevelData[finalKey];
+
+            value = updateArrayDataFromChangedElement(element, value, currentValue);
+        } else if (
+            element instanceof HTMLInputElement
+            && element.type === 'checkbox'
+            && !element.checked
+        ) {
+            // Unchecked checkboxes in a single value scenarios should map to `null`
+            value = null;
+        }
+
+        this.$updateModel(model, value, shouldRender, element.hasAttribute('name') ? element.getAttribute('name') : null, {});
     }
 
     /**
@@ -257,7 +271,7 @@ export default class extends Controller {
             this._dispatchEvent('live:update-model', {
                 modelName,
                 extraModelName: normalizedExtraModelName,
-                value,
+                value
             });
         }
 

src/LiveComponent/assets/src/set_deep_data.ts

@@ -1,6 +1,5 @@
 // post.user.username
-export function setDeepData(data, propertyPath, value) {
-    // cheap way to deep clone simple data
+export function parseDeepData(data, propertyPath) {
     const finalData = JSON.parse(JSON.stringify(data));
 
     let currentLevelData = finalData;
@@ -14,6 +13,18 @@ export function setDeepData(data, propertyPath, value) {
     // now finally change the key on that deeper object
     const finalKey = parts[parts.length - 1];
 
+    return {
+        currentLevelData,
+        finalData,
+        finalKey,
+        parts
+    }
+}
+
+// post.user.username
+export function setDeepData(data, propertyPath, value) {
+    const { currentLevelData, finalData, finalKey, parts } = parseDeepData(data, propertyPath)
+
     // make sure the currentLevelData is an object, not a scalar
     // if it is, it means the initial data didn't know that sub-properties
     // could be exposed. Or, you're just trying to set some deep
@@ -64,9 +75,14 @@ export function doesDeepPropertyExist(data, propertyPath) {
  */
 export function normalizeModelName(model) {
     return model
+        // Names ending in "[]" represent arrays in HTML.
+        // To get normalized name we need to ignore this part.
+        // For example: "user[mailing][]" becomes "user.mailing" (and has array typed value)
+        .replace(/\[]$/, '')
         .split('[')
         // ['object', 'foo', 'bar', 'ya']
         .map(function (s) {
             return s.replace(']', '')
-        }).join('.')
+        })
+        .join('.')
 }

src/LiveComponent/assets/src/update_array_data.ts

@@ -0,0 +1,51 @@
+/**
+ * Adds or removes a key from an array element based on an array element.
+ *
+ * Given an "array" element (e.g. )
+ * and the current data for "preferences" (e.g. ["text", "phone"]), this function will add or
+ * remove the value (e.g. email) from that array (based on if the element (un)checked) and
+ * return the final, updated array (e.g. ["text", "phone", "email"]).
+ *
+ * @param element       Current HTML element
+ * @param value         The value that should be set or removed from currentValues
+ * @param currentValues  Current data value
+ */
+export function updateArrayDataFromChangedElement(
+    element: HTMLElement,
+    value: string|null,
+    currentValues: any
+): Array<string> {
+    // Enforce returned value is an array
+    if (!(currentValues instanceof Array)) {
+        currentValues = [];
+    }
+
+    if (element instanceof HTMLInputElement && element.type === 'checkbox') {
+        const index = currentValues.indexOf(value);
+
+        if (element.checked) {
+            // Add value to an array if it's not in it already
+            if (index === -1) {
+                currentValues.push(value);
+            }
+
+            return currentValues;
+        }
+
+        // Remove value from an array
+        if (index > -1) {
+            currentValues.splice(index, 1);
+        }
+
+        return currentValues;
+    }
+
+    if (element instanceof HTMLSelectElement) {
+        // Select elements with `multiple` option require mapping chosen options to their values
+        currentValues = Array.from(element.selectedOptions).map(el => el.value);
+
+        return currentValues;
+    }
+
+    throw new Error(`The element used to determine array data from is unsupported (${element.tagName} provided)`);
+}