sproutcore/sproutcore20 — pull request #224 by manko (2011-11-30 23:02:49+00:00)
Title: Fix classNameBindings to work correctly with rerenders

Fix classNameBindings so that a class that was removed from DOM by an observer doesn't re-appear on the next rerender.

This happened because on the initial render, _applyClassNameBindings adds an observer *and* adds the class to view's classNames array (if the targeted property currently so dictates, as checked in _classStringForProperty). Now if the property changes, the observer removes the class from DOM, but not from classNames, and thus the next rerender produces the element with that class again.

Removing the class from classNames in the observer seems like the most sensible fix to me. I believe it cannot be fixed by removing the old class from classNames in _applyClassNameBindings (outside the observer function, that is), because for non-boolean properties we don't know what the old class might have been (or whether it even has changed, in fact).

Another possible fix might be to split the logic into two parts: one set of observers that keeps the classNames array up-to-date based on the paths listed in classNameBindings, and a separate observer that keeps CSS classes in DOM in sync with the classNames array. This would be a bit bigger change I guess, but perhaps a little bit cleaner end result, and would have the added bonus of allowing classNames to be updated in other ways, not just through classNameBindings (which does appear to usually be enough, though).