Html classes repeated when binding to itemViewClass's attributes

[drogus]

I will try to add failing test for this, but for now here is error reproduced on jsfiddle: http://jsfiddle.net/b4RMr/10/

Both boxes have classes from bindings repeated twice. When you change classNameBinding to "parentView.foo" instead of just "foo" it starts to work correctly.

[drogus]

I added failing test for the issue.

The biggest problem with this are not duplicated classes, but the fact that after a few attribute changes old classes are not always removed, so you end up with mixed new and old classes. It seems that the cause of the problem is using bindings to parent view (or probably more generally - multiple chained bindings), when you remove bindings and use direct paths such as "parentView.parentView.content.foo" it works correctly.

[drogus]

I thought that the issue lays somewhere inside _applyClassNameBindings, but it's actually caused by how bindings work. The problem is, classNameBindings generate a class name twice if bindings chain is used (by chain I mean binding through at least 2 bindings).

Here is a view that shows the problem:

  TemplateTests.MyView = SC.View.extend({
    foo: "foo",
    barBinding: "foo",
    bazBinding: "bar",
    quxBinding: "baz",
    fooBarBinding: "qux",
    fooBazBinding: "fooBar",
    classNameBindings: ["fooBaz"],
    classNames: ["child"]
  });

Interestingly, the above view produces sc-view child foo foo class, so foo is repeated only once.

Essentially the problem with mixed classes looks like that:

• classNameBindings produces more than one class
• _applyClassNameBindings adds observers to properties, for example for foo property
• when value changes, observer is run and it removes class here: https://github.com/sproutcore/sproutcore20/blob/2fb9d8c250/packages/sproutcore-views/lib/views/view.js#L249
• but since there is more than one class it removes only one of them and adds new one and we end up with both classes

So the quick fix is to make array of class names unique here: https://github.com/sproutcore/sproutcore20/blob/2fb9d8c250/packages/sproutcore-views/lib/views/view.js#L756, but it does not fix the real issue with binginds.

[drogus]

I commited my quick nasty fix for it if someone needs it fixed now :)

[erichocean]

Seems like the actual issue is that all bindings need to be synced before _applyClassNameBindings is run.

[wagenet]

@drogus, are you still seeing this issue on master?

[wagenet]

Now that I think about it, this sounds similar to an issue that @ebryn ran across recently.

[ebryn]

Yeah, this looks pretty similar to something I saw the other day. Thanks @wagenet, will try to fix this next week.

[manko]

I happened to bump into this issue and looked a bit into it. My interpretation is that there's actually two problems, both triggered by either an explicit rerender call or a view being removed and then re-appended to the DOM:

1. The observer set up in _applyClassNameBindings removes oldClass from DOM, but not from the classNames array. So if on the first render the binding results in a class being added, and then later the observer removes it from DOM, it will get added back on next render because it's still in classNames.
2. In _applyClassNameBindings, dasherizedClass is pushed to classNames without checking if it's already there. This means that on each rerender, the classes get re-added. (Happens regardless of any observers firing, i.e. regardless of point 1.)

I was able to fix the problems I was seeing by changing said observer to also remove oldClass from classNames, and in the setup in _applyClassNameBindings only push dasherizedClass to classNames if not already there. I can try to find the time to do a proper commit & pull request later, hopefully by next week.

[wagenet]

Should be fixed in master now.

[manko]

Great, I can confirm my problem number 2 is now gone. However, problem number 1 still stands. I wrote a test case and a fix, pull request here: https://github.com/sproutcore/sproutcore20/pull/224 (I guess it's really a separate issue in the end, not directly related to this one)