billpull / knockout-bootstrap

A plugin that adds custom bindings for twitter bootstrap objects such as tooltips and popovers.
233 stars 69 forks source link

Cannot apply bindings multiple times #49

Open billpull opened 10 years ago

billpull commented 10 years ago

when updating to knockout3 in the ko3-bs3 branch I get an error about applying bindings multiple times to the same node.

ghost commented 10 years ago

I am seeing the same behavior. This is the first time I am using your library, so I thought it was they way I was looking it up. I am using a popover with a toggle trigger because I wanted a click to open and close it. But I receive the multiple apply bindings error

ghost commented 10 years ago

I altered the popover bindingHandler to handle a few more options and so we did not get the multiple apply bindings error. The root problem is because it is animating, the selector is saying it is visible, but it is actually going away. Not sure if you liked my other changes, but I thought I would show you the changes below:

ko.bindingHandlers.popover = { init: function(element, valueAccessor, allBindingsAccessor, viewModel, bindingContext) { var $element = $(element);

  // read popover options
  var popoverBindingValues = ko.utils.unwrapObservable(valueAccessor());

  // build up all the options
  var tmpOptions = {};

  // set popover title - will use default title attr if none given
  if (popoverBindingValues.title) {
     tmpOptions.title = popoverBindingValues.title;
  }

  // set popover placement
  if (popoverBindingValues.placement) {
     tmpOptions.placement = popoverBindingValues.placement;
  }

  // set popover container
  if (popoverBindingValues.container) {
     tmpOptions.container = popoverBindingValues.container;
  }

  // set popover delay
  if (popoverBindingValues.delay) {
     tmpOptions.delay = popoverBindingValues.delay;
  }

  // create unique identifier to bind to
  var uuid = guid();
  var domId = "ko-bs-popover-" + uuid;

  // if template defined, assume a template is being used
  var tmplDom;
  var tmplHtml = "";
  if (popoverBindingValues.template) {
     // set popover template id
     var tmplId = popoverBindingValues.template;

     // set data for template
     var data = popoverBindingValues.data;

     // get template html
     if (!data) {
        tmplHtml = $('#' + tmplId).html();
     } else {
        tmplHtml = function() {
           var container = $('<div data-bind="template: { name: template, if: data, data: data }"></div>');

           ko.applyBindings({
              template: tmplId,
              data: data
           }, container[0]);
           return container;
        };
     }

     // create DOM object to use for popover content
     tmplDom = $('<div/>', {
        "class": "ko-popover",
        "id": domId
     }).html(tmplHtml);

  } else {
     // Must be using static content for body of popover
     if (popoverBindingValues.dataContent) {
        tmplHtml = popoverBindingValues.dataContent;
     }

     // create DOM object to use for popover content
     tmplDom = $('<div/>', {
        "class": "ko-popover",
        "id": domId
     }).html(tmplHtml);
  }

  // create correct binding context
  var childBindingContext = bindingContext.createChildContext(viewModel);

  // set internal content
  tmpOptions.content = $(tmplDom[0]).outerHtml();

  // Need to copy this, otherwise all the popups end up with the value of the last item
  var popoverOptions = $.extend({}, ko.bindingHandlers.popover.options, tmpOptions);

  // see if the close button should be added to the title
  if (popoverOptions.addCloseButtonToTitle) {
     var closeHtml = popoverOptions.closeButtonHtml;
     if (closeHtml === undefined) {
        closeHtml = '&times';
     }
     if (popoverOptions.title === undefined) {
        popoverOptions.title = ' ';
     }

     var titleHtml = '<span class="text-info" <strong>' + popoverOptions.title + '</strong>  ';
     var buttonHtml = '<button class="close pull-right" type="button" data-dismiss="popover">' + closeHtml + '</button>';
     popoverOptions.title = titleHtml + buttonHtml;
  }

  // Build up the list of eventTypes if it is defined
  var eventType = "";
  if (popoverBindingValues.trigger) {
     var triggers = popoverBindingValues.trigger.split(' ');

     for (var i = 0; i < triggers.length; i++) {
        var trigger = triggers[i];

        if (trigger !== 'manual') {
           if (i > 0) {
              eventType += ' ';
           }

           if (trigger === 'click') {
              eventType += 'click';
           } else if (trigger === 'hover') {
              eventType += 'mouseenter mouseleave';
           } else if (trigger === 'focus') {
              eventType += 'focus blur';
           }
        }
     }
  } else {
     eventType = 'click';
  }

  // bind popover to element click
  $element.bind(eventType, function() {
     var popoverAction = 'toggle';
     var popoverTriggerEl = $(this);

     // Check state before we toggle it so the animation gives us the correct state
     var popoverPrevStateVisible = $('#' + domId).is(':visible');

     // show/toggle popover
     popoverTriggerEl.popover(popoverOptions).popover(popoverAction);

     // hide other popovers and bind knockout to the popover elements
     var popoverInnerEl = $('#' + domId);
     $('.ko-popover').not(popoverInnerEl).parents('.popover').remove();

     // if the popover was visible, it should now be hidden, so bind the view model to our dom ID
     if (!popoverPrevStateVisible) {

        ko.applyBindingsToDescendants(childBindingContext, popoverInnerEl[0]);

        /* Since bootstrap calculates popover position before template is filled,
           * a smaller popover height is used and it appears moved down relative to the trigger element.
           * So we have to fix the position after the bind
        */

        var triggerElementPosition = $(element).offset().top;
        var triggerElementLeft = $(element).offset().left;
        var triggerElementHeight = $(element).outerHeight();
        var triggerElementWidth = $(element).outerWidth();

        var popover = $(popoverInnerEl).parents('.popover');
        var popoverHeight = popover.outerHeight();
        var popoverWidth = popover.outerWidth();
        var arrowSize = 10;

        switch (popoverOptions.placement) {
        case 'left':
           popover.offset({ top: triggerElementPosition - popoverHeight / 2 + triggerElementHeight / 2, left: triggerElementLeft - arrowSize - popoverWidth });
           break;
        case 'right':
           popover.offset({ top: triggerElementPosition - popoverHeight / 2 + triggerElementHeight / 2 });
           break;
        case 'top':
           popover.offset({ top: triggerElementPosition - popoverHeight - arrowSize, left: triggerElementLeft - popoverWidth / 2 + triggerElementWidth / 2 });
           break;
        case 'bottom':
           popover.offset({ top: triggerElementPosition + triggerElementHeight + arrowSize, left: triggerElementLeft - popoverWidth / 2 + triggerElementWidth / 2 });
        }

        // bind close button to remove popover
        var popoverParent;
        if (popoverOptions.container) {
           popoverParent = popoverOptions.container;
        } else {
           popoverParent = popoverTriggerEl.parent();
        }
        popoverParent.on('click', 'button[data-dismiss="popover"]', function () {
           popoverTriggerEl.popover('hide');
        });
     } 

     // Also tell KO *not* to bind the descendants itself, otherwise they will be bound twice
     return { controlsDescendantBindings: true };
  });

}, options: { placement: "right", html: true, addCloseButtonToTitle: true, trigger: "manual" } };

billpull commented 10 years ago

@jfoegen could you put that into a pull request or gist or something to make it easier for me to test and merge in thanks.

ghost commented 10 years ago

gist:9748560 I guess I never submitted publically before. If you need more info let me know.

ghost commented 10 years ago

gist:9793327 - I made some other alterations for problems I was seeing when I had multiple popovers

LastElf commented 9 years ago

I got this bug with the latest version and a bit of a related issue maybe?

I have a popover as a hover on an input group button. This builds itself from a template so I can easily bind data to many of these all over the page (It's a crafting recipe in a game. There's going to be 100s of these hover elements in the end, all built from a foreach). For some reason the input-group-btn class has a 0 font size. If I don't force the font size I get the above error.

However, if I force the font size it shows up, no binding error and it handles the binding of the rest of the template fine. Data shows up and such. However, this happens:

http://i.imgur.com/7omQJRg.jpg

Misalignment on the button and the popover has some weird sizing. Is there something I'm missing in the template classes that should fix this?

zhakhalov commented 9 years ago

Hello. I got this bug too in foreach statement. Simple fix this error with cleaning the node

insert this line at line 184: ko.cleanNode(popoverEl[0]); so around code became to: ko.cleanNode(popoverEl[0]); if (data) { koObject.applyBindings({ template: template, data: data }, popoverEl[0]); } else { koObject.applyBindings(viewModel, popoverEl[0]);