Skip to content

Commit

Permalink
bug #1155 Handle array-like objects when working with checkboxes (Lus…
Browse files Browse the repository at this point in the history
…tmored)

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

Discussion
----------

Handle array-like objects when working with checkboxes

| Q             | A
| ------------- | ---
| Bug fix?      | yes
| New feature?  | no <!-- please update src/**/CHANGELOG.md files -->
| Tickets       |
| License       | MIT

First of all please take a look at added test cases. I think it shows the purpose of this change clearly.

Background:
Recently when working with forms inside a component I ran into a corner case, where Symfony forms are setting a non-zero-indexed array for the properties of `ChoiceType` with both `multiple` and `expanded`. It breaks live-component model assumption that multiple checkboxes keep their value as an array, which in JavaScript is always zero-indexed.

It happens after a pretty weird set of steps:
- check any option (foo)
- check any other option (bar)
- check the third option (baz)
- uncheck option 'foo' <- Symfony forms store current value as `[1 => 'bar', 2 => 'baz']`
- uncheck option 'bar' <- at this point current implementation is treating checkboxes as a single value and send back `null` instead of `['baz']`.

This simple change fixes this flow (tested on a real life project). I don't see any correlation with failing tests.

Commits
-------

6b75580 Handle array-like objects when working with checkboxes
  • Loading branch information
weaverryan committed Oct 2, 2023
2 parents a760b37 + 6b75580 commit d341f7b
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 0 deletions.
3 changes: 3 additions & 0 deletions src/LiveComponent/assets/dist/live_controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,9 @@ function getValueFromElement(element, valueStore) {
if (Array.isArray(modelValue)) {
return getMultipleCheckboxValue(element, modelValue);
}
else if (Object(modelValue) === modelValue) {
return getMultipleCheckboxValue(element, Object.values(modelValue));
}
}
if (element.hasAttribute('value')) {
return element.checked ? element.getAttribute('value') : null;
Expand Down
4 changes: 4 additions & 0 deletions src/LiveComponent/assets/src/dom_utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,10 @@ export function getValueFromElement(element: HTMLElement, valueStore: ValueStore
const modelValue = valueStore.get(modelNameData.action);
if (Array.isArray(modelValue)) {
return getMultipleCheckboxValue(element, modelValue);
} else if (Object(modelValue) === modelValue) {
// we might get objects of values from forms, like {'1': 'foo', '2': 'bar'}
// this occurs in symfony forms with expanded ChoiceType when first checked options get unchecked
return getMultipleCheckboxValue(element, Object.values(modelValue));
}
}

Expand Down
5 changes: 5 additions & 0 deletions src/LiveComponent/assets/test/dom_utils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,9 @@ describe('getValueFromElement', () => {

expect(getValueFromElement(input, createStore({ foo: ['bar'] })))
.toEqual(['bar', 'the_checkbox_value']);

expect(getValueFromElement(input, createStore({ foo: {'1': 'bar'} })))
.toEqual(['bar', 'the_checkbox_value']);
});

it('Correctly removes data from valued unchecked checkbox', () => {
Expand All @@ -50,6 +53,8 @@ describe('getValueFromElement', () => {
.toEqual(['bar']);
expect(getValueFromElement(input, createStore({ foo: ['bar', 'the_checkbox_value'] })))
.toEqual(['bar']);
expect(getValueFromElement(input, createStore({ foo: {'1': 'bar', '2': 'the_checkbox_value'} })))
.toEqual(['bar']);
});

it('Correctly handles boolean checkbox', () => {
Expand Down

0 comments on commit d341f7b

Please sign in to comment.