Skip to content
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.

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

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

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

Conversation

ccpowers
Copy link

No description provided.

* @method _generateSelectOptions
* @param element {Element} The select element to generate options for
* @param opts {Object} Additional behavior options for the bindings
* @param [options.modelFormat] {Object} The function called before setting model values
Copy link
Contributor

Choose a reason for hiding this comment

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

opts.modelFormat?

Copy link
Author

Choose a reason for hiding this comment

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

Done.

options = $(element).children('option');

_.each(options, function(option) {
collection.push({'label': $(option).text(),
Copy link
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?

Copy link
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.

Copy link
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?

Copy link
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.

Copy link
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?

Copy link
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

Copy link
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!

Copy link
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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants