sendAction() not working on 1.12

[henrymazza]

At least on Ember 1.12.1 the line:

      _this.sendAction( changeAction, _this.slider.val() );

works only if you change it to:

      _this.sendAction( 'change', _this.slider.val() );

It is, sendAction() must receive the name of the property that holds the action’s name as a value and not the name itself!

[kennethkalmer]

Thanks so much for reporting! Will get a test in place and an update out a bit later today!

[knownasilya]

Would love a fix 👍

[knownasilya]

See my PR.

[kennethkalmer]

I was just thinking through the API a bit, and I'm wondering if we shouldn't rather omit an error if the values of the change and slide attributes that were passed in aren't strings. Hardcoding the change and slide action names feels wrong to me.

I was also skimming through the changelogs and I could not find any announced changes to sendAction in components. However, I did find emberjs/ember.js@7301768fbb14 (which made it into 1.13.0 onwards) that suggests that we could pass both a function or a string to sendAction and it will work as expected.

That leaves the problem of coping with Ember pre-1.13... Given that 1.13 is the first LTS release, and Ember 2.0 is on the horizon, maybe it is worth officially supporting on 1.13 and up and making a note in the related places in the documentation that older Ember versions should only pass in strings...

Wdyt @knownasilya @henrymazza ?

[knownasilya]

@kennethkalmer The convention is to have a static API that isn't changed. The component shouldn't care what the user passes in for the action.

{{my-comp myAction='usersAction'}}

// in my component
this.sendAction('myAction', data);

myAction will always be the same, because that is the API you are adhering too for your component. In 1.13, the new function syntax is available on this.attrs.myAction() while the old is still sendAction. This issue is for making this component usable in this major version. You shouldn't need to get the value of the actions for anything that you are doing, with the current sendAction setup you will also support the new syntax. You can remove the old syntax in a major version.

See the improved actions rfc, https://github.com/emberjs/rfcs/blob/00ac2685c86f27d41547012903f485a4ef338d27/active/0000-improved-actions.md

[henrymazza]

I think the events shouldn’t be exposed, it’s not very Ember_y_. The action will update a value, so just do it directly inside the component, the component exists to take away this complexity from your code.

Also, it’s what every built-in ember view/component do, it sets the streamingBinding right upon being edited — see how an input behaves. I’d agree that for the slider the event rate may be too high and some cases of live updating isn’t desirable (e.g. if it generates a lot of observes firing), so an option to only update the value upon change instead of slide could be handy.

Example:

{{range-slider value=allowance live=false step=0.10}}
{{input value=allowance type="number" step=0.10}}

See how clear the API gets? Or perhaps it’s a step too far?

[knownasilya]

@henrymazza I disagree, because Ember is headed into the "data down, actions up" methodology, like react, and this would be completely against that. It causes observer hell if you want to do anything mildly complex.

[henrymazza]

Ok. So are they gonna change the way the input components work today? 

-- 
Fabio Mazarotto

On 18 de junho de 2015 at 16:30:23, Ilya Radchenko (notifications@github.com) wrote:

@henrymazza I disagree, because Ember is headed into the "data down, actions up" methodology, like react, and this would be completely agains that. It causes observer hell if you want to do anything mildly complex.

—
Reply to this email directly or view it on GitHub.

[knownasilya]

@henrymazza you will be able to do <input type="text" onchange=(action 'name')>, i.e. use the DOM's event attributes.

[henrymazza]

So onslide and onchange FTW?

-- 
Fabio Mazarotto

On 18 de junho de 2015 at 17:13:19, Ilya Radchenko (notifications@github.com) wrote:

@henrymazza you will be able to do <input type="text" onchange=(action 'name')>

—
Reply to this email directly or view it on GitHub.

[knownasilya]

sounds like a win 👍

[kennethkalmer]

Thanks for the input guys, I'll review the RFC too and see what onchange and onslide would look like and work in the deprecation messages accordingly. If is looks good, I'll bump to a 1.0.0 to signal a major API breakage.

Are we happy with only supporting 1.13 and up from this API change?

[kennethkalmer]

Hmm, closed as a side-effect of merging in #6, but the question around 1.13 and API changes still hold