billpull / knockout-bootstrap

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

Binding Helpers do not work well with frameworks that dynamically compose views #3

Open rwhepburn opened 11 years ago

rwhepburn commented 11 years ago

It seems some of the ko-bs bindingHandlers don't work well with frameworks that perform dynamic UI composition (e.g. DurandalJS). In the case of Durandal, the framework composes a view and view model on demand, binds them together and then adds them to the DOM.

In the case of the popover binding helper, when the popover Template is defined in the view that Durandal is binding, the Template will not have been added to the DOM when the binding helper is applied. The popover binding handler uses the JQuery selector var tmplHtml = $('#' + tmplId).html() to find the template for the popover. This means the binding helper will fail to find the template and tmplHtml will end up being "undefined". Once Durandal finishes composing the UI, it's only then added to the DOM, but it's too late for the popover binding helper by this point.

In order to define a popover template in the dynamically composed view, a workaround I used was to change the popover binding helper implementation to assume the template would be a sibling to the current element like so var tmplHtml = $(element.parentElement).find('#' + tmplId).html(). This allows JQuery to find the template because it is not searching the main DOM.

The drawback is that you have to make sure the template definition is always a sibling. Maybe it's possible, but I didn't have time to see if there is a way to identify the root of the current element and use that instead of the elements parent.

I haven't tried but it looks like the progress bar and alert binding helpers could have the same issue. Maybe there's a more generic way to support scenarios where UI is being bound to the View Models and then added to the main DOM?

Anyways, really liking these binding helpers! They do a great job helping to keep the code clean. Well done!

billpull commented 11 years ago

Hmm that is an interesting scenario that I have not come across yet. Thanks for pointing this out. I will look into your suggestion and experiment on my own to see if there is a better way to do this. One thing I did think of was passing the template in as a variable reference to a string

so something like var popoverTemplate = "<div>...</div>" and then it wouldnt be necessary to have the $('#'+tmplId).html() call. This somewhat breaks with the convention that templates are defined in script tags for knockout (something I am not really a fan of).

rwhepburn commented 11 years ago

Awesome! Oh and I forgot to include a sample of how I organized my markup:

<div class="pull-right">
  <span class="badge" data-bind="popover: { template: 'infoPopupTemplate', title: 'More Information', placement: 'left' }">
    <i class="icon-info-sign icon-white"/>
  </span>
  <script id="infoPopupTemplate" type="text/html">
    <button class="close pull-right" type="button" data-dismiss="popover">×</button>
    <div>More info goes here!</div>
  </script>                            
</div>
billpull commented 11 years ago

Dont know much about Durandal but would it be possible to put the

  <script id="infoPopupTemplate" type="text/html">
    <button class="close pull-right" type="button" data-dismiss="popover">×</button>
    <div>More info goes here!</div>
  </script>           

at the end of the body tag rather than inline with that div. I am not sure if that would fix the problem but something to try in the short term while I investigate other possibilities.

rwhepburn commented 11 years ago

Yeah, that works as a work-around but it kinda breaks how I want to organize and encapsulate the various UI assets/templates when working with a modular composition framework like Durandal (I'm sure there are other similar frameworks out there too that may have this issue).

neverfox commented 11 years ago

+1 for this, as a Durandal user.

antoniomourao commented 11 years ago

"Another one bites the dust.."

neverfox commented 11 years ago

@rwhepburn Have you tried this in Durandal 2.0 using the composition module's addBindingHandler method? So far, it has solved all my problems related to binding handlers that need a stable DOM before being applied.

antoniomourao commented 11 years ago

I will look into that, thanks for the tip.

Vik-- commented 11 years ago

@neverfox , I am struggling to have ko-bs popover in my durandal SPA. I want to try your tip of using addBindingHandler method by upgrading to durandal 2.0. I was wondering if you have any working example of the usage of ko-bs with durandal 2.0. Thanks in advance.

rwhepburn commented 11 years ago

Unfortunately, I haven't had a chance to look into this yet so I haven't made any progress porting my workaround to use addBindingHandler.

neverfox commented 11 years ago

@Vik-- Here is the basic format:

define [
  "durandal/composition"
  "knockout"
  "jquery"
  "bootstrap"
], (composition, ko, $) ->
  "use strict"

  composition.addBindingHandler "popover",
    update: (element, valueAccessor, allBindingsAccessor, viewModel, bindingContext) ->
      options = ko.utils.unwrapObservable valueAccessor()
      $element = $(element)
      popover = $element.data "popover"
      if popover
        $.extend popover.options, options
      else
        $element.popover options

This assumes that on my binding I'm passing in an object that fits the expected options format for this component. You could of course do something more custom than that if you wanted to. If you're using ko-bs, then you could just pass ko.bindingHandlers.popover like composition.addBindingHandler "popover", ko.bindingHandlers.popover.

I put each of these modules I create in a bindings path that my require config can find and then, in the view model that needs this, I add bindings/popover to the module's dependencies.