Open MarkMT opened 7 years ago
One extra comment... in Components - Handling Events, I think the section "Sending Actions" is perhaps poorly named. In context, it seems to me that the real issue being discussed there is the fact that you can access event data by putting the event object in the handler function signature. The example used involves an action passed into the component from the containing template - which is an entirely reasonable way to illustrate the use of event data, but to me that aspect seems somewhat incidental to the real point of this section. Also as noted above, passing actions to components is dealt with in detail in Components - Triggering Changes with Events.
Here is a draft of what I have in mind for Components - Handling Events:
https://github.com/MarkMT/guides/blob/actions_guide/source/localizable/components/handling-events.md
There are some issues related to the way ember's handling of events is described in the guides that I think need some attention. Part of my concerns were detailed previously in https://github.com/emberjs/guides/issues/1345. However one aspect of that issue (the use of
sendAction
to invoke actions in a component) has since been addressed in https://github.com/emberjs/guides/commit/4f6e84ad701e03ef85e87be2299252f0d0197364 and there are some additional related concerns not mentioned there that I think also need improvement. So the following is intended to capture a summary of my current concerns as a point of reference for a coming PR.Currently event handling is dealt with in three places:
Issues In Templates - Actions, two different usages of the
action
helper are presented in different sections. The initial examples use the helper in the element space context (e.g.<button {{action "toggleBody"}}>
; terminology from the API docs), while later there is a discussion under "Modifying the action's first parameter" that uses an example of the helper in the attribute context to illustrate the use of thevalue
option (i.e.onblur={{action "bandDidChange"}}
). However there is no mention of the fact that the latter example represents a different usage from the one presented earlier or of its significance. There is no explanation of the fact that using theaction
helper in the attribute context produces a closure action or what that means. While the example that uses thevalue
option mentions the fact that the action in this context takes the event object as an argument, since the fact that this usage is different from the original examples is never mentioned, it's not at all clear to a new reader that this (i.e. the use of the event object) applies specifically to that context and not to the element space context.Because there is no identification of closure actions as being distinct from those defined in the element space context, there is also no discussion of the use of explicit action arguments in that context and no indication of how explicit arguments and the implicit event argument relate to one another, i.e. what order they get passed to the action.
Since there is no discussion of arguments in the context of closure actions, there is also no mention of currying or why that is useful. So the possibility of nested actions shown in the API docs is not covered. Including some discussion of this issue here would provide helpful background for when the issue arises later on in the context of invoking external actions inside components in Components - Triggering Changes with Events.
The fact that closure actions automatically receive the event object is mentioned in Components - Handling Events (text and examples beginning "Another way to preserve native event behaviors and use an action..."). This was added in https://github.com/emberjs/guides/pull/1624 in response to https://github.com/emberjs/ember.js/issues/13806. However here again, what a closure action is is not explained and the presentation is confusing (to me as a fairly new Ember user) because of the context in which it appears - i.e. the handling of events on a component as a whole. This obscures that fact that closure actions receive the event object any time they are used, e.g. in handling events on elements, in any execution context (component or controller), or as properties passed into a component from a parent context.
This also leads to some confusing advice:
The uninformed reader will easily interpret this as describing two alternative ways of accomplishing the same goal, when in fact they are two different contexts in which the event object is made available (an event on a component vs an event on an element).
The discussion in this section also refers to "The normal behavior for a function defined in
actions
" in reference to actions defined in the element space context. Yet it's not actually clear from the text that this is what is meant by "normal", and I would also suggest that the notion that one usage is "normal" and the other not is highly debatable.If closure actions were dealt with more fully in Templates - Actions, the discussion about inline event handlers in Components - Handling Events could be removed from this page completely. This would remove a potential source of confusion and also make the focus of the page - handling events on a component as a whole - much clearer.
Incidentally, the discussion about "Sending Actions" in Components - Handling Events should probably include a reference to Components - Triggering Changes with Events since the construct is essentially the same and is discussed in much more detail there.
I have a PR in development. A draft of the changes I propose to Templates - Actions can be seen at https://github.com/MarkMT/guides/blob/actions_guide/source/localizable/templates/actions.md. This potentially also addresses https://github.com/emberjs/guides/issues/1719, at least in part.
(I also have a few concerns about clarity in some parts of Components - Triggering Changes with Events, but I think those can be addressed separately)