Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

x-model.boolean modifier #3532

Merged
merged 3 commits into from
Nov 11, 2023
Merged

x-model.boolean modifier #3532

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0917937
Add x-model.boolean
gdebrauwer Apr 5, 2023
099b679
Merge branch 'main' into x-model-boolean-support
calebporzio Nov 11, 2023
9364cf4
fix
calebporzio Nov 11, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 41 additions & 11 deletions packages/alpinejs/src/directives/x-model.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ directive('model', (el, { modifiers, expression }, { effect, cleanup }) => {
})
}
}

if (typeof expression === 'string' && el.type === 'radio') {
// Radio buttons only work properly when they share a name attribute.
// People might assume we take care of that for them, because
Expand All @@ -69,7 +69,7 @@ directive('model', (el, { modifiers, expression }, { effect, cleanup }) => {
let removeListener = isCloning ? () => {} : on(el, event, modifiers, (e) => {
setValue(getInputValue(el, modifiers, e, getValue()))
})

if (modifiers.includes('fill'))
if ([null, ''].includes(getValue())
|| (el.type === 'checkbox' && Array.isArray(getValue()))) {
Expand Down Expand Up @@ -138,26 +138,44 @@ function getInputValue(el, modifiers, event, currentValue) {
else if (el.type === 'checkbox') {
// If the data we are binding to is an array, toggle its value inside the array.
if (Array.isArray(currentValue)) {
let newValue = modifiers.includes('number') ? safeParseNumber(event.target.value) : event.target.value
let newValue = null;

if (modifiers.includes('number')) {
newValue = safeParseNumber(event.target.value)
} else if (modifiers.includes('boolean')) {
newValue = safeParseBoolean(event.target.value)
} else {
newValue = event.target.value
}

return event.target.checked ? currentValue.concat([newValue]) : currentValue.filter(el => ! checkedAttrLooseCompare(el, newValue))
} else {
return event.target.checked
}
} else if (el.tagName.toLowerCase() === 'select' && el.multiple) {
return modifiers.includes('number')
? Array.from(event.target.selectedOptions).map(option => {
if (modifiers.includes('number')) {
return Array.from(event.target.selectedOptions).map(option => {
let rawValue = option.value || option.text
return safeParseNumber(rawValue)
})
: Array.from(event.target.selectedOptions).map(option => {
return option.value || option.text
} else if (modifiers.includes('boolean')) {
return Array.from(event.target.selectedOptions).map(option => {
let rawValue = option.value || option.text
return safeParseBoolean(rawValue)
})
}

return Array.from(event.target.selectedOptions).map(option => {
return option.value || option.text
})
} else {
let rawValue = event.target.value
return modifiers.includes('number')
? safeParseNumber(rawValue)
: (modifiers.includes('trim') ? rawValue.trim() : rawValue)
if (modifiers.includes('number')) {
return safeParseNumber(event.target.value)
} else if (modifiers.includes('boolean')) {
return safeParseBoolean(event.target.value)
}

return modifiers.includes('trim') ? event.target.value.trim() : event.target.value
}
})
}
Expand All @@ -168,6 +186,18 @@ function safeParseNumber(rawValue) {
return isNumeric(number) ? number : rawValue
}

function safeParseBoolean(rawValue) {
if ([1, '1', 'true', true].includes(rawValue)) {
return true
}

if ([0, '0', 'false', false].includes(rawValue)) {
return false
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking "How could we do this with only one comparison?" and my brain went to

Boolean(JSON.parse(rawValue.toString()))

But surely that would stupid right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that does not work, because if the rawValue is not 1/0/true/false, then we want to just return that raw value. With that code, I don't think that is possible 🤔

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I do see that. To me though, the use case of this modifier seems fundamentally contradictory to ever having it be any other values. I'd rather have it fail early (at a json parsing error) than continue to have the bad value end up somewhere else somehow.

But that's an opinionated design.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The x-model.number works like that as well so I thought it might be best to let it behave in the same way

alpine/packages/alpinejs/src/directives/x-model.js

Lines 166 to 170 in 6142a83

function safeParseNumber(rawValue) {
let number = rawValue ? parseFloat(rawValue) : null
return isNumeric(number) ? number : rawValue
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense 👍

}

return rawValue ? Boolean(rawValue) : null
}

function checkedAttrLooseCompare(valueA, valueB) {
return valueA == valueB
}
Expand Down
13 changes: 13 additions & 0 deletions packages/docs/src/en/directives/model.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -307,6 +307,19 @@ By default, any data stored in a property via `x-model` is stored as a string. T
<span x-text="typeof age"></span>
```

<a name="boolean"></a>
### `.boolean`

By default, any data stored in a property via `x-model` is stored as a string. To force Alpine to store the value as a JavaScript boolean, add the `.boolean` modifier. Both integers (1/0) and strings (true/false) are valid boolean values.

```alpine
<select x-model.boolean="isActive">
<option value="true">Yes</option>
<option value="false">No</option>
</select>
<span x-text="typeof isActive"></span>
```

<a name="debounce"></a>
### `.debounce`

Expand Down
55 changes: 55 additions & 0 deletions tests/cypress/integration/directives/x-model.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,61 @@ test('x-model with number modifier returns: null if empty, original value if cas
}
)

test('x-model casts value to boolean if boolean modifier is present',
html`
<div x-data="{ foo: null, bar: null, baz: [] }">
<input type="text" x-model.boolean="foo"></input>
<select x-model.boolean="bar">
<option value="true">yes</option>
<option value="false">no</option>
</select>
</div>
`,
({ get }) => {
get('input[type=text]').type('1')
get('div').should(haveData('foo', true))

get('input[type=text]').clear().type('0')
get('div').should(haveData('foo', false))

get('input[type=text]').clear().type('true')
get('div').should(haveData('foo', true))

get('input[type=text]').clear().type('false')
get('div').should(haveData('foo', false))

get('select').select('no')
get('div').should(haveData('bar', false))

get('select').select('yes')
get('div').should(haveData('bar', true))
}
)

test('x-model with boolean modifier returns: null if empty, original value if casting fails, numeric value if casting passes',
html`
<div x-data="{ foo: 0, bar: '' }">
<input x-model.boolean="foo"></input>
</div>
`,
({ get }) => {
get('input').clear()
get('div').should(haveData('foo', null))
get('input').clear().type('bar')
get('div').should(haveData('foo', 'bar'))
get('input').clear().type('1')
get('div').should(haveData('foo', true))
get('input').clear().type('1').clear()
get('div').should(haveData('foo', null))
get('input').clear().type('0')
get('div').should(haveData('foo', false))
get('input').clear().type('bar')
get('div').should(haveData('foo', 'bar'))
get('input').clear().type('0').clear()
get('div').should(haveData('foo', null))
}
)

test('x-model trims value if trim modifier is present',
html`
<div x-data="{ foo: '' }">
Expand Down
Loading