-
-
Notifications
You must be signed in to change notification settings - Fork 4.2k
[Glimmer2] Add support for target actions and move related ember-view tests to new harness #13532
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Closed
Closed
Changes from all commits
Commits
Show all changes
3 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
217 changes: 217 additions & 0 deletions
217
packages/ember-glimmer/lib/ember-views/target-action-support.js
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,217 @@ | ||
| import { Mixin } from 'ember-metal/mixin'; | ||
| import TargetActionSupport from 'ember-runtime/mixins/target_action_support'; | ||
| import alias from 'ember-metal/alias'; | ||
| import { computed } from 'ember-metal/computed'; | ||
| import { assert } from 'ember-metal/debug'; | ||
| import { inspect } from 'ember-metal/utils'; | ||
| import isNone from 'ember-metal/is_none'; | ||
| import { get } from 'ember-metal/property_get'; | ||
| import { ARGS } from '../component'; | ||
|
|
||
| /** | ||
| `Ember.ComponentTargetActionSupport` is a mixin that can be included in a | ||
| component class to add a `triggerAction` method with semantics similar to | ||
| the Handlebars `{{action}}` helper. By default the action's target is the | ||
| component itself, or the controller of the component if it is an outlet. | ||
|
|
||
| Note: In normal Ember usage, the `{{action}}` helper is usually the best | ||
| choice. This mixin is most often useful when you are doing more complex | ||
| event handling in custom component subclasses. | ||
|
|
||
| For example: | ||
|
|
||
| ```javascript | ||
| App.SaveButtonComponent = Ember.Component.extend(Ember.ComponentTargetActionSupport, { | ||
| action: 'save', | ||
| click: function() { | ||
| this.triggerAction(); // Sends the `save` action, along with the current context | ||
| // to the current controller | ||
| } | ||
| }); | ||
| ``` | ||
|
|
||
| The `action` can be provided as properties of an optional object argument | ||
| to `triggerAction` as well. | ||
|
|
||
| ```javascript | ||
| App.SaveButtonComponent = Ember.Component.extend(Ember.ComponentTargetActionSupport, { | ||
| click: function() { | ||
| this.triggerAction({ | ||
| action: 'save' | ||
| }); // Sends the `save` action, along with the current context | ||
| // to the current controller | ||
| } | ||
| }); | ||
| ``` | ||
|
|
||
| @class ComponentTargetActionSupport | ||
| @namespace Ember | ||
| @extends Ember.TargetActionSupport | ||
| @private | ||
| */ | ||
| export default Mixin.create(TargetActionSupport, { | ||
| /** | ||
| @property target | ||
| @private | ||
| */ | ||
| target: computed(function () { | ||
| let outletState = get(this, 'outletState'); | ||
| return outletState ? getCurrentOutletState(outletState).render.controller : undefined; | ||
| }), | ||
|
|
||
| // @override | ||
| send(actionName, ...args) { | ||
| let target; | ||
| let action = this.actions && this.actions[actionName]; | ||
|
|
||
| if (action) { | ||
| var shouldBubble = action.apply(this, args) === true; | ||
|
||
| if (!shouldBubble) { return; } | ||
| } | ||
|
|
||
| if (target = get(this, 'target')) { | ||
| assert( | ||
| 'The `target` for ' + inspect(this) + ' (' + target + | ||
| ') does not have a `send` method', | ||
| typeof target.send === 'function' | ||
| ); | ||
| target.send(...arguments); | ||
| return; | ||
| } | ||
| assert(`${inspect(this)} had no action handler for: ${actionName}`, action); | ||
| }, | ||
| /** | ||
| TODO: This looks like it's not even used by the view layer. Deprecate and remove? | ||
| @property actionContext | ||
| @private | ||
| */ | ||
| actionContext: alias('target'), | ||
| /** | ||
| If the component is currently inserted into the DOM of a parent component, this | ||
| property will point to the parent's controller, if present, or the parent itself. | ||
|
|
||
| @property targetObject | ||
| @type Ember.Controller | ||
| @default null | ||
| @private | ||
| */ | ||
| targetObject: computed('parentView', function(key) { | ||
| if (this._targetObject) { return this._targetObject; } | ||
| let parentComponent = get(this, 'parentView'); | ||
| if (parentComponent) { | ||
| let outletState = get(parentComponent, 'outletState'); | ||
| return outletState ? getCurrentOutletState(outletState).render.controller : parentComponent; | ||
| } | ||
| return null; | ||
| }), | ||
| /** | ||
| Calls a action passed to a component. | ||
| For example a component for playing or pausing music may translate click events | ||
| into action notifications of "play" or "stop" depending on some internal state | ||
| of the component: | ||
| ```javascript | ||
| // app/components/play-button.js | ||
| export default Ember.Component.extend({ | ||
| click() { | ||
| if (this.get('isPlaying')) { | ||
| this.sendAction('play'); | ||
| } else { | ||
| this.sendAction('stop'); | ||
| } | ||
| } | ||
| }); | ||
| ``` | ||
| The actions "play" and "stop" must be passed to this `play-button` component: | ||
| ```handlebars | ||
| {{! app/templates/application.hbs }} | ||
| {{play-button play=(action "musicStarted") stop=(action "musicStopped")}} | ||
| ``` | ||
| When the component receives a browser `click` event it translate this | ||
| interaction into application-specific semantics ("play" or "stop") and | ||
| calls the specified action. | ||
| ```javascript | ||
| // app/controller/application.js | ||
| export default Ember.Controller.extend({ | ||
| actions: { | ||
| musicStarted() { | ||
| // called when the play button is clicked | ||
| // and the music started playing | ||
| }, | ||
| musicStopped() { | ||
| // called when the play button is clicked | ||
| // and the music stopped playing | ||
| } | ||
| } | ||
| }); | ||
| ``` | ||
| If no action is passed to `sendAction` a default name of "action" | ||
| is assumed. | ||
| ```javascript | ||
| // app/components/next-button.js | ||
| export default Ember.Component.extend({ | ||
| click() { | ||
| this.sendAction(); | ||
| } | ||
| }); | ||
| ``` | ||
| ```handlebars | ||
| {{! app/templates/application.hbs }} | ||
| {{next-button action=(action "playNextSongInAlbum")}} | ||
| ``` | ||
| ```javascript | ||
| // app/controllers/application.js | ||
| App.ApplicationController = Ember.Controller.extend({ | ||
| actions: { | ||
| playNextSongInAlbum() { | ||
| ... | ||
| } | ||
| } | ||
| }); | ||
| ``` | ||
| @method sendAction | ||
| @param [action] {String} the action to call | ||
| @param [params] {*} arguments for the action | ||
| @public | ||
| */ | ||
| sendAction(action, ...contexts) { | ||
| let actionName; | ||
|
|
||
| // Send the default action | ||
| if (action === undefined) { | ||
| action = 'action'; | ||
| } | ||
| actionName = get(this, `${ARGS}.${action}`) || get(this, action); | ||
| actionName = validateAction(this, actionName); | ||
|
|
||
| // If no action name for that action could be found, just abort. | ||
| if (actionName === undefined) { return; } | ||
|
|
||
| if (typeof actionName === 'function') { | ||
| actionName(...contexts); | ||
| } else { | ||
| this.triggerAction({ | ||
| action: actionName, | ||
| actionContext: contexts | ||
| }); | ||
| } | ||
| } | ||
| }); | ||
|
|
||
| function getCurrentOutletState(outletState) { | ||
| let { outlets, render } = outletState; | ||
| return (Object.keys(outlets).length > 0) ? getCurrentOutletState(outlets[render.outlet]) : outletState; | ||
| } | ||
|
|
||
| function validateAction(component, actionName) { | ||
| //TODO: How to check if this is a reference? | ||
| if (actionName && typeof actionName.value === 'function') { | ||
| return actionName.value(); | ||
| } | ||
|
|
||
| assert( | ||
| 'The default action was triggered on the component ' + component.toString() + | ||
| ', but the action name (' + actionName + ') was not a string.', | ||
| isNone(actionName) || typeof actionName === 'string' || typeof actionName === 'function' | ||
| ); | ||
| return actionName; | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This duplication seems odd. What is actually changed from the mixin in ember-views?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is indeed, the mixin in ember views actually only provided the TargetActionSupport mixin and a couple of CPs. Most of the actual methods for targetable actions in views was implemented in ember-htmlbars/lib/component.js. It made sense to me to actually unify these implementations into its own mixin and not pollute the component.js file for glimmer.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree with the intent, but I would rather see the implementation moved into the "real"
target-action-support.js. Every file we add topackages/ember-glimmer/lib/<some other package name>/lib/must be moved before we ship, and assuming the implementation here is the same asember-htmlbars/componentthere is no reason to make our lives harder...Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the only key difference is in the way the action is resolved either from
attrs.or theARGSsymbol on the component.That said, I get what you're saying, but I'm not sure how do make the change without modifying the API of the old
view_target_action_support.jsmixin. If I move thesend()andsendAction()intotarget-action-support.jsthenview_target_action_supportwill also have those methods, even though in glimmer1 they're only added at thecomponent.jslevel, whereview_target_action_supportis mixed-in.This means that anyone manually mixing-in
view_target_action_support.jswill get more than they bargained for. If this is fine though, I'll go ahead and do that