Update actions to kebob case & arguments. #159
Conversation
Robdel12
force-pushed
the
fs/rename-actions
branch
2 times, most recently
from
8d4a05a to
9dcc22e
Compare
| // action body... | ||
| } | ||
| ``` | ||
|
|
||
| Most of the time all you need is the value that has been selected, but | ||
| sometimes your action requires more context than just that. In those | ||
| cases, you can associate arbitrary attributes with the component | ||
| itself and use them later inside your action handler. For example: | ||
| cases, you can use the `componentState` argument. It contains a couple |
There was a problem hiding this comment.
I'm a little confused about componentState thing. Should there be an actual argument to the component called componentState? In the example, I just see action and required
There was a problem hiding this comment.
Ah, ok, I see now that componentState is a hash of potential arguments (like isRequired, isMultiple, etc.). Could we make it clearer that componentState isn't the actual component property that you'll be using, but is instead a componentState hash?
| this.sendAction('action', nextValue, this); | ||
| this.sendAction('onchange', this, nextValue, event); | ||
| this.sendAction('action', nextValue, event, this.get('componentState')); | ||
| this._handleAction('on-change', nextValue, event); |
There was a problem hiding this comment.
Does this dispatch correctly? I'm concerned that bubbling an action with sendAction AND calling the closure action could just dispatch the action twice? Is sendAction a no-op if a closure action is passed in or something?
Now we send the same arguments for all of the actions. The value of the select & the event.
Robdel12
force-pushed
the
fs/rename-actions
branch
from
9dcc22e to
a575e12
Compare
Robdel12
force-pushed
the
fs/rename-actions
branch
from
157811a to
8b17929
Compare
| let actionValue = this.get(action); | ||
|
|
||
| if(typeof actionValue === 'string') { | ||
| Ember.warn(`x-select: You must use the action helper for all actions. The try: ${action}=(action "${actionValue}") in your template`, false, {id: 'x-select-string-action'}); |
There was a problem hiding this comment.
"The try: "
Seems like a typo?
| return; | ||
| } | ||
|
|
||
| this.get(action)(value, event); |
There was a problem hiding this comment.
This is the right track, but I think we can make it a bit clearer and improve the error handling even more.
_handleActionisn't really what it's doing. It seems like it's dispatching the action so that something else can handle it.callAction,invokeAction,doActionwould be better candidates.- We issue a warning if
actionValueis a string, but it if we're going to go to the effort of performing an explicit check on the type of the action value, then we should issue a warning if it's anything except a function. Because anything except a function will fail. - If we've already got the action in the form of
actionValue, then let's just call it:
actionValue(value, event);
- see adopted-ember-addons#159 - change event names to kebab-case to smooth transition from 2.x to 3.x - change argument order to match 3.x - NOTE: last argument `component` is deprecated in 3.x
Now we send the same arguments for all of the actions, they're kebob cased, and are closure actions.