Merge lp:~mwhudson/loggerhead/js-cleanup into lp:loggerhead

Proposed by Michael Hudson-Doyle
Status: Merged
Merged at revision: not available
Proposed branch: lp:~mwhudson/loggerhead/js-cleanup
Merge into: lp:loggerhead
Diff against target: None lines
To merge this branch: bzr merge lp:~mwhudson/loggerhead/js-cleanup
Reviewer Review Type Date Requested Status
Paul Hummer (community) Approve
Review via email: mp+5087@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Michael Hudson-Doyle (mwhudson) wrote :

Hi,

This branch cleans up the Collapsible animation js thing. It gets rid of the hack of storing alternate image urls in the title and alt attributes, creates some of the structure it needs rather than demanding it be included in the template, and starts the animation before the ajax load begins, which means that if the server answers fast enough it's not obvious that ajax is happening at all.

Cheers,
mwh

Revision history for this message
Paul Hummer (rockstar) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'loggerhead/static/javascript/changelog.js'
2--- loggerhead/static/javascript/changelog.js 2009-03-16 02:58:10 +0000
3+++ loggerhead/static/javascript/changelog.js 2009-03-31 21:11:38 +0000
4@@ -60,7 +60,6 @@
5 open_node: item.query('.long_description'),
6 close_node: item.query('.short_description'),
7 source: global_path + '+revlog/' + revid,
8- source_target: item.query('.source_target'),
9 loading: item.query('.loading'),
10 is_open: false
11 });
12
13=== modified file 'loggerhead/static/javascript/custom.js'
14--- loggerhead/static/javascript/custom.js 2009-03-17 04:26:28 +0000
15+++ loggerhead/static/javascript/custom.js 2009-03-31 21:20:06 +0000
16@@ -80,13 +80,15 @@
17 function Collapsable(config)
18 {
19 this.is_open = config.is_open;
20- this.source_target = config.source_target;
21 this.open_node = config.open_node;
22 this.close_node = config.close_node;
23 this.expand_icon = config.expand_icon;
24 this.source = config.source;
25 this.loading = config.loading;
26 this.node_process = config.node_process;
27+ this.container = null;
28+ this.anim = null;
29+ this._loading = false;
30 }
31
32 function get_height(node) {
33@@ -100,22 +102,110 @@
34 return height;
35 }
36
37+Collapsable.prototype._animate = function (callback)
38+{
39+ if (this.anim) this.anim.stop();
40+
41+ this.anim = new Y.Anim(
42+ {
43+ node: this.container,
44+ from: {
45+ marginBottom: this.container.getStyle('marginBottom')
46+ },
47+ to: {
48+ marginBottom: 0
49+ },
50+ duration: 0.2
51+ });
52+
53+ this.anim.run();
54+ this.anim.on('end', this.animComplete, this, callback);
55+}
56+
57 Collapsable.prototype._load_finished = function(tid, res, args)
58 {
59 var newNode = Y.Node.create(res.responseText.split('\n').splice(1).join(''));
60 if (this.node_process)
61 this.node_process(newNode);
62- this.source_target.ancestor().replaceChild(newNode, this.source_target);
63- this.source_target = null;
64 this.source = null;
65- this.loading.setStyle('display', 'none');
66- this.open(args[0]);
67+ newNode.setStyle('display', 'none');
68+ this.loading.ancestor().insertBefore(newNode, this.loading);
69+ var delta = this.loading.get('region').height - get_height(newNode);
70+ newNode.setStyle('display', 'block');
71+ this.container.setStyle('marginBottom', parseFloat(this.container.getStyle('marginBottom')) + delta);
72+ this.loading.ancestor().removeChild(this.loading);
73+ this._animate(args[0]);
74 };
75
76+Collapsable.prototype._ensure_container = function(callback)
77+{
78+ if (this.container == null) {
79+ this.container = Y.Node.create('<div style="overflow:hidden;"></div>');
80+ if (this.closed_node) {
81+ this.closed_node.ancestor().replaceChild(
82+ this.container, this.closed_node);
83+ this.container.appendChild(this.closed_node);
84+ if (this.open_node) {
85+ this.container.appendChild(this.open_node);
86+ }
87+ }
88+ else {
89+ this.open_node.ancestor().replaceChild(
90+ this.container, this.open_node);
91+ this.container.appendChild(this.open_node);
92+ }
93+ }
94+}
95+
96+/* What happens when you click open.
97+ *
98+ * 1. The arrow flips to the expanded position.
99+ *
100+ * 2. If necessary, the div which will be running the animation is
101+ * created and the open/closed content stuffed into it (and has height
102+ * set to the height of the closed content).
103+ *
104+ * 3. The open content is shown and the closed content is closed.
105+ *
106+ * 4. The animation to expose all of the open content is started.
107+ *
108+ * 5. If we have to do ajax to load content, start the request.
109+ *
110+ * 6. When the request completes, parse the content into a node, run
111+ * the node_process callback over it and replace the spinner (assumed
112+ * to be appropriately contained in the open node) with the new node.
113+ *
114+ * 7. If the animation showing the open content has not completed,
115+ * stop it.
116+ *
117+ * 8. Start a new animation to show the rest of the new content.
118+ */
119+
120 Collapsable.prototype.open = function(callback)
121 {
122+ this.expand_icon.set('src', expanded_icon_path);
123+
124+ this._ensure_container();
125+
126+ var open_height = get_height(this.open_node);
127+
128+ var close_height;
129+ if (this.close_node) {
130+ close_height = this.close_node.get('region').height;
131+ }
132+ else {
133+ close_height = 0;
134+ }
135+
136+ this.container.setStyle('marginBottom', close_height - open_height);
137+ if (this.close_node) {
138+ this.close_node.setStyle('display', 'none');
139+ }
140+ this.open_node.setStyle('display', 'block');
141+
142+ this._animate(callback);
143+
144 if (this.source) {
145- this.loading.setStyle('display', 'block');
146 Y.io.queue(
147 this.source,
148 {
149@@ -126,49 +216,19 @@
150 return;
151 }
152
153- var open_height = get_height(this.open_node);
154-
155- var close_height;
156- if (this.close_node) {
157- close_height = this.close_node.get('region').height;
158- }
159- else {
160- close_height = 0;
161- }
162-
163- var container = this.open_node.ancestor('.container');
164-
165- var anim = new Y.Anim(
166- {
167- node: container,
168- from: {
169- marginBottom: close_height - open_height
170- },
171- to: {
172- marginBottom: 0
173- },
174- duration: 0.2
175- });
176-
177- anim.on('end', this.openComplete, this, callback);
178- container.setStyle('marginBottom', close_height - open_height);
179- if (this.close_node) {
180- this.close_node.setStyle('display', 'none');
181- }
182- this.open_node.setStyle('display', 'block');
183- this.expand_icon.set('src', this.expand_icon.get('alt'));
184- anim.run();
185 };
186
187-Collapsable.prototype.openComplete = function(evt, callback)
188+Collapsable.prototype.animComplete = function(evt, callback)
189 {
190+ this.anim = null;
191+ if (this._loading) return;
192 if (callback) callback();
193 this.is_open = true;
194 };
195
196 Collapsable.prototype.close = function()
197 {
198- var container = this.open_node.ancestor('.container');
199+ this._ensure_container();
200
201 var open_height = this.open_node.get('region').height;
202
203@@ -182,7 +242,7 @@
204
205 var anim = new Y.Anim(
206 {
207- node: container,
208+ node: this.container,
209 from: {
210 marginBottom: 0
211 },
212@@ -200,8 +260,8 @@
213 if (this.close_node) {
214 this.close_node.setStyle('display', 'block');
215 }
216- this.open_node.ancestor('.container').setStyle('marginBottom', 0);
217- this.expand_icon.set('src', this.expand_icon.get('title'));
218+ this.container.setStyle('marginBottom', 0);
219+ this.expand_icon.set('src', collapsed_icon_path);
220 this.is_open = false;
221 };
222
223
224=== modified file 'loggerhead/static/javascript/diff.js'
225--- loggerhead/static/javascript/diff.js 2009-03-18 04:08:41 +0000
226+++ loggerhead/static/javascript/diff.js 2009-03-31 21:11:38 +0000
227@@ -192,7 +192,6 @@
228 open_node: item.query('.diffinfo'),
229 close_node: null,
230 source: source_url,
231- source_target: item.query('.source_target'),
232 is_open: specific_path != null,
233 loading: item.query('.loading'),
234 node_process: node_process
235
236=== modified file 'loggerhead/templates/changelog.pt'
237--- loggerhead/templates/changelog.pt 2009-03-16 23:21:29 +0000
238+++ loggerhead/templates/changelog.pt 2009-03-31 21:15:48 +0000
239@@ -86,37 +86,26 @@
240 </td>
241 <td class="expcell show_if_js">
242 <div class="expand_revisioninfo">
243- <!-- So, this is interesting. I'm using "alt" to have the correct URL for the image of the expanded icon
244- and "title" tag for the contracted URL of the image. This is a bit of a hack, but it's better than
245- other approaches I tried :) -->
246 <a href="#">
247- <img tal:attributes="src python:branch.static_url('/static/images/treeCollapsed.png');
248- alt python:branch.static_url('/static/images/treeExpanded.png');
249- title python:branch.static_url('/static/images/treeCollapsed.png')"
250+ <img tal:attributes="src python:branch.static_url('/static/images/treeCollapsed.png')"
251 class="expand_icon" />
252 </a>
253 </div>
254 </td>
255 <td class="summcell">
256- <div style="overflow: hidden">
257- <div class="container">
258- <div class="short_description">
259- <a tal:attributes="title python:'Show revision '+entry.revno;
260- href python:url(['/revision', entry.revno], clear=1);
261- class string:link"
262- tal:content="entry/short_comment"></a>
263- <div class="loading" style="display:none;">
264- <img tal:attributes="src python:branch.static_url('/static/images/spinner.gif')" />
265- </div>
266- </div>
267- <div class="long_description" style="display: none">
268- <a tal:attributes="title python:'Show revision '+entry.revno;
269- href python:url(['/revision', entry.revno], clear=1);
270- class string:link"
271- tal:content="structure python:util.fixed_width(entry.comment)"></a>
272- <div class="source_target">
273- </div>
274- </div>
275+ <div class="short_description">
276+ <a tal:attributes="title python:'Show revision '+entry.revno;
277+ href python:url(['/revision', entry.revno], clear=1);
278+ class string:link"
279+ tal:content="entry/short_comment"></a>
280+ </div>
281+ <div class="long_description" style="display: none">
282+ <a tal:attributes="title python:'Show revision '+entry.revno;
283+ href python:url(['/revision', entry.revno], clear=1);
284+ class string:link"
285+ tal:content="structure python:util.fixed_width(entry.comment)"></a>
286+ <div class="loading">
287+ <img tal:attributes="src python:branch.static_url('/static/images/spinner.gif')" />
288 </div>
289 </div>
290 </td>
291
292=== modified file 'loggerhead/templates/macros.pt'
293--- loggerhead/templates/macros.pt 2009-03-16 03:22:02 +0000
294+++ loggerhead/templates/macros.pt 2009-03-31 19:28:09 +0000
295@@ -13,6 +13,8 @@
296 </tal:comment>
297 <script type="text/javascript">
298 var global_path = <tal:block content="python:'\''+branch.url('/')+'\''" />;
299+ var collapsed_icon_path = <tal:block content="python:'\''+branch.static_url('/static/images/treeCollapsed.png')+'\''" />;
300+ var expanded_icon_path = <tal:block content="python:'\''+branch.static_url('/static/images/treeExpanded.png')+'\''" />;
301 </script>
302 <script type="text/javascript"
303 tal:attributes="src
304
305=== modified file 'loggerhead/templates/revision.pt'
306--- loggerhead/templates/revision.pt 2009-03-19 00:44:05 +0000
307+++ loggerhead/templates/revision.pt 2009-03-31 21:15:48 +0000
308@@ -124,9 +124,7 @@
309 id string:${specific_path};
310 title string:View changes to ${specific_path} only"
311 class="the-link">
312- <img tal:attributes="src python:branch.static_url('/static/images/treeExpanded.png');
313- title python:branch.static_url('/static/images/treeCollapsed.png');
314- alt python:branch.static_url('/static/images/treeExpanded.png')"
315+ <img tal:attributes="src python:branch.static_url('/static/images/treeExpanded.png')"
316 class="expand_diff"/>
317 <tal:b content="specific_path" />
318 </a>
319@@ -175,22 +173,14 @@
320 id string:${item/filename};
321 title string:View changes to ${item/filename} only"
322 class="the-link">
323- <img tal:attributes="src python:branch.static_url('/static/images/treeCollapsed.png');
324- title python:branch.static_url('/static/images/treeCollapsed.png');
325- alt python:branch.static_url('/static/images/treeExpanded.png')"
326+ <img tal:attributes="src python:branch.static_url('/static/images/treeCollapsed.png')"
327 class="expand_diff" />
328 <tal:b content="item/filename" />
329 </a>
330 </div>
331- <div style="overflow: hidden">
332- <div class="container">
333- <div class="loading" style="display:none">
334- <img tal:attributes="src python:branch.static_url('/static/images/spinner.gif')" />
335- </div>
336- <div class="diffinfo">
337- <div class="source_target" >
338- </div>
339- </div>
340+ <div class="diffinfo" style="display:none">
341+ <div class="loading">
342+ <img tal:attributes="src python:branch.static_url('/static/images/spinner.gif')" />
343 </div>
344 </div>
345 </div>

Subscribers

People subscribed via source and target branches