Browse Source

Merge pull request #815 from HugoDF/fix-checkbox

Improve consistency of `checked` attribute value comparisons
Caleb Porzio 4 years ago
parent
commit
2167d7bab8
4 changed files with 32 additions and 5 deletions
  1. 3 3
      src/directives/bind.js
  2. 2 2
      src/directives/model.js
  3. 4 0
      src/utils.js
  4. 23 0
      test/model.spec.js

+ 3 - 3
src/directives/bind.js

@@ -1,4 +1,4 @@
-import { arrayUnique, isBooleanAttr, convertClassStringToArray, camelCase } from '../utils'
+import { arrayUnique, checkedAttrLooseCompare, isBooleanAttr, convertClassStringToArray, camelCase } from '../utils'
 import Alpine from '../index'
 
 export function handleAttributeBindingDirective(component, el, attrName, expression, extraVars, attrType, modifiers) {
@@ -19,7 +19,7 @@ export function handleAttributeBindingDirective(component, el, attrName, express
             if (el.attributes.value === undefined && attrType === 'bind') {
                 el.value = value
             } else if (attrType !== 'bind') {
-                el.checked = el.value == value
+                el.checked = checkedAttrLooseCompare(el.value, value)
             }
         } else if (el.type === 'checkbox') {
             // If we are explicitly binding a string to the :value, set the string,
@@ -32,7 +32,7 @@ export function handleAttributeBindingDirective(component, el, attrName, express
                     // I'm purposely not using Array.includes here because it's
                     // strict, and because of Numeric/String mis-casting, I
                     // want the "includes" to be "fuzzy".
-                    el.checked = value.some(val => val == el.value)
+                    el.checked = value.some(val => checkedAttrLooseCompare(val, el.value))
                 } else {
                     el.checked = !!value
                 }

+ 2 - 2
src/directives/model.js

@@ -1,5 +1,5 @@
 import { registerListener } from './on'
-import { isNumeric } from '../utils'
+import { isNumeric, checkedAttrLooseCompare } from '../utils'
 
 export function registerModelListener(component, el, modifiers, expression, extraVars) {
     // If the element we are binding to is a select, a radio, or checkbox
@@ -35,7 +35,7 @@ function generateModelAssignmentFunction(el, modifiers, expression) {
             // If the data we are binding to is an array, toggle its value inside the array.
             if (Array.isArray(currentValue)) {
                 const newValue = modifiers.includes('number') ? safeParseNumber(event.target.value) : event.target.value
-                return event.target.checked ? currentValue.concat([newValue]) : currentValue.filter(i => i !== newValue)
+                return event.target.checked ? currentValue.concat([newValue]) : currentValue.filter(el => !checkedAttrLooseCompare(el, newValue))
             } else {
                 return event.target.checked
             }

+ 4 - 0
src/utils.js

@@ -20,6 +20,10 @@ export function isTesting() {
         || navigator.userAgent.includes("jsdom")
 }
 
+export function checkedAttrLooseCompare(valueA, valueB) {
+    return valueA == valueB
+}
+
 export function warnIfMalformedTemplate(el, directive) {
     if (el.tagName.toLowerCase() !== 'template') {
         console.warn(`Alpine: [${directive}] directive should only be added to <template> tags. See https://github.com/alpinejs/alpine#${directive}`)

+ 23 - 0
test/model.spec.js

@@ -205,6 +205,29 @@ test('x-model checkbox array binding supports .number modifier', async () => {
     await wait(() => { expect(document.querySelector('span').getAttribute('bar')).toEqual("[3]") })
 })
 
+test('x-model checkbox array binding is consistent (if value is initially checked, it can be unchecked)', async () => {
+    // https://github.com/alpinejs/alpine/issues/814
+    document.body.innerHTML = `
+        <div
+            x-data="{
+                selected: [2]
+            }"
+        >
+            <input type="checkbox" value="2" x-model="selected" />
+
+            <span x-bind:bar="JSON.stringify(selected)"></span>
+        </div>
+    `
+
+    Alpine.start()
+
+    expect(document.querySelector('input[type=checkbox]').checked).toEqual(true)
+    expect(document.querySelector('span').getAttribute('bar')).toEqual("[2]")
+
+    fireEvent.change(document.querySelector('input[type=checkbox]'), { target: { checked: false } })
+    await wait(() => { expect(document.querySelector('span').getAttribute('bar')).toEqual("[]") })
+})
+
 test('x-model binds radio value', async () => {
     document.body.innerHTML = `
         <div x-data="{ foo: 'bar' }">