Merge lp:~wallyworld/lazr-js/activator-flash-node into lp:lazr-js
Status: | Merged | ||||
---|---|---|---|---|---|
Approved by: | Ian Booth | ||||
Approved revision: | 207 | ||||
Merged at revision: | 206 | ||||
Proposed branch: | lp:~wallyworld/lazr-js/activator-flash-node | ||||
Merge into: | lp:lazr-js | ||||
Diff against target: |
106 lines (+36/-3) 3 files modified
examples/activator/index.html (+16/-0) src-js/lazrjs/activator/activator.js (+7/-3) src-js/lazrjs/activator/tests/activator.js (+13/-0) |
||||
To merge this branch: | bzr merge lp:~wallyworld/lazr-js/activator-flash-node | ||||
Related bugs: |
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Deryck Hodge (community) | code | Approve | |
Review via email: mp+53349@code.launchpad.net |
Commit message
[r=deryck][bug=735832] Add support to the Activator for a custom animation node to be specified for success, failure or cancellation.
Description of the change
The branch adds support to the Activator for a custom animation node to be specified for success, failure or cancellation.
The default behaviour is to animate the content_box node but this is not always sufficient to get the correct items being flashed. Depending on the HTML inside the content box, it may be necessary to specify a custom node to animate to get the desired effect.
A new Activator config parameter - animationNode - is supported. If undefined, the content box is used.
== Tests ==
A new test_correct_
== Lint ==
jslint: 2 files to lint.
jslint: No problem found in '/home/
jslint: No problem found in '/home/
This looks good, thanks!
I'll just not for future reference that the activator widget breaks from good YUI widget design, by declaring all attributes local on "this". The ATTRS object is unused -- i.e. this.some_node rather than this.get( 'some_node' ). It's fine how you've done your code, following the pattern already there, but good widget design would store this node in ATTRS and take advantage of YUI's widget framework. Most of the other widgets do this correctly in lazr-js.
As I say, the way you've done it here is fine. Just wanted you to know, if you didn't already. I wouldn't ask you to refactor the widget to follow the others, just for this simple change.
Cheers,
deryck