Merge lp:~wallyworld/lazr-js/activator-flash-node into lp:lazr-js

Proposed by Ian Booth
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
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_animation_node() test was added.

== Lint ==

jslint: 2 files to lint.
jslint: No problem found in '/home/ian/projects/lazrjs-branches/devel/src-js/lazrjs/activator/activator.js'.

jslint: No problem found in '/home/ian/projects/lazrjs-branches/devel/src-js/lazrjs/activator/tests/activator.js'.

To post a comment you must log in.
207. By Ian Booth

Lint

Revision history for this message
Deryck Hodge (deryck) wrote :

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

review: Approve (code)
208. By Ian Booth

Formatting fix

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'examples/activator/index.html'
2--- examples/activator/index.html 2011-01-14 17:34:54 +0000
3+++ examples/activator/index.html 2011-03-16 00:09:14 +0000
4@@ -35,6 +35,13 @@
5 var activator = new Y.lazr.activator.Activator(
6 {contentBox: content_box});
7
8+ // Optionally, it's possible to specify a custom node to animate on
9+ // success, failure, or cancellation. The default is to use the
10+ // content_box but depending on the HTML inside the content box, we may
11+ // need to specify a different node to get the required animation.
12+ //var activator = new Y.lazr.activator.Activator(
13+ // {contentBox: content_box, animationNode: custom_node});
14+
15 activator.subscribe('act', function (e) {
16 editor.setStyle('visibility', 'visible');
17 });
18@@ -221,6 +228,15 @@
19
20 </ul>
21
22+<p>Optionally, these DOM elements can be specified:</p>
23+
24+<ul>
25+ <li><strong>Animation node:</strong>
26+ A node that is animated on success, failure, or cancellation. The default
27+ is to use the content box node.
28+ </li>
29+</ul>
30+
31 <h3>Custom events</h3>
32
33 <table>
34
35=== modified file 'src-js/lazrjs/activator/activator.js'
36--- src-js/lazrjs/activator/activator.js 2010-04-13 13:02:44 +0000
37+++ src-js/lazrjs/activator/activator.js 2011-03-16 00:09:14 +0000
38@@ -156,7 +156,7 @@
39 }
40 this._setStatusClass(C_SUCCESS);
41 this._renderMessage('Message', message_node);
42- var anim = Y.lazr.anim.green_flash({node: this.get('contentBox')});
43+ var anim = Y.lazr.anim.green_flash({node: this.animation_node});
44 anim.run();
45 },
46
47@@ -170,7 +170,7 @@
48 renderFailure: function(message_node) {
49 this._renderMessage('Error', message_node);
50 this._setStatusClass(C_FAILURE);
51- var anim = Y.lazr.anim.red_flash({node: this.get('contentBox')});
52+ var anim = Y.lazr.anim.red_flash({node: this.animation_node});
53 anim.run();
54 },
55
56@@ -184,7 +184,7 @@
57 renderCancellation: function(message_node) {
58 this._renderMessage('Message', message_node);
59 this._setStatusClass(C_CANCEL);
60- var anim = Y.lazr.anim.red_flash({node: this.get('contentBox')});
61+ var anim = Y.lazr.anim.red_flash({node: this.animation_node});
62 anim.run();
63 },
64
65@@ -231,6 +231,10 @@
66 throw new Error("Can't find element with CSS class " +
67 C_ACT + ".");
68 }
69+ this.animation_node = cfg.animationNode;
70+ if (this.animation_node === undefined) {
71+ this.animation_node = this.get('contentBox');
72+ }
73 },
74
75 /**
76
77=== modified file 'src-js/lazrjs/activator/tests/activator.js'
78--- src-js/lazrjs/activator/tests/activator.js 2011-01-14 17:13:05 +0000
79+++ src-js/lazrjs/activator/tests/activator.js 2011-03-16 00:09:14 +0000
80@@ -46,6 +46,7 @@
81 }
82 this.workspace.appendChild(Y.Node.create(
83 '<div id="example-1">' +
84+ '<div id="custom-animation-node"/>' +
85 '<span class="yui3-activator-data-box">' +
86 ' Original Value' +
87 '</span>' +
88@@ -67,6 +68,18 @@
89 this.workspace.set('innerHTML', '');
90 },
91
92+ test_correct_animation_node: function() {
93+ // Check that the correct animation node is used.
94+ // First check the default.
95+ Assert.areEqual(this.activator.get('contentBox'),
96+ this.activator.animation_node);
97+ // Now check a custom one.
98+ var custom_node = Y.one('#custom-animation-node');
99+ this.activator = new Y.lazr.activator.Activator(
100+ {contentBox: Y.one('#example-1'), animationNode: custom_node});
101+s Assert.areEqual(custom_node, this.activator.animation_node);
102+ },
103+
104 test_unhiding_action_button: function() {
105 this.action_button.addClass('yui3-activator-hidden');
106 Assert.isTrue(this.action_button.hasClass('yui3-activator-hidden'));

Subscribers

People subscribed via source and target branches