Skip to content

Commit

Permalink
fix: bind null option and input values consistently
Browse files Browse the repository at this point in the history
Null and undefined `value` bindings should always be set to an empty string. This allows native browser validation of `required` fields to work as expected with placeholder options.

Resolves #8312
  • Loading branch information
theodorejb committed Feb 26, 2023
1 parent ca53151 commit ade8ef4
Show file tree
Hide file tree
Showing 4 changed files with 44 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ export default class AttributeWrapper extends BaseAttributeWrapper {
}

if (is_indirectly_bound_value) {
const update_value = b`${element.var}.value = ${element.var}.__value;`;
const update_value = b`@set_input_value(${element.var}, ${element.var}.__value);`;
block.chunks.hydrate.push(update_value);

updater = b`
Expand Down
7 changes: 4 additions & 3 deletions test/js/samples/select-dynamic-value/expected.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@ import {
insert,
noop,
safe_not_equal,
select_option
select_option,
set_input_value
} from "svelte/internal";

function create_fragment(ctx) {
Expand All @@ -24,9 +25,9 @@ function create_fragment(ctx) {
option1 = element("option");
option1.textContent = "2";
option0.__value = "1";
option0.value = option0.__value;
set_input_value(option0, option0.__value);
option1.__value = "2";
option1.value = option1.__value;
set_input_value(option1, option1.__value);
},
m(target, anchor) {
insert(target, select, anchor);
Expand Down
28 changes: 28 additions & 0 deletions test/runtime/samples/binding-select-null-placeholder/_config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
const items = [ { id: 'a' }, { id: 'b' } ];

export default {
props: {
foo: null,
items
},

test({ assert, component, target }) {
const select = target.querySelector( 'select' );
const options = target.querySelectorAll( 'option' );

assert.equal( options[0].selected, true );
assert.equal( options[0].disabled, true );
assert.equal( options[1].selected, false );
assert.equal( options[1].disabled, false );

// placeholder option value must be blank string for native required field validation
assert.equal( options[0].value, '' );
assert.equal( select.checkValidity(), false );

component.foo = items[0];

assert.equal( options[0].selected, false );
assert.equal( options[1].selected, true );
assert.equal( select.checkValidity(), true );
}
};
11 changes: 11 additions & 0 deletions test/runtime/samples/binding-select-null-placeholder/main.svelte
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<script>
export let foo;
export let items;
</script>

<select bind:value={foo} required>
<option value={null} disabled>Select an option</option>
{#each items as item}
<option value={item}>{item.id}</option>
{/each}
</select>

0 comments on commit ade8ef4

Please sign in to comment.