launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #06241
[Merge] lp:~jcsackett/launchpad/grackle-lp-ui-improvements into lp:launchpad
j.c.sackett has proposed merging lp:~jcsackett/launchpad/grackle-lp-ui-improvements into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~jcsackett/launchpad/grackle-lp-ui-improvements/+merge/91456
Summary
=======
Since integrating grackle as a mail archiver and creating a new mail list
viewing UI is feature level work, we wanted to set up a skeleton for nav
controls and move to using mustache for rendering, to make future enhancements
easier.
Pre Implementation
==================
Curtis Hovey, Rick H
Implementation
==============
team_mailinglists.js
--------------------
Rendering of the messages is now done through mustache. The function that
previously created nodes for each message now creates mustache model data. It
still has a recursive section to deal with threaded messages.
This section was also updated to account for a change in the grackle
api--instead of messages that are part of a thread being directly nested in
the topmost message, a list of the messages ids that are threaded is
returned. The new logic in the js module accounts for this, tracking messages
that have been processed as threaded messages are dealt with, so they aren't
rendered a second time as the total list of messages is processed. This way
messages are rendered once, as part of their thread.
We also added very basic navigation pieces to the YUI object, simply binding
supplied elements to fire navigation events.
team-mailinglist-archive.pt
---------------------------
The html for the navigation was added, and the setup for the js module was
updated.
test_team_mailinglists.*
------------------------
Tests for navigation were added, and setup for tests was updated.
Tests
=====
bin/test -vvct team_mailinglists --layer=YUI
QA
==
Again, as we have not integrated anything into the actual code yet, this is
essentially no-qa. One can visit ~$team-name/+mailing-list-archive and confirm
that the page loads with the "no messages" text.
Lint
====
= Launchpad lint =
Checking for conflicts and issues in changed files.
Linting changed files:
lib/lp/registry/javascript/team_mailinglists.js
lib/lp/registry/javascript/tests/test_team_mailinglists.html
lib/lp/registry/javascript/tests/test_team_mailinglists.js
lib/lp/registry/templates/team-mailinglist-archive.pt
--
https://code.launchpad.net/~jcsackett/launchpad/grackle-lp-ui-improvements/+merge/91456
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jcsackett/launchpad/grackle-lp-ui-improvements into lp:launchpad.
=== modified file 'lib/lp/registry/javascript/team_mailinglists.js'
--- lib/lp/registry/javascript/team_mailinglists.js 2012-01-24 23:04:43 +0000
+++ lib/lp/registry/javascript/team_mailinglists.js 2012-02-03 16:38:21 +0000
@@ -16,12 +16,24 @@
MessageList.NAME = "messageList";
MessageList.ATTR = {
+ forwards_nagivation: {
+ value: null
+ },
+
+ backwards_navigation: {
+ value: null
+ },
+
messages: {
value: []
},
container: {
value: null
+ },
+
+ template: {
+ value: null
}
};
@@ -32,51 +44,105 @@
if (config.messages !== undefined) {
this.set('messages', config.messages);
}
+ if (config.forwards_navigation !== undefined) {
+ this.set('forwards_navigation', config.forwards_navigation);
+ }
+
+ if (config.backwards_navigation !== undefined) {
+ this.set('backwards_navigation', config.backwards_navigation);
+ }
+ this._bind_nav();
+
+ var template = '<div class="message-list">' +
+ '{{#items}}' +
+ '<li style="margin-left: {{indent}}px">' +
+ '<a href="#" id="message-{{message_id}}">' +
+ '{{subject}}</a>' +
+ '<div>{{from}}, {{date}}</div>' +
+ '</li>' +
+ '{{/items}}' +
+ '</div>';
+ this.set('template', template);
+ },
+
+ _bind_nav: function () {
+ /* XXX j.c.sackett 1-2-2012
+ * These signals aren't currently caught by anything in the message
+ * list. They exist so that once we have batching calls from grackle
+ * ironed out we can easily add the functions in wherever they make
+ * the most sense, be that here or in a grackle js module.
+ *
+ * When we are actually integrating grackle, these may need updating,
+ * and we'll need tests ensuring the signals actually *do* something.
+ */
+ var forwards = this.get('forwards_navigation');
+ var backwards = this.get('backwards_navigation');
+
+ forwards.on('click', function(e) {
+ Y.fire('messageList:forwards', e);
+ });
+
+ backwards.on('click', function(e) {
+ Y.fire('messageList:backwards', e);
+ });
},
display_messages: function () {
- var messages = this.get('messages');
- var container = Y.one('#messagelist');
+ var mustache_model = Y.Array([]);
+ var processed_ids = Y.Array([]);
+ var messages = Y.Array(this.get('messages'));
+ var container = this.get('container');
+ this._create_mustache_model(
+ messages, mustache_model, processed_ids, 0);
+ var content = Y.lp.mustache.to_html(
+ this.get('template'), {items: mustache_model});
+ container.setContent(content);
+ },
+
+ _create_mustache_model: function
+ (messages, mustache_model, processed_ids, indent) {
+ // Right now messages are only being displayed treaded, by date. More
+ // sophisticated model creation will be needed for threaded by
+ // subject.
var i;
+ filter_func = function(item) {
+ var nested_ids = messages[i].nested_messages;
+ var index = nested_ids.indexOf(item.message_id);
+ return (index !== -1);
+ };
for (i = 0; i < messages.length; i++) {
- var message_node = this._create_message_node(messages[i], 0);
- container.appendChild(message_node);
- }
- },
-
- _create_message_node: function(message, indent) {
- var message_node = Y.Node.create('<li></li>');
- var message_id = Y.DataType.Number.format(
- message.message_id, {'prefix': 'message-'});
- var subject_node = Y.Node.create('<a href="#"></a>')
- .set('id', message_id)
- .set('text', message.headers.Subject);
-
- var info = message.headers.From + ', ' + message.headers.Date;
- var info_node = Y.Node.create('<div></div>')
- .set('text', info);
- message_node.appendChild(subject_node);
- message_node.appendChild(info_node);
-
- if (message.nested_messages !== undefined) {
- indent = indent + 10;
- var nested_messages = Y.Node.create('<ul></ul>');
- var indentation = Y.DataType.Number.format(
- indent, {'suffix': 'px'});
- var i;
- nested_messages.setStyle('margin-left', indentation);
-
- for (i = 0; i < message.nested_messages.length; i++) {
- nested_node = _create_message_node(
- message.nested_messages[num], indent);
- nested_messages.appendChild(nested_node);
+ var message = messages[i];
+ // Only create mustache data for messages not already processed.
+ // Messages will have been processed already if they were part of
+ // the nested messages for an earlier message in the list.
+ if (processed_ids.indexOf(message.message_id) === -1) {
+ processed_ids.push(message.message_id);
+ mustache_model.push({
+ message_id: message.message_id,
+ indent: indent,
+ subject: message.headers.Subject,
+ to: message.headers.To,
+ from: message.headers.From,
+ date: message.headers.Date
+ });
+
+ if (message.nested_messages !== undefined) {
+ indent = indent + 10;
+ // Create a new array of the nested messages from the ids
+ // provided by the current message's `nested_messages`
+ // parameter.
+ nested_messages = messages.filter(filter_func);
+ this._create_mustache_model(
+ nested_messages,
+ mustache_model,
+ processed_ids,
+ indent);
+ }
}
- message_node.appendChild(nested_messages);
}
- return message_node;
}
});
module.MessageList = MessageList;
-}, '0.1', {requires: ['base', 'node', 'datatype']});
+}, '0.1', {requires: ['base', 'node', 'datatype', 'lp.mustache']});
=== modified file 'lib/lp/registry/javascript/tests/test_team_mailinglists.html'
--- lib/lp/registry/javascript/tests/test_team_mailinglists.html 2012-01-23 19:25:08 +0000
+++ lib/lp/registry/javascript/tests/test_team_mailinglists.html 2012-02-03 16:38:21 +0000
@@ -12,7 +12,9 @@
</script>
<link rel="stylesheet" href="../../../app/javascript/testing/test.css" />
<script type="text/javascript"
- src="../../../../app/javascript/testing/testrunner.js"></script>
+ src="../../../app/javascript/testing/testrunner.js"></script>
+ <script type="text/javascript"
+ src="../../../contrib/javascript/lp.mustache.js"></script>
<!-- The module under test -->
<script type="text/javascript" src="../team_mailinglists.js"></script>
@@ -23,6 +25,27 @@
<body class="yui-skin-sam">
<!-- The example markup required by the script to run -->
+ <table style="width: 100%;" class="upper-batch-nav">
+ <tbody>
+ <tr>
+ <td style="white-space: nowrap" class="batch-navigation-index">
+ <strong>1</strong>→<strong>2</strong> of 2 results</td>
+
+ <td style="text-align: right; white-space: nowrap"
+ class="batch-navigation-links">
+ <a href="#" class="first js-action inactive">First</a>
+ •
+ <a href="#" class="previous js-action inactive">Previous</a>
+ •
+ <a href="#" class="next js-action inactive">
+ <strong>Next</strong>
+ </a>
+ •
+ <a href="#" class="last js-action inactive">Last</a>
+ </td>
+ </tr>
+ </tbody>
+ </table>
<div id="messagelist">
</div>
</body>
=== modified file 'lib/lp/registry/javascript/tests/test_team_mailinglists.js'
--- lib/lp/registry/javascript/tests/test_team_mailinglists.js 2012-01-24 23:03:27 +0000
+++ lib/lp/registry/javascript/tests/test_team_mailinglists.js 2012-02-03 16:38:21 +0000
@@ -4,8 +4,9 @@
filter: 'raw',
combine: false,
fetchCSS: false
-}).use('event', 'lp.client', 'node', 'test', 'widget-stack',
- 'console', 'lp.registry.team.mailinglists', function(Y) {
+}).use('event', 'lp.mustache', 'node', 'node-event-simulate', 'test',
+ 'widget-stack', 'console', 'lp.registry.team.mailinglists',
+ function(Y) {
// Local aliases
var Assert = Y.Assert,
@@ -42,13 +43,35 @@
'attachments': []
}
],
- container: Y.one('#messagelist')
+ container: Y.one('#messagelist'),
+ forwards_navigation: Y.all('.last,.next'),
+ backwards_navigation: Y.all('.first,.previous')
};
var message_list = new Y.lp.registry.team.mailinglists.MessageList(
config);
message_list.display_messages();
var message = Y.one("#message-3");
Assert.areEqual(message.get('text'), 'Please stop breaking things');
+ },
+
+ test_nav: function () {
+ var config = {
+ messages: [],
+ container: Y.one('#messagelist'),
+ forwards_navigation: Y.all('.last,.next'),
+ backwards_navigation: Y.all('.first,.previous')
+ };
+ var message_list = new Y.lp.registry.team.mailinglists.MessageList(
+ config);
+
+ var fired = false;
+ Y.on('messageList:backwards', function () {
+ fired = true;
+ });
+
+ var nav_link = Y.one('.first');
+ nav_link.simulate('click');
+ Assert.isTrue(fired);
}
}));
=== modified file 'lib/lp/registry/templates/team-mailinglist-archive.pt'
--- lib/lp/registry/templates/team-mailinglist-archive.pt 2012-01-24 22:03:18 +0000
+++ lib/lp/registry/templates/team-mailinglist-archive.pt 2012-02-03 16:38:21 +0000
@@ -10,17 +10,18 @@
<head>
<tal:block metal:fill-slot="head_epilogue">
<tal:script condition="view/messages">
- <!--We'll add this in later, for now it's a place holder because this is where
- we bolt in the js module some day.-->
<script type="text/javascript">
- LPS.use('lp.registry.team.mailinglists', function(Y) {
+ YUI().use('lp.registry.team.mailinglists', function(Y) {
Y.on('domready', function() {
var config = {
messages: LP.cache['mail'],
- container: Y.one('#messagelist')
+ container: Y.one('#messagelist'),
+ forwards_navigation: Y.all('.last,.next'),
+ backwards_navigation: Y.all('.first,.previous')
};
var mailinglist_module = Y.lp.registry.team.mailinglists;
var message_list = new mailinglist_module.MessageList(config);
+ message_list.display_messages();
});
});
</script>
@@ -31,13 +32,35 @@
<body>
<div metal:fill-slot="main">
+ <table style="width: 100%;" class="upper-batch-nav">
+ <tbody>
+ <tr>
+ <td style="white-space: nowrap" class="batch-navigation-index">
+ <strong>1</strong>→<strong>2</strong> of 2 results</td>
+
+ <td style="text-align: right; white-space: nowrap"
+ class="batch-navigation-links">
+ <a href="#" class="first js-action inactive">First</a>
+ •
+ <a href="#" class="previous js-action inactive">Previous</a>
+ •
+ <a href="#" class="next js-action inactive">
+ <strong>Next</strong>
+ </a>
+ •
+ <a href="#" class="last js-action inactive">Last</a>
+ </td>
+ </tr>
+ </tbody>
+ </table>
+
<div id="messagelist">
<tal:comment replace="nothing">
The json loaded and manipulated messages go here.
</tal:comment>
<tal:no-messages condition="not: view/messages">
- There are no messages in <tal:teamname
- replace="view/context/displayname"/>'s mail archive.
+ There are no messages in <tal:teamname
+ replace="view/context/displayname"/>'s mail archive.
</tal:no-messages>
</div>
