launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #03800
[Merge] lp:~wallyworld/lazr-js/picker-entry-extra-title-links into lp:lazr-js
Ian Booth has proposed merging lp:~wallyworld/lazr-js/picker-entry-extra-title-links into lp:lazr-js.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #791053 in LAZR Javascript Library: "Add extra title rendering options for picker"
https://bugs.launchpad.net/lazr-js/+bug/791053
For more details, see:
https://code.launchpad.net/~wallyworld/lazr-js/picker-entry-extra-title-links/+merge/63069
For the disclosure feature work, enhancements are required to the picker - extra information needs to be rendered so that the user can be confident that they are choosing the correct entry.
This branch provides the infrastructure to allow picker implementations to:
1. Render an alternate title for each picker entry - rendered in parentheses after the main title
2. Optionally provide hrefs for title/alt title which cause these to be rendered as anchors and the links are opened in new windows.
== Implementation ==
Modify the picker rendering javascript. The title and description rendering is broken out to separate functions. The description is not rendered any differently but may be for subsequent work.
There's one lint error - I'm not sure exactly what I need to change to fix it.
== Demo ==
See screenshot : http://people.canonical.com/~ianb/person-picker-links.png
This example shows just the alternate titles having links.
== Tests ==
Add new tests to picker/tests/picker.js
== Lint ==
jslint: 2 files to lint.
jslint: No problem found in '/home/ian/projects/lazrjs-branches/devel/src-js/lazrjs/picker/picker.js'.
jslint: Lint found in '/home/ian/projects/lazrjs-branches/devel/src-js/lazrjs/picker/tests/picker.js':
Line 158 character 52: Script URL.
Assert.areEqual(link_node.get('href'), 'javascript:void(0)');
--
https://code.launchpad.net/~wallyworld/lazr-js/picker-entry-extra-title-links/+merge/63069
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wallyworld/lazr-js/picker-entry-extra-title-links into lp:lazr-js.
=== modified file 'src-js/lazrjs/picker/picker.js'
--- src-js/lazrjs/picker/picker.js 2011-01-14 15:50:06 +0000
+++ src-js/lazrjs/picker/picker.js 2011-06-01 05:58:23 +0000
@@ -317,6 +317,69 @@
},
/**
+ * Return a node containing the specified text. If a href is provided,
+ * then the text will be linkified with with the given css class. The
+ * link will open in a new window (but the browser can be configured to
+ * open a new tab instead if the user so wishes).
+ * @param text the text to render
+ * @param href the URL of the new window
+ * @param css the style to use when rendering the link
+ */
+ _text_or_link: function(text, href, css) {
+ var result;
+ if (href !== null) {
+ result=Y.Node.create(
+ "<a class='"+css+"' href=javascript:void(0)>"+text+"</a>");
+ Y.on("click", function(e) {
+ e.halt();
+ window.open(href);
+ }, result);
+ } else {
+ result = document.createTextNode(text);
+ }
+ return result;
+ },
+
+ /**
+ * Render a node containing the title part of the picker entry.
+ * The title will consist of some main text with some optional alternate
+ * text which will be rendered in parentheses after the main text. The
+ * title/alt_title text may separately be turned into a link with user
+ * specified URLs.
+ * @param data a json data object with the details to render
+ */
+ _renderTitleUI: function(data) {
+ var li_title = Y.Node.create(
+ '<span></span>').addClass(C_RESULT_TITLE);
+ var title = this._text_or_link(
+ data.title, data.title_link, data.link_css);
+ li_title.appendChild(title);
+ if (data.alt_title !== null) {
+ var alt_title = this._text_or_link(
+ data.alt_title, data.alt_title_link, data.link_css);
+ li_title.appendChild(" (");
+ li_title.appendChild(alt_title);
+ li_title.appendChild(")");
+ }
+ return li_title;
+ },
+
+ /**
+ * Render a node containing the description part of the picker entry.
+ * @param data a json data object with the details to render
+ */
+ _renderDescriptionUI: function(data) {
+ var li_desc = Y.Node.create(
+ '<div><br /></div>').addClass(C_RESULT_DESCRIPTION);
+ if (data.description) {
+ li_desc.replaceChild(
+ document.createTextNode(data.description),
+ li_desc.one('br'));
+ }
+ return li_desc;
+ },
+
+ /**
* Update the UI based on the results attribute.
*
* @method _syncResultsUI
@@ -331,18 +394,9 @@
Y.Array.each(results, function(data, i) {
// Sort out the title span.
- var li_title = Y.Node.create(
- '<span></span>').addClass(C_RESULT_TITLE);
- li_title.appendChild(
- document.createTextNode(data.title));
+ var li_title = this._renderTitleUI(data);
// Sort out the description div.
- var li_desc = Y.Node.create(
- '<div><br /></div>').addClass(C_RESULT_DESCRIPTION);
- if (data.description) {
- li_desc.replaceChild(
- document.createTextNode(data.description),
- li_desc.one('br'));
- }
+ var li_desc = this._renderDescriptionUI(data);
// Put the list item together.
var li = Y.Node.create('<li></li>').addClass(
i % 2 ? Y.lazr.ui.CSS_ODD : Y.lazr.ui.CSS_EVEN);
=== modified file 'src-js/lazrjs/picker/tests/picker.js'
--- src-js/lazrjs/picker/tests/picker.js 2011-01-14 17:13:05 +0000
+++ src-js/lazrjs/picker/tests/picker.js 2011-06-01 05:58:23 +0000
@@ -109,6 +109,66 @@
'Unexpected description value.');
},
+ test_alternate_title_text: function () {
+ this.picker.render();
+ var image_url = '../../lazr/assets/skins/sam/search.png';
+ this.picker.set('results', [
+ {
+ image: image_url,
+ css: 'yui3-blah-blue',
+ value: 'jschmo',
+ title: 'Joe Schmo',
+ description: 'joe@xxxxxxxxxxx',
+ alt_title: 'Another Joe'
+ }
+ ]);
+ var bb = this.picker.get('boundingBox');
+ var li = bb.one('.yui3-picker-results li');
+ var title_el = li.one('.yui3-picker-result-title');
+ Assert.isNotNull(title_el, "Missing title element");
+ Assert.areEqual(
+ 'Joe Schmo\u00a0(Another Joe)', title_el.get('text'),
+ 'Unexpected title value.');
+ },
+
+ test_title_links: function () {
+ this.picker.render();
+ var image_url = '../../lazr/assets/skins/sam/search.png';
+ this.picker.set('results', [
+ {
+ image: image_url,
+ css: 'yui3-blah-blue',
+ value: 'jschmo',
+ title: 'Joe Schmo',
+ description: 'joe@xxxxxxxxxxx',
+ alt_title: 'Another Joe',
+ title_link: 'http://somewhere.com',
+ alt_title_link: 'http://somewhereelse.com',
+ link_css: 'cool-style'
+ }
+ ]);
+
+ function check_link(picker, link_selector, title) {
+ var bb = picker.get('boundingBox');
+ var link_clicked = false;
+ var link_node = bb.one(link_selector);
+
+ Assert.areEqual(link_node.get('text'), title);
+ Assert.areEqual(link_node.get('href'), 'javascript:void(0)');
+
+ Y.on('click', function(e) {
+ link_clicked = true;
+ }, link_node);
+ simulate(bb, link_selector, 'click');
+ Assert.isTrue(link_clicked,
+ link_selector + ' link was not clicked');
+ }
+ check_link(
+ this.picker, '.cool-style:nth-child(1)', 'Joe Schmo');
+ check_link(
+ this.picker, '.cool-style:nth-child(2)', 'Another Joe');
+ },
+
test_results_display_escaped: function () {
this.picker.render();
this.picker.set('results', [
Follow ups
