Launchpad itself

Merge lp:~bac/launchpad/bug-422334-contact-team into lp:launchpad

  1. bug-422334-contact-team
  2. Merge into devel
Proposed by Brad Crittenden
Status: Merged
Approved by: Curtis Hovey
Approved revision: no longer in the source branch.
Merged at revision: not available
Proposed branch: lp:~bac/launchpad/bug-422334-contact-team
Merge into: lp:launchpad
Diff against target: None lines
To merge this branch: bzr merge lp:~bac/launchpad/bug-422334-contact-team
Reviewer Review Type Date Requested Status
Curtis Hovey (community) code Approve
Review via email: mp+12089@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Brad Crittenden (bac) wrote :

= Summary =

Bug 422334 is about the 'Contact team' link being in the team details portlet on
team-index.pt and not in the side action portlet as in person-index.pt.

== Proposed fix ==

Move the contact link to the side. Since the code from person-index.pt needs to be
duplicated in team-index.pt it was turned into a macro in person-macros.pt for re-use.

The new layout can be seen at:
http://people.canonical.com/~bac/bug-422334-contact-team/

== Pre-implementation notes ==

IRC notes from Curtis.

== Implementation details ==

As above.

== Tests ==

None as the link is checked in existing tests but not placement.

== Demo and Q/A ==

Go to a team page and verify the placement.

= Launchpad lint =

Checking for conflicts. and issues in doctests and templates.
Running jslint, xmllint, pyflakes, and pylint.
Using normal rules.

Linting changed files:
  lib/lp/registry/templates/team-index.pt
  lib/lp/registry/templates/person-index.pt
  lib/lp/registry/templates/person-macros.pt

Revision history for this message
Curtis Hovey (sinzui) wrote :

This is lovely improvement. Thank you

review: Approve
Revision history for this message
Brad Crittenden (bac) wrote :

Thanks for the review, Curtis. Testing revealed another required change:

http://pastebin.ubuntu.com/273728/

Revision history for this message
Brad Crittenden (bac) wrote :

The fix to restore the link to +mugshots was added. A full diff is at http://pastebin.ubuntu.com/273791/

Revision history for this message
Curtis Hovey (sinzui) wrote :

> The fix to restore the link to +mugshots was added. A full diff is at
> http://pastebin.ubuntu.com/273791/

Thank you very much for finding and fixing this problem.

review: Approve (code)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/registry/templates/person-index.pt'
2--- lib/lp/registry/templates/person-index.pt 2009-09-16 17:56:17 +0000
3+++ lib/lp/registry/templates/person-index.pt 2009-09-18 17:29:26 +0000
4@@ -45,13 +45,9 @@
5 <div metal:fill-slot="side" tal:define="overview_menu context/menu:overview">
6 <tal:menu replace="structure view/@@+global-actions" />
7
8- <div class="portlet"
9- tal:condition="view/can_contact">
10- <a href="+contactuser" class="sprite mail"
11- tal:attributes="title view/contact_link_title"
12- tal:content="view/specific_contact_text">Contact
13- this person</a>
14- </div>
15+ <metal:contact use-macro="context/@@+person-macros/contact">
16+ Contact this person
17+ </metal:contact>
18
19 <div id="involvement" class="portlet involvement"
20 tal:condition="context/is_valid_person">
21
22=== modified file 'lib/lp/registry/templates/person-macros.pt'
23--- lib/lp/registry/templates/person-macros.pt 2009-09-09 18:50:13 +0000
24+++ lib/lp/registry/templates/person-macros.pt 2009-09-18 17:29:26 +0000
25@@ -23,14 +23,6 @@
26 No contact email
27 </tal:no_preferredemail>
28
29- <div tal:condition="view/can_contact">
30- <img src="/@@/mail" alt="email"/>
31- <a href="+contactuser"
32- tal:attributes="title view/contact_link_title"
33- tal:content="view/specific_contact_text"
34- >Contact this person</a>
35- </div>
36-
37 <tal:emails repeat="email view/visible_email_addresses">
38 <div style="white-space: nowrap">
39 <img src="/@@/mail" alt="mail" />
40@@ -80,6 +72,17 @@
41 </div>
42 </metal:macro>
43
44+<!-- contact macro -->
45+<metal:macro define-macro="contact">
46+ <div class="portlet"
47+ tal:condition="view/can_contact">
48+ <a href="+contactuser" class="sprite mail"
49+ tal:attributes="title view/contact_link_title"
50+ tal:content="view/specific_contact_text">Contact
51+ this person</a>
52+ </div>
53+</metal:macro>
54+
55 <!-- subteam-of macro. -->
56 <metal:macro define-macro="subteam-of">
57 <div class="portlet"
58
59=== modified file 'lib/lp/registry/templates/team-index.pt'
60--- lib/lp/registry/templates/team-index.pt 2009-09-16 13:46:16 +0000
61+++ lib/lp/registry/templates/team-index.pt 2009-09-18 17:29:26 +0000
62@@ -30,6 +30,10 @@
63 <span tal:content="view/visibility_info">Viewable by team members.</span>
64 </div>
65
66+ <metal:contact use-macro="context/@@+person-macros/contact">
67+ Contact this team
68+ </metal:contact>
69+
70 <ul id="join-team"
71 class="super-add-action"
72 tal:define="link context/menu:overview/join"