CSS Binding does not handle null value properly

[Gle2nn]

If the css binding is passed a null binding value the binding will remove any classes added by the class binidng from the subject HTML node.

Why? The update method of the css binding verifies that the binding value is non-null and an object before processing it as described in the documentation. If the value is null or not an object the css binding invokes the update method of the class binding. The class binding removes any classes previously added using the binding and then adds the classes contained in the binding value (the null value in this case).

I think the fix is simple: if the css binding is passed a null it should not invoke the class binding update method.

[webketje]

Then how would one instruct to remove all classes at once? They would need to provide false values for all css-bound classes. Such a change would not be backwards-compatible if users previously relied on the css binding behavior to remove all classes, over what the attr: { class: ... } binding might instruct.

[Gle2nn]

Then how would one instruct to remove all classes at once? They would need to provide false values for all css-bound classes. Such a change would not be backwards-compatible if users previously relied on the css binding behavior to remove all classes, over what the attr: { class: ... } binding might instruct.

If the CSS binding is used as documented, with an object as the binding value, then you've already done the work because each class has an associated expression that determines whether the class is added to the element. If the expression is an observable then KO will do the updates automatically.

If the CLASS binding is used then you need to determine the set of classes and wrap it into a string for the binding value. To clear all classes you can use null, an empty string, or (I suspect) a string consisting of whitespace only.

it's unclear to me why one would use both bindings to manipulate the same CSS class.

The documentation says you can use both the CSS and CLASS bindings on the same element so long as the sets of class names don't intersect. It also says that the CSS binding can be used with a string value for backward compatibility (null isn't a string).

The concern about backward compatibility is valid. To determine what should be done, if anything, should start with the intent of the API. If the intention is to treat null like a string and forward the update to the class binding method so be it, but this should be documented. If, however, the intention was to treat null as if it was an object with no members ({}), then the CSS binding has a bona fide defect that should be fixed.

A check for a non-null value before calling the CLASS binding would limit the application of the backward compatibility support. You can clear all classes in this way by using an empty string.

FWIW, I encountered this behaviour while working on a set of reusable templates that take parameters for things such as the CSS, CLASS and STYLE bindings. I was specifying a string for the CLASS binding and the parameter for the CSS binding was undefined. It took a bit of splunking around to figure out why my CLASS binding wasn't working (it was, of course, but the CSS binding undid it).

I'm currently avoiding the issue by using an empty object and things work as expected now. Bindings like ATTR understand that they shouldn't do anything with a null value (i.e. attr: {title: null }) so the behaviour of the CSS binding struck me as odd.

[mbest]

@Gle2nn This seems reasonable, but the backwards compatibility issue is important. I'll look through the previous implementations to see if we actually handled that correctly.

[mbest]

This was a deliberate decision to handle null in the string case. See #1468

[Gle2nn]

This was a deliberate decision to handle null in the string case. See #1468

Fair enough and thanks for checking into it.

It may be worth adding some documentation to the effect that using the css binding with a null object may cause unintended consequences. Using an object with no properties will, in effect, be a no-op for this binding.

The interplay between the css and class bindings is, to me, unfortunate. In retrospect it may have been better to leave css as it was (i.e. string based) and introduced a new binding (maybe 'rules') that has the behaviour of the css binding as it is defined today (ignoring the backwards compatibility support). It would be great to see the interplay between the two bindings described clearly in the documentation.

Given the ease in which bindings can be added, it may make sense in the long run to deprecate css and class and add two new bindings with clearer behavour. Just my $0.02, fwiw.

Thanks again, cheers.