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

Fix form reset for x-model radio, checkbox arrays, select multiple and various modifiers #4159

Merged
merged 1 commit into from
Apr 22, 2024
Merged

Fix form reset for x-model radio, checkbox arrays, select multiple and various modifiers #4159

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
7 changes: 4 additions & 3 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 @@ -71,7 +71,8 @@ directive('model', (el, { modifiers, expression }, { effect, cleanup }) => {

if (modifiers.includes('fill'))
if ([undefined, null, ''].includes(getValue())
|| (el.type === 'checkbox' && Array.isArray(getValue()))) {
|| (el.type === 'checkbox' && Array.isArray(getValue()))
|| (el.tagName.toLowerCase() === 'select' && el.multiple)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
|| (el.tagName.toLowerCase() === 'select' && el.multiple)) {
|| (el.tagName === 'SELECT' && el.multiple)) {

Copy link
Contributor Author

Choose a reason for hiding this comment

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

While this is definitely fine for HTML4 and HTML5, I'm afraid it would break usage in XHTML documents. Not sure about SVG, never used Alpine in an SVG context (yet).

Also, there are two other places checking for this tagName using toLowerCase comparison. If it is changed, it should be changed in all places.

Copy link
Contributor Author

@bb bb Apr 22, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

You made me curious, so I verified. It does not seem to break XHTML documents, at least not in current Firefox, Chrome, nor Safari versions (on Desktop). It gets weird when embedding svg inline in XHTML, though.... but honestly, who has not switched to HTML5 within the last 15 or so years 😃

So I'm quite sure either approach is fine here.

I don't have any old stuff or IE available for testing currently... not relevant for me, not sure about Alpine's promises in this direction.

Example (validated using https://validator.w3.org/check) prints every element uppercased:

<?xml version="1.0" encoding="UTF-8" ?>
<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN" "http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml" xml:lang="en" lang="en">
<head>
<title>x</title>
</head><body>
<div>
<select name="s">
<option>foo</option>
</select>
</div>
<script type="text/javascript">
    document.querySelectorAll("*").forEach(e => console.log(e.tagName))
</script>
</body></html>

prints every element uppercased.

Interestingly, when embedding SVG without explicit namespaces, tag names for svg are lowercase. When embedding SVG with explicit namespace, they're uppercase.

Tagnames svg and path are lowercase, while HTML elements stay uppercase, e.g. SELECT, svg

<?xml version="1.0" encoding="UTF-8" ?>
<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN" "http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml" xml:lang="en" lang="en">
<head>
<title>x</title>
</head><body>
<div>
<select name="s">
<option>foo</option>
</select>
<svg viewBox="0 0 10 10" style="background-color:#fff" version="1.1" xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink" xml:space="preserve" x="0px" y="0px" width="10" height="10">
    <path d="M 3 3 L 3 7 L 7 7 L 7 3 L 3 3 Z" fill="#f00"/>
</svg>
</div>
<script type="text/javascript">
    document.querySelectorAll("*").forEach(e => console.log(e.tagName))
</script>
</body></html>

Tagnames uppercase with namespace prefix (e.g. SVG:SVG):

<?xml version="1.0" encoding="UTF-8" ?>
<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN" "http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml" xmlns:svg="http://www.w3.org/2000/svg" xml:lang="en" lang="en">
<head>
<title>x</title>
</head><body>
<div>
<select name="s">
<option>foo</option>
</select>
<svg:svg viewBox="0 0 10 10" style="background-color:#fff" version="1.1" xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink" xml:space="preserve" x="0px" y="0px" width="10" height="10">
    <svg:path d="M 3 3 L 3 7 L 7 7 L 7 3 L 3 3 Z" fill="#f00"/>
</svg:svg>
</div>
<script type="text/javascript">
    document.querySelectorAll("*").forEach(e => console.log(e.tagName))
</script>
</body></html>

setValue(
getInputValue(el, modifiers, { target: el }, getValue())
);
Expand All @@ -91,7 +92,7 @@ directive('model', (el, { modifiers, expression }, { effect, cleanup }) => {
// on nextTick so the page doesn't end up out of sync
if (el.form) {
let removeResetListener = on(el.form, 'reset', [], (e) => {
nextTick(() => el._x_model && el._x_model.set(el.value))
nextTick(() => el._x_model && el._x_model.set(getInputValue(el, modifiers, { target: el }, getValue())))
})
cleanup(() => removeResetListener())
}
Expand Down Expand Up @@ -149,7 +150,7 @@ function getInputValue(el, modifiers, event, currentValue) {
newValue = event.target.value
}

return event.target.checked ? currentValue.concat([newValue]) : currentValue.filter(el => ! checkedAttrLooseCompare(el, newValue))
return event.target.checked ? (currentValue.includes(newValue) ? currentValue : currentValue.concat([newValue])) : currentValue.filter(el => !checkedAttrLooseCompare(el, newValue));
} else {
return event.target.checked
}
Expand Down
185 changes: 184 additions & 1 deletion 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 @@ -210,6 +210,169 @@ test('x-model updates value when the form is reset',
}
)

test(
"x-model radio updates value when the form is reset",
html`
<div x-data="{ foo: undefined }">
<form>
<input type="radio" value="radio1" x-model.fill="foo"></input>
<input type="radio" value="radio2" x-model.fill="foo" checked></input>
<input type="radio" value="radio3" x-model.fill="foo"></input>
<button type="reset">Reset</button>
</form>
<span x-text="foo"></span>
</div>
`,
({ get }) => {
get("span").should(haveText("radio2"));
get("input[value='radio1']").click();
get("span").should(haveText("radio1"));
get("button").click();
get("span").should(haveText("radio2"));
}
);

test(
"x-model.number radio updates value when the form is reset",
html`
<div x-data="{ foo: undefined }">
<form>
<input type="radio" value="1" x-model.number.fill="foo"></input>
<input type="radio" value="2" x-model.number.fill="foo" checked></input>
<input type="radio" value="3" x-model.number.fill="foo"></input>
<button type="reset">Reset</button>
</form>
</div>
`,
({ get }) => {
get("[x-data]").should(haveData("foo", 2));
get("input[value='1']").click();
get("[x-data]").should(haveData("foo", 1));
get("button").click();
get("[x-data]").should(haveData("foo", 2));
}
);

test(
"x-model.boolean radio updates value when the form is reset",
html`
<div x-data="{ foo: undefined }">
<form>
<input type="radio" value="true" x-model.boolean.fill="foo" checked></input>
<input type="radio" value="false" x-model.boolean.fill="foo"></input>
<button type="reset">Reset</button>
</form>
</div>
`,
({ get }) => {
get("[x-data]").should(haveData("foo", true));
get("input[value='false']").click();
get("[x-data]").should(haveData("foo", false));
get("button").click();
get("[x-data]").should(haveData("foo", true));
}
);

test(
"x-model checkbox array updates value when the form is reset",
html`
<div x-data="{ foo: [] }">
<form>
<input type="checkbox" value="checkbox1" x-model.fill="foo"></input>
<input type="checkbox" value="checkbox2" x-model.fill="foo" checked></input>
<input type="checkbox" value="checkbox3" x-model.fill="foo" checked></input>
<input type="checkbox" value="checkbox4" x-model.fill="foo"></input>
<button type="reset">Reset</button>
</form>
<span x-text="foo"></span>
</div>
`,
({ get }) => {
get("span").should(haveText("checkbox2,checkbox3"));
get("input[value='checkbox1']").click();
get("span").should(haveText("checkbox2,checkbox3,checkbox1"));
get("input[value='checkbox3']").click();
get("span").should(haveText("checkbox2,checkbox1"));
get("button").click();
get("span").should(haveText("checkbox2,checkbox3"));
}
);

test(
"x-model.number checkbox array updates value when the form is reset",
html`
<div x-data="{ foo: [] }">
<form>
<input type="checkbox" value="1" x-model.number.fill="foo"></input>
<input type="checkbox" value="2" x-model.number.fill="foo" checked></input>
<input type="checkbox" value="3" x-model.number.fill="foo" checked></input>
<input type="checkbox" value="4" x-model.number.fill="foo"></input>
<button type="reset">Reset</button>
</form>
</div>
`,
({ get }) => {
get("[x-data]").should(haveData("foo", [2, 3]));
get("input[value='1']").click();
get("[x-data]").should(haveData("foo", [2, 3, 1]));
get("input[value='3']").click();
get("[x-data]").should(haveData("foo", [2, 1]));
get("button").click();
get("[x-data]").should(haveData("foo", [2, 3]));
}
);

test(
"x-model select updates value when the form is reset",
html`
<div x-data="{ a: null, b: null, c: null, d: null }">
<form>
<select id="a" x-model.fill="a">
<option value="123">123</option>
<option value="456" selected>456</option>
<option value="789">789</option>
</select>
<select id="b" x-model.fill="b" multiple>
<option value="123" selected>123</option>
<option value="456">456</option>
<option value="789" selected>789</option>
</select>
<select id="c" x-model.number.fill="c">
<option value="123">123</option>
<option value="456" selected>456</option>
<option value="789">789</option>
</select>
<select id="d" x-model.number.fill="d" multiple>
<option value="123" selected>123</option>
<option value="456">456</option>
<option value="789" selected>789</option>
</select>
<button type="reset">Reset</button>
</form>
</div>
`,
({ get }) => {
get("[x-data]").should(haveData("a", "456"));
get("[x-data]").should(haveData("b", ["123", "789"]));
get("[x-data]").should(haveData("c", 456));
get("[x-data]").should(haveData("d", [123, 789]));
get("select#a").select("789");
get("select#b").select("456");
get("select#c").select("789");
get("select#d").select("456");
get("[x-data]").should(haveData("a", "789"));
get("[x-data]").should(haveData("b", ["456"]));
get("[x-data]").should(haveData("c", 789));
get("[x-data]").should(haveData("d", [456]));
get("button").click();
get("[x-data]").should(haveData("a", "456"));
get("[x-data]").should(haveData("b", ["123", "789"]));
get("[x-data]").should(haveData("c", 456));
get("[x-data]").should(haveData("d", [123, 789]));
}
);


test('x-model with fill modifier takes input value on null, empty string or undefined',
html`
<div x-data="{ a: 123, b: 0, c: '', d: null, e: {} }">
Expand All @@ -236,7 +399,7 @@ test('x-model with fill modifier takes input value on null, empty string or unde

test('x-model with fill modifier works with select elements',
html`
<div x-data="{ a: null, b: null, c: null, d: null }">
<div x-data="{ a: null, b: null, c: null, d: null, e: null, f: null }">
<select x-model.fill="a">
<option value="123">123</option>
<option value="456" selected>456</option>
Expand All @@ -245,11 +408,31 @@ test('x-model with fill modifier works with select elements',
<option value="123" selected>123</option>
<option value="456" selected>456</option>
</select>
<select x-model.number.fill="c">
<option value="123">123</option>
<option value="456" selected>456</option>
</select>
<select x-model.number.fill="d" multiple>
<option value="123" selected>123</option>
<option value="456" selected>456</option>
</select>
<select x-model.boolean.fill="e">
<option value="true" selected>true</option>
<option value="false">false</option>
</select>
<select x-model.boolean.fill="f" multiple>
<option value="true" selected>true</option>
<option value="false" selected>false</option>
</select>
</div>
`,
({ get }) => {
get('[x-data]').should(haveData('a', '456'));
get('[x-data]').should(haveData('b', ['123', '456']));
get('[x-data]').should(haveData('c', 456));
get('[x-data]').should(haveData('d', [123, 456]));
get('[x-data]').should(haveData('e', true));
get('[x-data]').should(haveData('f', [true, false]));
}
);

Expand Down
Loading