Fix form reset for x-model radio, checkbox arrays, select multiple and various modifiers #4159
Conversation
bb
changed the title
Fix form reset for x-model radio inputs Fix form reset for x-model checkbox array inputs Fix form reset for x-model radio inputs with number and boolean modifiers Fix x-model select multiple with number and boolean modifiers Fix form reset for select multiple
|
I started out adding more and more conditions in the reset listener but settled on simply always using the nextTick(() => el._x_model && (el.type !== 'radio' || el.checked) && (((el.type === 'checkbox' && Array.isArray(getValue())) || el.type === 'radio' || (el.tagName.toLowerCase() === 'select')) ? setValue(getInputValue(el, modifiers, { target: el }, getValue())) : el._x_model.set(el.value)))now it's just nextTick(() => el._x_model && el._x_model.set(getInputValue(el, modifiers, { target: el }, getValue())))I kept using the public setter (which was prepared to allow programmatic overriding of x-model as described in a comment below that line). However, there are some places in the code which do not use the overridable setter: cloning and when using |
bb
changed the title
| @@ -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)) { | |||
There was a problem hiding this comment.
| || (el.tagName.toLowerCase() === 'select' && el.multiple)) { | |
| || (el.tagName === 'SELECT' && el.multiple)) { |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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>
|
Thanks @bb! Thanks for all your work on this |
This PR fixes form reset for many input types (radio, checkbox array, select multiple) plain as well as in combination with
numberandbooleanmodifiers for casting as well asfillmodifier for<select multipleand also fixes duplicated entries with checkbox arrays during reset.A few tests have been added, all of them were failing before this PR and are good now.
Some of the fixes in more detail:
... and possibly a few more.