Action handler subsystem #1261
Action handler subsystem #1261
Conversation
| @@ -121,6 +126,39 @@ export class ActionService { | |||
| } | |||
|
|
|||
| /** | |||
| * Installs action handler for the specified element. | |||
| * @param {!Element} target | |||
| * @param {function(!ActionInvocation)} handler | |||
There was a problem hiding this comment.
do we need a bang on function? (cant remember if function was implicitly not nullable, i know Function definitely is nullable by default)
There was a problem hiding this comment.
No, lower-case function is non-nullable.
|
@dvoytenko LGTM |
| */ | ||
| installActionHandler(target, handler) { | ||
| const debugid = target.tagName + '#' + target.id; | ||
| assert(this.isAmpElement_(target), 'AMP element is expected: %s', debugid); |
There was a problem hiding this comment.
Should we pass the target so the error has associatedElement?
There was a problem hiding this comment.
Not in this case. This is a non-authoring visible error. Unfortunately, assert and associate elements are a bit abused. I believe we are tracking it somewhere in one of the issues. Right, @erwinmombay ?
There was a problem hiding this comment.
@dvoytenko correct, tracked in #1031 will continue work on it soon.
|
deferring to @jridgewell for final LGTM as he has some concerns. |
|
|
No description provided.