Skip to content

Commit

Permalink
Merge pull request #386 from sveltejs/gh-312
Browse files Browse the repository at this point in the history
bind:group for checkbox inputs
  • Loading branch information
Rich-Harris authored Mar 17, 2017
2 parents 5c436ac + 7b057e4 commit 3aa1814
Show file tree
Hide file tree
Showing 11 changed files with 186 additions and 21 deletions.
2 changes: 2 additions & 0 deletions src/generators/Generator.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ export default class Generator {
this.components = {};
this.events = {};

this.bindingGroups = [];

// track which properties are needed, so we can provide useful info
// in dev mode
this.expectedProperties = {};
Expand Down
4 changes: 4 additions & 0 deletions src/generators/dom/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -343,6 +343,10 @@ export default function dom ( parsed, source, options, names ) {
);
}

if ( generator.bindingGroups.length ) {
constructorBlock.addLine( `this._bindingGroups = [ ${Array( generator.bindingGroups.length ).fill( '[]' ).join( ', ' )} ];` );
}

constructorBlock.addBlock( deindent`
this._observers = {
pre: Object.create( null ),
Expand Down
4 changes: 3 additions & 1 deletion src/generators/dom/visitors/Element.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,8 @@ export default {
allUsedContexts: [],

init: new CodeBuilder(),
update: new CodeBuilder()
update: new CodeBuilder(),
teardown: new CodeBuilder()
};

const isToplevel = generator.current.localElementDepth === 0;
Expand Down Expand Up @@ -85,6 +86,7 @@ export default {

generator.current.builders.init.addBlock( local.init );
if ( !local.update.isEmpty() ) generator.current.builders.update.addBlock( local.update );
if ( !local.teardown.isEmpty() ) generator.current.builders.teardown.addBlock( local.teardown );

generator.createMountStatement( name );

Expand Down
18 changes: 11 additions & 7 deletions src/generators/dom/visitors/attributes/addElementAttributes.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import attributeLookup from './lookup.js';
import addElementBinding from './addElementBinding';
import deindent from '../../../../utils/deindent.js';
import flattenReference from '../../../../utils/flattenReference.js';
import getStaticAttributeValue from './binding/getStaticAttributeValue.js';

export default function addElementAttributes ( generator, node, local ) {
node.attributes.forEach( attribute => {
Expand All @@ -13,8 +14,12 @@ export default function addElementAttributes ( generator, node, local ) {

let dynamic = false;

const isBoundOptionValue = node.name === 'option' && name === 'value'; // TODO check it's actually bound
const propertyName = isBoundOptionValue ? '__value' : metadata && metadata.propertyName;
const isIndirectlyBoundValue = name === 'value' && (
node.name === 'option' || // TODO check it's actually bound
node.name === 'input' && /^(checkbox|radio)$/.test( getStaticAttributeValue( node, 'type' ) )
);

const propertyName = isIndirectlyBoundValue ? '__value' : metadata && metadata.propertyName;

const isXlink = name.slice( 0, 6 ) === 'xlink:';

Expand Down Expand Up @@ -134,12 +139,11 @@ export default function addElementAttributes ( generator, node, local ) {
local.update.addLine( updater );
}

if ( isBoundOptionValue ) {
local.init.addLine( `${local.name}.value = ${local.name}.__value` );
if ( isIndirectlyBoundValue ) {
const updateValue = `${local.name}.value = ${local.name}.__value;`;

if (dynamic) {
local.update.addLine( `${local.name}.value = ${local.name}.__value` );
}
local.init.addLine( updateValue );
if ( dynamic ) local.update.addLine( updateValue );
}
}

Expand Down
71 changes: 60 additions & 11 deletions src/generators/dom/visitors/attributes/addElementBinding.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
import deindent from '../../../../utils/deindent.js';
import flattenReference from '../../../../utils/flattenReference.js';
import getSetter from './binding/getSetter.js';
import getStaticAttributeValue from './binding/getStaticAttributeValue.js';

export default function createBinding ( generator, node, attribute, current, local ) {
const { name } = flattenReference( attribute.value );
const { name, keypath } = flattenReference( attribute.value );
const { snippet, contexts, dependencies } = generator.contextualise( attribute.value );

if ( dependencies.length > 1 ) throw new Error( 'An unexpected situation arose. Please raise an issue at https://github.com/sveltejs/svelte/issues — thanks!' );
Expand All @@ -14,20 +15,20 @@ export default function createBinding ( generator, node, attribute, current, loc

const handler = current.getUniqueName( `${local.name}ChangeHandler` );

const isMultipleSelect = node.name === 'select' && node.attributes.find( attr => attr.name.toLowerCase() === 'multiple' ); // TODO ensure that this is a static attribute
const value = getBindingValue( local, node, attribute, isMultipleSelect );
const isMultipleSelect = node.name === 'select' && node.attributes.find( attr => attr.name.toLowerCase() === 'multiple' ); // TODO use getStaticAttributeValue
const bindingGroup = attribute.name === 'group' ? getBindingGroup( generator, current, attribute, keypath ) : null;
const value = getBindingValue( generator, local, node, attribute, isMultipleSelect, bindingGroup );
const eventName = getBindingEventName( node );

let setter = getSetter({ current, name, context: '__svelte', attribute, dependencies, snippet, value });

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

let updateElement;

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

const value = generator.current.getUniqueName( 'value' );
const i = generator.current.getUniqueName( 'i' );
const option = generator.current.getUniqueName( 'option' );
Expand All @@ -49,7 +50,35 @@ export default function createBinding ( generator, node, attribute, current, loc
${ifStatement}
}
`;
} else {
}

// <input type='checkbox|radio' bind:group='selected'> special case
else if ( attribute.name === 'group' ) {
const type = getStaticAttributeValue( node, 'type' );

if ( type === 'checkbox' ) {
local.init.addLine(
`component._bindingGroups[${bindingGroup}].push( ${local.name} );`
);

local.teardown.addBlock(
`component._bindingGroups[${bindingGroup}].splice( component._bindingGroups[${bindingGroup}].indexOf( ${local.name} ), 1 );`
);

updateElement = `${local.name}.checked = ~${snippet}.indexOf( ${local.name}.__value );`;
}

else if ( type === 'radio' ) {
throw new Error( 'TODO' );
}

else {
throw new Error( `Unexpected bind:group` ); // TODO catch this in validation with a better error
}
}

// everything else
else {
updateElement = `${local.name}.${attribute.name} = ${snippet};`;
}

Expand Down Expand Up @@ -93,14 +122,34 @@ function getBindingEventName ( node ) {
return 'change';
}

function getBindingValue ( local, node, attribute, isMultipleSelect ) {
function getBindingValue ( generator, local, node, attribute, isMultipleSelect, bindingGroup ) {
// <select multiple bind:value='selected>
if ( isMultipleSelect ) {
return `[].map.call( ${local.name}.selectedOptions, function ( option ) { return option.__value; })`;
}

// <select bind:value='selected>
if ( node.name === 'select' ) {
return 'selectedOption && selectedOption.__value';
}

// <input type='checkbox' bind:group='foo'>
if ( attribute.name === 'group' ) {
return `${generator.helper( 'getBindingGroupValue' )}( component._bindingGroups[${bindingGroup}] )`;
}

// everything else
return `${local.name}.${attribute.name}`;
}

function getBindingGroup ( generator, current, attribute, keypath ) {
// TODO handle contextual bindings — `keypath` should include unique ID of
// each block that provides context
let index = generator.bindingGroups.indexOf( keypath );
if ( index === -1 ) {
index = generator.bindingGroups.length;
generator.bindingGroups.push( keypath );
}

return index;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
export default function getStaticAttributeValue ( node, name ) {
const attribute = node.attributes.find( attr => attr.name.toLowerCase() === name );
if ( !attribute ) return null;

if ( attribute.value.length !== 1 || attribute.value[0].type !== 'Text' ) {
// TODO catch this in validation phase, give a more useful error (with location etc)
throw new Error( `'${name} must be a static attribute` );
}

return attribute.value[0].data;
}
Empty file.
8 changes: 8 additions & 0 deletions src/shared/dom.js
Original file line number Diff line number Diff line change
Expand Up @@ -53,3 +53,11 @@ export function setAttribute ( node, attribute, value ) {
export function setXlinkAttribute ( node, attribute, value ) {
node.setAttributeNS( 'http://www.w3.org/1999/xlink', attribute, value );
}

export function getBindingGroupValue ( group ) {
var value = [];
for ( var i = 0; i < group.length; i += 1 ) {
if ( group[i].checked ) value.push( group[i].__value );
}
return value;
}
78 changes: 78 additions & 0 deletions test/generator/samples/binding-input-checkbox-group/_config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
const values = [
{ name: 'Alpha' },
{ name: 'Beta' },
{ name: 'Gamma' }
];

export default {
data: {
values,
selected: [ values[1] ]
},

'skip-ssr': true, // values are rendered as [object Object]

html: `
<label>
<input type="checkbox"> Alpha
</label>
<label>
<input type="checkbox"> Beta
</label>
<label>
<input type="checkbox"> Gamma
</label>
<p>Beta</p>`,

test ( assert, component, target, window ) {
const inputs = target.querySelectorAll( 'input' );
assert.equal( inputs[0].checked, false );
assert.equal( inputs[1].checked, true );
assert.equal( inputs[2].checked, false );

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

inputs[0].checked = true;
inputs[0].dispatchEvent( event );

assert.htmlEqual( target.innerHTML, `
<label>
<input type="checkbox"> Alpha
</label>
<label>
<input type="checkbox"> Beta
</label>
<label>
<input type="checkbox"> Gamma
</label>
<p>Alpha, Beta</p>
` );

component.set({ selected: [ values[1], values[2] ] });
assert.equal( inputs[0].checked, false );
assert.equal( inputs[1].checked, true );
assert.equal( inputs[2].checked, true );

assert.htmlEqual( target.innerHTML, `
<label>
<input type="checkbox"> Alpha
</label>
<label>
<input type="checkbox"> Beta
</label>
<label>
<input type="checkbox"> Gamma
</label>
<p>Beta, Gamma</p>
` );
}
};
7 changes: 7 additions & 0 deletions test/generator/samples/binding-input-checkbox-group/main.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{{#each values as value}}
<label>
<input type="checkbox" value="{{value}}" bind:group='selected' /> {{value.name}}
</label>
{{/each}}

<p>{{selected.map( function ( value ) { return value.name; }).join( ', ' ) }}</p>
4 changes: 2 additions & 2 deletions test/helpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -102,11 +102,11 @@ function cleanChildren ( node ) {
export function setupHtmlEqual () {
return env().then( window => {
assert.htmlEqual = ( actual, expected, message ) => {
window.document.body.innerHTML = actual.trim();
window.document.body.innerHTML = actual.replace( />[\s\r\n]+</g, '><' ).trim();
cleanChildren( window.document.body, '' );
actual = window.document.body.innerHTML;

window.document.body.innerHTML = expected.trim();
window.document.body.innerHTML = expected.replace( />[\s\r\n]+</g, '><' ).trim();
cleanChildren( window.document.body, '' );
expected = window.document.body.innerHTML;

Expand Down

0 comments on commit 3aa1814

Please sign in to comment.