Merge lp:~wallyworld/lazr-js/picker-entry-extra-title-links into lp:lazr-js
Status: | Merged | ||||
---|---|---|---|---|---|
Approved by: | Ian Booth | ||||
Approved revision: | 218 | ||||
Merged at revision: | 211 | ||||
Proposed branch: | lp:~wallyworld/lazr-js/picker-entry-extra-title-links | ||||
Merge into: | lp:lazr-js | ||||
Diff against target: |
283 lines (+189/-15) 3 files modified
src-js/lazrjs/picker/picker.js (+93/-11) src-js/lazrjs/picker/tests/picker.js (+84/-0) src-py/lazr/js/build.py (+12/-4) |
||||
To merge this branch: | bzr merge lp:~wallyworld/lazr-js/picker-entry-extra-title-links | ||||
Related bugs: |
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Curtis Hovey (community) | code | Approve | |
Review via email: mp+63069@code.launchpad.net |
Commit message
[r=sinzui][bug=791053] Enhance the picker to display an alternate title entry and optionally linkify the title/alt titles.
Description of the change
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.
3. Optionally render badges (small images) to the right of the picker entry title.
Building the tarball to test this branch revealed a bug introduced to build.py. A change was made to allow the combo build to be invoked from the source directory rather than the egg. This broke Launchpad's jsbuild so a fix was done to cater for both scenarios.
== 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://
This example shows just the alternate titles having links.
== Tests ==
Add new tests to picker/
== Lint ==
jslint: 2 files to lint.
jslint: No problem found in '/home/
jslint: Lint found in '/home/
Line 158 character 52: Script URL.
Hi Ian.
Thank you for providing this branch. I see a few trivial issues I would like you to fix. I also have some reservation about the action link, but I think user testing is more important than my own musings.
> === modified file 'src-js/ lazrjs/ picker/ picker. js' lazrjs/ picker/ picker. js 2011-01-14 15:50:06 +0000 lazrjs/ picker/ picker. js 2011-06-03 02:06:33 +0000 Y.Node. create(
> --- src-js/
> +++ src-js/
> @@ -317,6 +317,93 @@
> },
>
> /**
> + * 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) {
> + result=
whitespace around operators.
+ "<a class='"+css+"' href=javascript :void(0) ></a>") ;
whitespace around operators.
> + result.set('text', text); createTextNode( text);
> + Y.on('click', function(e) {
> + e.halt();
> + window.open(href);
> + }, result);
> + } else {
> + result = document.
> + }
> + 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
> + */
I worry that this presupposes every user of lazrjs using display names and
Ids like Lp does. I think we may need to revisit this. I would not be
surprised if other lazr stakeholders would want this to follow the Web UI
guidelines; an action link:
action >
or an action button
[action button]
These are also the same guidelines that define the green we use of actions
that do not leave the current page/context.
> + _renderTitleUI: function(data) { </span> ').addClass( C_RESULT_ TITLE); appendChild( title); title_link, data.link_css); appendChild( ' ( ')
> + var li_title = Y.Node.create(
> + '<span>
> + var title = this._text_or_link(
> + data.title, data.title_link, data.link_css);
> + li_title.
> + if (data.alt_title) {
> + var alt_title = this._text_or_link(
> + data.alt_title, data.alt_
> + li_title.
I think this is an invalid named entity. Entities are terminated with a
semi-colon.
> + .appendChild( alt_title)
> + .appendChild(')');
> + }
> + return li_title;
> + },
...
> === modified file 'src-js/ lazrjs/ picker/ tests/picker. js' lazrjs/ picker/ tests/picker. js 2011-01-14 17:13:05 +0000 lazrjs/ picker/ tests/picker. js 2011-06-03 ...
> --- src-js/
> +++ src-js/