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

knockout.js dynamic add #81

Closed
jxmiller opened this issue Feb 14, 2014 · 15 comments
Closed

knockout.js dynamic add #81

jxmiller opened this issue Feb 14, 2014 · 15 comments
Labels

Comments

@jxmiller
Copy link

Using knockout.js in a repeating element. When I add an object to the view model, it adds a row to a table where a slider is supposed to render. The error I get in chrome is: "Calling context element does not have instance of Slider bound to it. Check your code to make sure the JQuery object returned from the call to the slider() initializer is calling the method".

I have tried to destroy and recreate but no luck. Here is my binding handler where the error occurs.
ko.bindingHandlers.sliderValue = {
init: function (element, valueAccessor, allBindings, viewModel, bindingContext) {
var value = valueAccessor();
$(element).slider('destroy');
$(element).slider({});
$(element).slider('setValue', value())
$(element).on('slide', function (ev) {
value(ev.value);
});
},
update: function (element, valueAccessor, allBindings, viewModel, bindingContext) {
var value = valueAccessor();
$(element).slider('setValue', value());
}
};

@seiyria
Copy link
Owner

seiyria commented Feb 14, 2014

Could you please add a jsfiddle? Thanks!

@jxmiller
Copy link
Author

http://jsfiddle.net/ERz7u/1/

I found out how to get it to work. Essentially I was using the original non-forked slider without the commented code in the fiddle. For that it worked. I changed to this slider when I saw a few added abilities that I wanted to use, namely tooltip: always. This is when I had the bug. so with the fiddle above if you run as--is you will see the error I get, but there is a comment that if you undo the comment it will work.

Note, I am not ever instantiating slider in .js except now I am in the knockout binding. I was using the data-attribs to auto slider everything.

Thanks

@jxmiller
Copy link
Author

It appears that dynamically loaded html that has data-attribs might not work in this version.

@jxmiller
Copy link
Author

In my above comment, I meant to say NOT, instead of now...

@seiyria
Copy link
Owner

seiyria commented Feb 14, 2014

So, based on your prior comments, does this work now? Or is there still something to investigate?

@jxmiller
Copy link
Author

To me it is a bug. A bug that a lot of javascript libs have when concerning dynamically added DOM elements. Did I get it to work? Yes. I still use data-attribs, but they do nothing until I manually call $(elem).slider(). I would rather not call that but not huge to me, just my point of view.

Thanks

@seiyria
Copy link
Owner

seiyria commented Feb 24, 2014

Having used knockout a bit, I thought about this some more. Wouldn't it make more sense to pass all of that data in to the slider binding and pass it in that way? Also, many of the knockout bindings I've seen that wrap jQuery libraries do a similar thing. I'm not sure how you would envision it functioning otherwise, would you care to elaborate?

@jxmiller
Copy link
Author

I am all for a suggestion change. Can you post a fiddle example? Just so you can see how I am using it best I have the ko.binding code (from my first post here) and then I bind in html like this As far as I understand it, doing the custom ko.binding allows me to then use sliderValue in my data-bind attributes like you see below >

<tbody data-bind="foreach: fooArray">
     <tr>
         <td>
             <input type="text" class="slider form-control" value="" data-bind="sliderValue: $data"
                                       data-slider-min="0"
                                       data-slider-max="3"
                                       data-slider-step=".25"
                                       data-slider-orientation="horizontal"
                                       data-slider-tooltip="always" />
         </td>
      </tr>
</tbody>

And here is the custom binder I am using now (just slightly different than my first post above) >

ko.bindingHandlers.sliderValue = {
    init: function (element, valueAccessor, allBindings, viewModel, bindingContext) {
        var value = valueAccessor();
        $(element).slider({value: value()}); // is this what you mean?
        //$(element).slider();
        //$(element).slider('setValue', value())
        $(element).on('slide', function (ev) {
            value(ev.value);
        });
    },
    update: function (element, valueAccessor, allBindings, viewModel, bindingContext) {
        var value = valueAccessor();
        $(element).slider('setValue', value());
    }
};

@seiyria
Copy link
Owner

seiyria commented Feb 24, 2014

While it's not really a jsfiddle, this shows what I mean, more precisely:

<a href="#" data-bind="tooltip: {title: 'Another tooltip', placement: 'right'}">have a</a>

Such that the binding is actually passing an object in. From personal experience, this is preferable to using a lot of data- attributes.

@seiyria
Copy link
Owner

seiyria commented Apr 28, 2014

I'm going to close this, please re-open if you feel that the way I suggested is not viable and this is still a problem.

@seiyria seiyria closed this as completed Apr 28, 2014
@cosminstefanxp
Copy link

Hi, Just for future reference, I've made this work accordingly to the proper way knockout parameters should be provided (also as mentioned by @seiyria above):

http://jsfiddle.net/ERz7u/21/

@seiyria
Copy link
Owner

seiyria commented May 23, 2014

@cosminstefanxp thanks for that!

@seiyria
Copy link
Owner

seiyria commented May 23, 2014

I've added this to our README, as I'm sure I'll make one for use with angular eventually, and I'm sure other things will come along as well.

@cosminstefanxp
Copy link

Thanks! 👍
I've also put it as a github project here: https://github.com/cosminstefanxp/bootstrap-slider-knockout-binding , in case some changes will be done.

@seiyria
Copy link
Owner

seiyria commented May 23, 2014

I'll link to that instead then. Thanks again!

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

No branches or pull requests

3 participants