Collectionbinder sort static content

[amakhrov]

When we call collectionBinder.bind we pass a container to it.
Sometimes this container includes some content that is not a part of the collection itself - e.g. table header. I call such content "static content", as it's not supposed to be dynamically changed with collectionBinder.

So the issue is that the static content moves to the bottom of the container each time sortRootEls is called. The expected behavior I think is not to touch the static content during sorting at all.

The added unit tests demonstrate the issue.
The suggested solution is to check the position of the first collection element before the sorting starts, and then adjust the algorithm to take that into account.

[platinumazure]

Not that I don't support this, but I wonder if it would make things simpler to just have the CollectionBinder require an empty parent element?

The "static content" case is then handled by adding another container for the bound models themselves:

<div class="containerWithStaticStuff">
  <div class="theStaticContent"></div>
  <div class="actualCollectionBinderParentElement">
    <!-- This element is where actual binding should happen -->
  </div>
</div>

As it is, it's sort of a behavioral quirk that we happen to be using .append fairly exclusively and so the static content "works". In particular, it does not work if we wanted the static content to be at the bottom, etc. Admittedly, that would probably be a breaking change and should go in with a new release version; but I think it would make things easier to reason about here.

Just my 2 cents. I'm otherwise happy to see this go in-- just think we're fixing a problem we could easily avoid.

[rogerroelofs]

My thoughts exactly. In the OP's use case, pointing the Collectionbinder
at an empty tbody element solves all the problems as far as I can see.

On Fri, Feb 20, 2015 at 10:25 AM, Kevin Partington <[email protected]

wrote:

Not that I don't support this, but I wonder if it would make things
simpler to just have the CollectionBinder require an empty parent element?

The "static content" case is then handled by adding another container for
the bound models themselves:

As it is, it's sort of a behavioral quirk that we happen to be using
.append fairly exclusively and so the static content "works". In
particular, it does not work if we wanted the static content to be at
the bottom, etc. Admittedly, that would probably be a breaking change and
should go in with a new release version; but I think it would make things
easier to reason about here.

Just my 2 cents. I'm otherwise happy to see this go in-- just think we're
fixing a problem we could easily avoid.

—
Reply to this email directly or view it on GitHub
#210 (comment)
.

Roger

Roger Roelofs
Know what you value.

[amakhrov]

Adding another container works  if you have div-based markup.
In my cases i need to  display table data and benefit from automatic column sizing - so i use <table>. What's more, I need to group related rows, so i use multiple tbody elements for that (each collection model is associated with tbody, not tr). Looks like there is no way to create nested tbody. So i ended up with having my collection-level tbodies in the same container with thead.

I realize it's quite a rare case, and an alternative solution would be to use js for manually handling groups of rows instead of using multiple tbodies.
So I in no way insist on this change - however just curious if anyone else has come across a similar issue.

Sent from my T-Mobile 4G LTE Device

-------- Original message --------

From: Kevin Partington

Date:02/20/2015 7:25 AM (GMT-08:00)

To: "theironcook/Backbone.ModelBinder"

Cc: amakhrov

Subject: Re: [Backbone.ModelBinder] Collectionbinder sort static content (#210)

Not that I don't support this, but I wonder if it would make things simpler to just have the CollectionBinder require an empty parent element? The "static content" case is then handled by adding another container for the bound models themselves:

As it is, it's sort of a behavioral quirk that we happen to be using .append fairly exclusively and so the static content "works". In particular, it does not work if we wanted the static content to be at the bottom, etc. Admittedly, that would probably be a breaking change and should go in with a new release version; but I think it would make things easier to reason about here.

Just my 2 cents. I'm otherwise happy to see this go in-- just think we're fixing a problem we could easily avoid.

—
Reply to this email directly or view it on GitHub.

[platinumazure]

@amakhrov I'll admit I hadn't thought of that possibility. You are right that tables do not support nesting tbody or other related elements to make this work properly.

I guess in that case, I would rather see support for "extra" elements be made explicit and not automatic. (For example, if the ViewManagerFactory takes in as an option { prefixHtml: "<div>some html</div>", postfixHtml: "<div>some more html</div>" } or similar.)

Again, that's totally a breaking change and I don't insist on this pull request being closed/altered to acommodate my design philosophy. I accept that this pull request is solving a real issue that was exposed by the auto-sort feature. I'm just thinking long-term: in my view, for 2.x, we need to consider solidifying the design of CollectionBinder and making its behavior simpler in the default case (and requiring consumers to opt in to more complicated behavior).