-
Notifications
You must be signed in to change notification settings - Fork 176
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
Allow binding <select>s via data attributes #257
Allow binding <select>s via data attributes #257
Conversation
@akre54 would you mind taking another look at this one? Thank you for your feedback yesterday! |
binding = binding || {}; | ||
namespace = '.stickit.' + model.cid; | ||
|
||
binding = binding || {}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
Minor nits but otherwise looks good. Thanks! |
Squash down to one commit and I'll merge. Thanks again |
@akre54 many thanks for reviewing this PR! Just fixed those last issues + squashed it down! |
Actually -- my apologies, want to squash in one last change (simply changing |
(updated that and Travis seems happy about it) |
…a_attributes Allow binding <select>s via data attributes
This attempts to resolve #249 by allowing pre-rendered
<select>
<option>
s to have values bound from a data attribute (data-stickit-bind-val
, which was previously used only internally and nameddata-stickit_bind_val
), rather than from the<option value>
.The tests now verify that pre-rendered
<select>
s withoutdata-stickit-bind-val
continue to have their options processed to a collection containing string values, and that ones withdata-stickit-bind-val
now support numeric values.This fixes a longstanding issue with binding pre-rendered selects at the cost of some extra complexity and a bit of duplicative markup.