Skip to content

Added automatic generation of select options for stickit bindings. #69

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

Open
ccpowers wants to merge 8 commits into
base: master
Choose a base branch
from
Open

Added automatic generation of select options for stickit bindings. #69

Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
9a65077
Added automatic generation of select options for stickit bindings.
katypowers Jun 18, 2015
7baf251
Automatic generation of selectOptions for stickit bindings.
katypowers Jun 23, 2015
28ba9b6
Fixed problem in form view if statement
katypowers Jun 30, 2015
c4ca7e1
Fixed problem in form view if statement
katypowers Jun 30, 2015
0518dd7
Fixed problem with 'apply' in generate select options.
katypowers Jul 1, 2015
48f46a5
Merge branch 'master' of https://github.com/ccpowers/backbone-torso
katypowers Jul 1, 2015
7990583
Fixed docstring for __generateSelectOptions.
katypowers Jul 6, 2015
53ac0d8
Merge remote-tracking branch 'upstream/master'
katypowers Jul 9, 2015
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
29 changes: 28 additions & 1 deletion modules/FormView.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -213,6 +213,11 @@
var attr = $(element).data('model'),
options = self.__getFieldOptions(attr),
fieldBinding = self.__generateModelFieldBinding(attr, options);

//add select options
if ($(element).is('select')) {
fieldBinding.selectOptions = self.__generateSelectOptions(element, options);
}
self.bindings['[data-model="' + attr + '"]'] = fieldBinding;
});
},
Expand All @@ -230,7 +235,7 @@
/**
* @method __generateModelFieldBinding
* @param field {String} A specific model field
* @param options {Object} Additional heavior options for the bindings
* @param options {Object} Additional behavior options for the bindings
* @param [options.modelFormat] {Object} The function called before setting model values
* @param [options.viewFormat] {Object} The function called before setting view values
* @private
Expand All @@ -253,6 +258,28 @@
return options.viewFormat ? options.viewFormat.apply(this, params) : value;
}
};
},

/**
* @method __generateSelectOptions
* @param element {Element} The select element to generate options for
* @param opts {Object} Additional behavior options for the bindings
* @param [opts.modelFormat] {Object} The function called before setting model values
* @private
* @return {<Stickit select options hash>}
*/
__generateSelectOptions: function(element, opts) {
var collection = [],
options = $(element).children('option');

_.each(options, function(option) {
collection.push({'label': $(option).text(),

@kentmw kentmw Jun 24, 2015

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

last question. This all looks good, but I'm a little worried about blowing away your select options if they have different classes/attributes that you had added before. Will you lose classes?

@ccpowers ccpowers Jun 25, 2015

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I just tested this- you will lose classes on the select options. You won't lose data on the actual select though. There might be a clever way to get around this with the 'classes' bindings available in stickit - I will think about it some more.

@ccpowers ccpowers Jun 29, 2015

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

There doesn't seem to be a way to keep your classes while binding select options using selectOptions - any ideas?

@kentmw kentmw Jul 30, 2015

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@ccpowers, I think you should merge this. Though, let's create an enhancement to keep classes on two-way bound select options.

@kentmw kentmw Aug 20, 2015

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@ccpowers, do you want to keep working on this or should I take over?

@ccpowers ccpowers Aug 20, 2015

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

@kentmw I'm happy to finish it but i'm about to leave the country for a month so if you want it done before October, you should probably take over...just let me know

@kentmw kentmw Aug 20, 2015

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We can wait until you get back, that will be fine. If we need it before you return, we'll create a new PR. Have fun!

@kentmw kentmw Oct 15, 2015

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hey @ccpowers, are you back? Any interest in picking this back up? We're pretty close to finishing.

'value': opts.modelFormat ? opts.modelFormat.apply(this, [$(option).val()]) : $(option).val()});
});

return {collection: collection,

@kentmw kentmw Jun 22, 2015

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Add an issue to Torso that explains the bug and reference the issue in this commit message.

@ccpowers ccpowers Jun 23, 2015

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Done. See #84.

labelPath: 'label',
valuePath: 'value'};
}
});

Expand Down