Get further on classNameBindings

[mitchlloyd]

This gets the bulk of the classNameBindings syntax working (2 additional tests passing).

The logic added to -normalize-class is stolen almost verbatim from class_name_binding.js. I believe this file can be removed.

A couple of other tests around classNameBindings are failing because spaces are being added to classes when returning nothing from normalizeClass. These remain skipped for now.

[ef4]

Woohoo, nice, a glimmer PR. Looks good to me.

[mitchlloyd]

It would be possible to get 2 more tests passing if we decide that it's okay to leave some extra spaces in the class attribute.

Right now, spaces are placed between every class name when building templates. When subexpressions for class name bindings are later evaluated they might be null, leaving an empty spot surrounded by spaces:

<div class="is-urgent static"></div>
<!-- later isUrgent is false resulting in an extra space -->
<div class=" static"></div>

Personally this behavior doesn't bother me and even serves as a signal that there may be a class in that empty spot at some point. However, I ran into a similar issue with spaces when testing a React component and it was a little bit of a gotcha.

Is it worth trying to normalize class name spacing somewhere or are we okay with extra spaces? If we're good with the spaces, I can update the tests to use jQuery hasClass helpers and the like rather than matching against the exact string value of the class attributes.

[ef4]

It would be great if you also want to work on removing the extra spaces to fix those other tests. The concat hook (https://github.com/emberjs/ember.js/blob/d13a1240c4bb344e9a58dc53d9e0499d31c38f8c/packages/ember-htmlbars/lib/hooks/concat.js) could be extended to take a separator option, and the underlying stream concat that it's based on (

ember.js/packages/ember-metal/lib/streams/utils.js

Line 171 in d13a124

export function concat(array, separator) {

) already offers a separator option.

Then you'd want to compose concat with some new filter hook that can strip out the nulls.

@mmun can probably tell us if that's a reasonable idea, he's a streams master.

[mitchlloyd]

@ef4 That makes a lot of sense. I might need some help from @mmun on the filter or compact hook, but I'd be willing to take a shot at it.