classNameBindings as a computed property

[YoranBrondsema]

Hi,

I'm struggling to find a clean solution to a use-case where I think having classNameBindings as a computed property would provide a solution.

I have a following component. It has a computed class side which takes values 'left' or 'right'. The property side is quite expensive to compute, i.e. it requires network requests.

extend default Ember.Component.extend({
  classNameBindings: ['side'], // either 'left' or 'right'
  side: function() {
    // some expensive operation involving network requests
  }.property(...)
});

Now there are cases where I don't actually need to have the class defined on the element. In those cases, I would like to skip the calculation of side altogether. Ideally, I would be able to do something like:

extend default Ember.Component.extend({
  classNameBindings: function() {
    if (this.get('sideIsNotRequired')) {
      return [];
    } else {
      return ['side'];
    }
  }.property('sideIsNotRequired'),
  side: function() {
    // some expensive operation involving network requests
  }.property(...)
});

However, this throws a Assertion Failed: Only arrays are allowed for 'classNameBindings' error. Now I have to resort to something like:

extend default Ember.Component.extend({
  classNameBindings: ['side'], // either 'left' or 'right'
  side: function() {
    if (this.get('sideIsNotRequired')) {
      return null;
    } else {
      // some expensive operation involving network requests
    }
  }.property('sideIsNotRequired', ...)
});

But this isn't as nice semantically. Also, the same logic would have to be repeated for each class that might or might not be required.

Now I could also create a containing <div> in the template:

<div class={{theClasses}}>
  ...
</div>

with in the component

extend default Ember.Component.extend({
  theClasses: function() {
    if (this.get('sideIsNotRequired')) {
      return '';
    } else {
      return `${this.get('side')}`;
    }    
  }.property('side', 'sideIsNotRequired'),
});

But this creates an extra HTML element.

To conclude, my question is more like a feature request. It feels kind of like an artificial limitation to me that classNameBindings has to be a static array, especially given the previous workaround.

Are there any plans to make classNameBindings a computed property? I'm sure there are technical reasons of why it's not yet the case. If so, would those be hard to overcome?

[pittst3r]

But this isn't as nice semantically.

This seems better semantically IMO. Your "ideal" example does the same thing but is less clear what's going on.

Also, the same logic would have to be repeated for each class that might or might not be required.

I'm not sure how this could be avoided even in your preferred syntax. You might be able to cram more logic into a single function but you wouldn't want to do that anyway.

[YoranBrondsema]

I guess it's a matter of personal preference what is clearer or not. In my mind, the choice of whether or not to include a certain CSS class belongs in the definition of classNameBindings. That is clearest to me.

Having said that, I still think the way classNameBindings works now is limiting compared to what you can do in your Handlebars templates, where you have full power over what CSS classes you add and what value they have (not just the "what value they have" part).

I understand this is not a high priority issue. If someone picks it up to work on it, cool, in the other case it's cool too.

[pixelhandler]

@YoranBrondsema this seems more like a Question rather than an Ember.js issue.

Were the docs clear regarding the use of classNameBindings?

Also, regarding the question "Are there any plans to make classNameBindings a computed property?"

Perhaps search the RFC repo's issues / pull requests. That is where the plans are formulated for features and enhancements - https://github.com/emberjs/rfcs

[acorncom]

@YoranBrondsema Not sure this handles it for you exactly (as some of the logic has to bleed out into the containing template for a component), but you can set a component's class from the containing template (see this Twiddle for an example: https://ember-twiddle.com/4eb2cfc4b98109e43175c97a1d0d77fe?openFiles=templates.application.hbs%2Ctemplates.components.my-component.hbs)

If moving some of the calculations about whether side is available up a level works in your case, that might be a way to achieve what you're after ...

[YoranBrondsema]

Thank you for the suggestion @acorncom. That works, but indeed as you say logic that belongs inside the component must be implemented in the containing template :-(.

This is kind of my point as well. There are many ways to solve this case, Ember.js is so flexible in the use of computed properties inside templates, therefore it feels strange that there's such a limitation on classNameBindings.

@pixelhandler Good idea but I'm not sure this belongs in an RFC as I don't think it's substantial enough. Maybe a core team member can advise on this issue?

[pixelhandler]

@YoranBrondsema if you'd like to propose that the core team consider a feature/enhancement your best bet would be an RFC issue.

To conclude, my question is more like a feature request. It feels kind of like an artificial limitation to me that classNameBindings has to be a static array, especially given the previous workaround.

Are there any plans to make classNameBindings a computed property? I'm sure there are technical reasons of why it's not yet the case. If so, would those be hard to overcome?

It sounds like you'd like to propose that classNameBindings can be a static array or a computed property.

Since there many ways to accomplish conditional classnames in a component perhaps the path forward is to use the template, perhaps {{my-component class=(if meetsCondition 'complex-condition' 'simple')}}

See CONTRIBUTING.md#requesting-a-feature which recommends creating an RFC Issue to request a feature/enhancement (no need for a PR with RFC, only an issue in the repo)

There has been some history in the Ember.js issue tracker here in this repo for "feature" issues but that has moved over to the RFC's repo. The issues typically for tracking bugs, or potential bugs.

[YoranBrondsema]

@pixelhandler Gotcha. I opened an RFC issue here: emberjs/rfcs#138

[stefanpenner]

closing in-favor of the referenced rfc