Merge lp:~jcsackett/launchpad/oh-oh-pick-me-pick-me into lp:launchpad

Proposed by j.c.sackett
Status: Merged
Approved by: j.c.sackett
Approved revision: no longer in the source branch.
Merged at revision: 13149
Proposed branch: lp:~jcsackett/launchpad/oh-oh-pick-me-pick-me
Merge into: lp:launchpad
Diff against target: 215 lines (+181/-2)
4 files modified
lib/lp/app/widgets/popup.py (+1/-2)
lib/lp/registry/javascript/personpicker.js (+74/-0)
lib/lp/registry/javascript/tests/test_personpicker.html (+40/-0)
lib/lp/registry/javascript/tests/test_personpicker.js (+66/-0)
To merge this branch: bzr merge lp:~jcsackett/launchpad/oh-oh-pick-me-pick-me
Reviewer Review Type Date Requested Status
Steve Kowalik (community) code Approve
Review via email: mp+63047@code.launchpad.net

Commit message

Creates a personpicker YUI widget

Description of the change

Summary
=======
As part of the work for disclosure, a better personpicker is needed that users can trust. The current personpicker is actually just the regular picker, passed a person-specific vocabulary. Because the personpicker will have it's own UI attributes (e.g. "Assign me" and "Remove assignee" buttons like the bug assign pickerPatcher), we need a PersonPicker YUI widget.

Preimplementation
=================
Spoke with Deryck Hodge about creating a new widget, and Curtis Hovey.

Implementation
==============
lib/lp/app/widgets/popup.py
---------------------------
Drive by whitespace fixes.

lib/lp/registry/javascript/personpicker.js
------------------------------------------
Created PersonPicker, which extends Y.lazr.Picker. It creates an assign_me and a remove_asignee button, as those are UI elements we want when using the person picker, and wires them up to provide the users name or to clear the entry as appropriate.

lib/lp/registry/javascript/tests/test_personpicker.html
lib/lp/registry/javascript/tests/test_personpicker.js
-----------------------------------------------------
Tests.

Tests
=====
firefox lib/lp/registry/javascript/tests/test_personpicker.html

QA
==
None. This is a new widget not yet wired into any of the UI.

Lint
====
= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/app/widgets/popup.py
  lib/lp/registry/javascript/personpicker.js
  lib/lp/registry/javascript/tests/test_personpicker.html
  lib/lp/registry/javascript/tests/test_personpicker.js

To post a comment you must log in.
Revision history for this message
Martin Pool (mbp) wrote :

Could you attach a screenshot please?

Revision history for this message
Steve Kowalik (stevenk) wrote :

This looks good to me, but I share Martin's concerns, that:

1. This will require a UI review.
2. I'd like to see how this looks and behaves before I approve it.

review: Needs Information (code)
Revision history for this message
j.c.sackett (jcsackett) wrote :

> This looks good to me, but I share Martin's concerns, that:
>
> 1. This will require a UI review.
> 2. I'd like to see how this looks and behaves before I approve it.

Screenshot is available here: http://people.canonical.com/~jc/images/disclosure/personpicker.png

Revision history for this message
Steve Kowalik (stevenk) :
review: Approve (code)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'lib/lp/app/widgets/popup.py'
--- lib/lp/app/widgets/popup.py 2011-05-29 23:34:26 +0000
+++ lib/lp/app/widgets/popup.py 2011-06-01 14:02:31 +0000
@@ -8,8 +8,6 @@
8__metaclass__ = type8__metaclass__ = type
99
10import cgi10import cgi
11
12import re
13import simplejson11import simplejson
14from z3c.ptcompat import ViewPageTemplateFile12from z3c.ptcompat import ViewPageTemplateFile
15from zope.app.form.browser.itemswidgets import (13from zope.app.form.browser.itemswidgets import (
@@ -156,6 +154,7 @@
156154
157155
158class PersonPickerWidget(VocabularyPickerWidget):156class PersonPickerWidget(VocabularyPickerWidget):
157
159 include_create_team_link = False158 include_create_team_link = False
160159
161 def chooseLink(self):160 def chooseLink(self):
162161
=== added file 'lib/lp/registry/javascript/personpicker.js'
--- lib/lp/registry/javascript/personpicker.js 1970-01-01 00:00:00 +0000
+++ lib/lp/registry/javascript/personpicker.js 2011-06-01 14:02:31 +0000
@@ -0,0 +1,74 @@
1/* Copyright 2011 Canonical Ltd. This software is licensed under the
2 * GNU Affero General Public License version 3 (see the file LICENSE).
3 *
4 * @namespace Y.lp.registry.personpicker
5 * @requires lazr.picker
6 */
7YUI.add('lp.registry.personpicker', function(Y) {
8var namespace = Y.namespace('lp.registry.personpicker');
9
10var footer_label = ".yui3-picker-footer-slot"
11
12var PersonPicker = function() {
13 PersonPicker.superclass.constructor.apply(this, arguments);
14
15 this._extra_buttons = Y.Node.create('<div class="extra-form-buttons"/>');
16};
17
18Y.extend(PersonPicker, Y.lazr.Picker, {
19 hide: function() {
20 this.get('boundingBox').setStyle('display', 'none');
21 },
22
23 show: function() {
24 this.get('boundingBox').setStyle('display', 'block');
25 },
26
27 remove: function () {
28 this.fire('save', {value: ''});
29 },
30
31 assign_me: function () {
32 name = LP.links.me.replace('/~', '');
33 this.fire('save', {value: name});
34 },
35
36 renderUI: function() {
37 this.constructor.superclass.renderUI.call(this);
38 var remove_button, assign_me_button;
39 //# TODO config should set extrabuttons
40 var show_remove_button = true;
41 var show_assign_me_button = true;
42 var remove_button_text = "Remove assignee";
43 var assign_me_button_text = "Assign me";
44 if (show_remove_button) {
45 remove_button = Y.Node.create(
46 '<a class="yui-picker-remove-button bg-image" ' +
47 'href="javascript:void(0)" ' +
48 'style="background-image: url(/@@/remove); padding-right: 1em">' +
49 remove_button_text + '</a>');
50 remove_button.on('click', this.remove, this);
51 this._extra_buttons.appendChild(remove_button);
52 }
53 if (show_assign_me_button) {
54 assign_me_button = Y.Node.create(
55 '<a class="yui-picker-assign-me-button bg-image" ' +
56 'href="javascript:void(0)" ' +
57 'style="background-image: url(/@@/person)">' +
58 assign_me_button_text + '</a>');
59 assign_me_button.on('click', this.assign_me, this);
60 this._extra_buttons.appendChild(assign_me_button);
61 }
62 },
63
64 syncUI: function() {
65 // call Picker's sync
66 this.constructor.superclass.syncUI.call(this);
67 footer_slot = Y.one(footer_label);
68 footer_slot.appendChild(this._extra_buttons);
69 }
70});
71PersonPicker.NAME = 'person-picker';
72namespace.PersonPicker = PersonPicker
73
74}, "0.1", {"requires": ['lazr.picker']});
075
=== added file 'lib/lp/registry/javascript/tests/test_personpicker.html'
--- lib/lp/registry/javascript/tests/test_personpicker.html 1970-01-01 00:00:00 +0000
+++ lib/lp/registry/javascript/tests/test_personpicker.html 2011-06-01 14:02:31 +0000
@@ -0,0 +1,40 @@
1<html>
2 <head>
3 <title>Launchpad PersonPicker</title>
4 <!-- YUI 3.0 Setup -->
5 <script type="text/javascript" src="../../../../canonical/launchpad/icing/yui/yui/yui.js"></script>
6 <script type="text/javascript"
7 src="../../../../canonical/launchpad/icing/lazr/build/lazr.js"></script>
8 <link rel="stylesheet"
9 href="../../../../canonical/launchpad/icing/yui/cssreset/reset.css"/>
10 <link rel="stylesheet"
11 href="../../../../canonical/launchpad/icing/yui/cssfonts/fonts.css"/>
12 <link rel="stylesheet"
13 href="../../../../canonical/launchpad/icing/yui/cssbase/base.css"/>
14 <link rel="stylesheet"
15 href="../../../../canonical/launchpad/javascript/test.css" />
16 <script type="text/javascript" src="../../../app/javascript/client.js"></script>
17 <script type="text/javascript" src="../../../app/javascript/lp.js"></script>
18
19 <!-- The module under test -->
20 <script type="text/javascript" src="../personpicker.js"></script>
21
22 <!-- The test suite -->
23 <script type="text/javascript" src="test_personpicker.js"></script>
24 </head>
25 <body class="yui-skin-sam">
26
27 <!-- The example markup required by the script to run -->
28 <input type="text" value="" id="choose"
29 name="choose" size="20"
30 maxlength=""
31 onKeyPress="" style=""
32 class="" />
33 <span class="">
34 (<a id="show-widget" href="/people/">Find&hellip;</a>)
35 </span>
36
37 <!-- The test output -->
38 <div id="log"></div>
39 </body>
40</html>
041
=== added file 'lib/lp/registry/javascript/tests/test_personpicker.js'
--- lib/lp/registry/javascript/tests/test_personpicker.js 1970-01-01 00:00:00 +0000
+++ lib/lp/registry/javascript/tests/test_personpicker.js 2011-06-01 14:02:31 +0000
@@ -0,0 +1,66 @@
1/* Copyright 2011 Canonical Ltd. This software is licensed under the
2 * GNU Affero General Public License version 3 (see the file LICENSE).
3 */
4
5YUI({
6 base: '../../../../canonical/launchpad/icing/yui/',
7 filter: 'raw', combine: false,
8 fetchCSS: false
9 }).use('test', 'console', 'plugin', 'lp.registry.personpicker',
10 'node-event-simulate', function(Y) {
11
12 var suite = new Y.Test.Suite("lp.registry.personpicker Tests");
13
14 suite.add(new Y.Test.Case({
15 name: 'personpicker',
16
17 setUp: function() {
18 window.LP = {
19 links: {me: '/~no-one'},
20 cache: {}
21 }
22 },
23
24 test_render: function () {
25 var personpicker = new Y.lp.registry.personpicker.PersonPicker();
26 personpicker.render();
27 personpicker.show();
28
29 // The extra buttons section exists
30 Y.Assert.isNotNull(Y.one('.extra-form-buttons'));
31 Y.Assert.isNotNull(Y.one('.yui-picker-remove-button'));
32 Y.Assert.isNotNull(Y.one('.yui-picker-assign-me-button'));
33 },
34
35 test_buttons: function () {
36 var personpicker = new Y.lp.registry.personpicker.PersonPicker();
37 personpicker.render();
38 personpicker.show();
39
40 // Patch the picker so the assign_me and remove methods can be
41 // tested.
42 var data = null;
43 personpicker.on('save', function (result) {
44 data = result.value;
45 });
46 var remove = Y.one('.yui-picker-remove-button');
47 remove.simulate('click');
48 Y.Assert.areEqual('', data);
49
50 var assign_me = Y.one('.yui-picker-assign-me-button');
51 assign_me.simulate('click');
52 Y.Assert.areEqual('no-one', data);
53 }
54 }));
55
56 // Lock, stock, and two smoking barrels.
57 Y.Test.Runner.add(suite);
58
59 var console = new Y.Console({newestOnTop: false});
60 console.render('#log');
61
62 Y.on('domready', function() {
63 Y.Test.Runner.run();
64 });
65});
66