Skip to content

Commit

Permalink
Merge pull request #330 from sveltejs/gh-313
Browse files Browse the repository at this point in the history
two-way binding with <select multiple>
  • Loading branch information
Rich-Harris authored Mar 2, 2017
2 parents 3ae25bd + 806cefe commit f799cae
Show file tree
Hide file tree
Showing 4 changed files with 101 additions and 15 deletions.
10 changes: 4 additions & 6 deletions src/generators/dom/visitors/Element.js
Original file line number Diff line number Diff line change
Expand Up @@ -79,12 +79,10 @@ export default {
}

// special case – bound <option> without a value attribute
if ( node.name === 'option' && !node.attributes.find( attribute => attribute.type === 'Attribute' && attribute.name === 'value' ) ) { // TODO check it's bound
// const dynamic = node.children.length > 1 || node.children[0].type !== 'Text';
// TODO do this in init for static values... have to do it in `leave`, because they don't exist yet
local.update.addLine(
`${name}.__value = ${name}.textContent`
);
if ( node.name === 'option' && !node.attributes.find( attribute => attribute.type === 'Attribute' && attribute.name === 'value' ) ) { // TODO check it's bound
const statement = `${name}.__value = ${name}.textContent;`;
local.update.addLine( statement );
node.initialUpdate = statement;
}

generator.current.builders.init.addBlock( local.init );
Expand Down
31 changes: 22 additions & 9 deletions src/generators/dom/visitors/attributes/binding/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,13 +45,17 @@ export default function createBinding ( generator, node, attribute, current, loc
eventName = 'input';
}

const isMultipleSelect = node.name === 'select' && node.attributes.find( attr => attr.name.toLowerCase() === 'multiple' ); // TODO ensure that this is a static attribute
let value;

if ( local.isComponent ) {
value = 'value';
} else if ( node.name === 'select' ) {
// TODO <select multiple> – can use select.selectedOptions
value = 'selectedOption && selectedOption.__value';
if ( isMultipleSelect ) {
value = `[].map.call( ${local.name}.selectedOptions, function ( option ) { return option.__value; })`;
} else {
value = 'selectedOption && selectedOption.__value';
}
} else {
value = `${local.name}.${attribute.name}`;
}
Expand Down Expand Up @@ -101,7 +105,7 @@ export default function createBinding ( generator, node, attribute, current, loc
}

// special case
if ( node.name === 'select' ) {
if ( node.name === 'select' && !isMultipleSelect ) {
setter = `var selectedOption = ${local.name}.selectedOptions[0] || ${local.name}.options[0];\n` + setter;
}

Expand Down Expand Up @@ -130,19 +134,26 @@ export default function createBinding ( generator, node, attribute, current, loc
let updateElement;

if ( node.name === 'select' ) {
// TODO select multiple
const value = generator.current.getUniqueName( 'value' );
const i = generator.current.getUniqueName( 'i' );
const option = generator.current.getUniqueName( 'option' );

const ifStatement = isMultipleSelect ?
deindent`
${option}.selected = ~${value}.indexOf( ${option}.__value );` :
deindent`
if ( ${option}.__value === ${value} ) {
${option}.selected = true;
break;
}`;

updateElement = deindent`
var ${value} = ${contextual ? attribute.value : `root.${attribute.value}`};
console.log( 'value', ${value} );
for ( var ${i} = 0; ${i} < ${local.name}.options.length; ${i} += 1 ) {
var ${option} = ${local.name}.options[${i}];
if ( ${option}.__value === ${value} ) {
${option}.selected = true;
break;
}
${ifStatement}
}
`;
} else {
Expand All @@ -164,7 +175,9 @@ export default function createBinding ( generator, node, attribute, current, loc
node.initialUpdate = updateElement;

local.update.addLine(
`if ( !${local.name}_updating ) ${updateElement}`
`if ( !${local.name}_updating ) {
${updateElement}
}`
);

generator.current.builders.teardown.addLine( deindent`
Expand Down
68 changes: 68 additions & 0 deletions test/generator/binding-select-multiple/_config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
export default {
skip: true, // selectedOptions doesn't work in JSDOM???

data: {
selected: [ 'two', 'three' ]
},

html: `
<select>
<option>one</option>
<option>two</option>
<option>three</option>
</select>
<p>selected: two, three</p>
`,

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

const change = new window.Event( 'change' );

options[1].selected = false;
select.dispatchEvent( change );

assert.deepEqual( component.get( 'selected' ), [ 'three' ] );
assert.htmlEqual( target.innerHTML, `
<select>
<option>one</option>
<option>two</option>
<option>three</option>
</select>
<p>selected: three</p>
` );

options[0].selected = true;
select.dispatchEvent( change );

assert.deepEqual( component.get( 'selected' ), [ 'one', 'three' ] );
assert.htmlEqual( target.innerHTML, `
<select>
<option>one</option>
<option>two</option>
<option>three</option>
</select>
<p>selected: one, three</p>
` );

component.set({ selected: [ 'one', 'two' ] });

assert.ok( options[0].selected );
assert.ok( options[1].selected );
assert.ok( !options[2].selected );

assert.htmlEqual( target.innerHTML, `
<select>
<option>one</option>
<option>two</option>
<option>three</option>
</select>
<p>selected: one, two</p>
` );
}
};
7 changes: 7 additions & 0 deletions test/generator/binding-select-multiple/main.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
<select multiple bind:value='selected'>
<option>one</option>
<option>two</option>
<option>three</option>
</select>

<p>selected: {{selected.join( ', ' )}}</p>

0 comments on commit f799cae

Please sign in to comment.