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
=== modified file 'lib/lp/registry/templates/person-index.pt'
--- lib/lp/registry/templates/person-index.pt 2009-09-16 17:56:17 +0000
+++ lib/lp/registry/templates/person-index.pt 2009-09-18 17:29:26 +0000
@@ -45,13 +45,9 @@
45<div metal:fill-slot="side" tal:define="overview_menu context/menu:overview">45<div metal:fill-slot="side" tal:define="overview_menu context/menu:overview">
46 <tal:menu replace="structure view/@@+global-actions" />46 <tal:menu replace="structure view/@@+global-actions" />
4747
48 <div class="portlet"48 <metal:contact use-macro="context/@@+person-macros/contact">
49 tal:condition="view/can_contact">49 Contact this person
50 <a href="+contactuser" class="sprite mail"50 </metal:contact>
51 tal:attributes="title view/contact_link_title"
52 tal:content="view/specific_contact_text">Contact
53 this person</a>
54 </div>
5551
56 <div id="involvement" class="portlet involvement"52 <div id="involvement" class="portlet involvement"
57 tal:condition="context/is_valid_person">53 tal:condition="context/is_valid_person">
5854
=== modified file 'lib/lp/registry/templates/person-macros.pt'
--- lib/lp/registry/templates/person-macros.pt 2009-09-09 18:50:13 +0000
+++ lib/lp/registry/templates/person-macros.pt 2009-09-18 17:29:26 +0000
@@ -23,14 +23,6 @@
23 No contact email23 No contact email
24 </tal:no_preferredemail>24 </tal:no_preferredemail>
2525
26 <div tal:condition="view/can_contact">
27 <img src="/@@/mail" alt="email"/>
28 <a href="+contactuser"
29 tal:attributes="title view/contact_link_title"
30 tal:content="view/specific_contact_text"
31 >Contact this person</a>
32 </div>
33
34 <tal:emails repeat="email view/visible_email_addresses">26 <tal:emails repeat="email view/visible_email_addresses">
35 <div style="white-space: nowrap">27 <div style="white-space: nowrap">
36 <img src="/@@/mail" alt="mail" />28 <img src="/@@/mail" alt="mail" />
@@ -80,6 +72,17 @@
80 </div>72 </div>
81</metal:macro>73</metal:macro>
8274
75<!-- contact macro -->
76<metal:macro define-macro="contact">
77 <div class="portlet"
78 tal:condition="view/can_contact">
79 <a href="+contactuser" class="sprite mail"
80 tal:attributes="title view/contact_link_title"
81 tal:content="view/specific_contact_text">Contact
82 this person</a>
83 </div>
84</metal:macro>
85
83<!-- subteam-of macro. -->86<!-- subteam-of macro. -->
84<metal:macro define-macro="subteam-of">87<metal:macro define-macro="subteam-of">
85 <div class="portlet"88 <div class="portlet"
8689
=== modified file 'lib/lp/registry/templates/team-index.pt'
--- lib/lp/registry/templates/team-index.pt 2009-09-16 13:46:16 +0000
+++ lib/lp/registry/templates/team-index.pt 2009-09-18 17:29:26 +0000
@@ -30,6 +30,10 @@
30 <span tal:content="view/visibility_info">Viewable by team members.</span>30 <span tal:content="view/visibility_info">Viewable by team members.</span>
31 </div>31 </div>
3232
33 <metal:contact use-macro="context/@@+person-macros/contact">
34 Contact this team
35 </metal:contact>
36
33 <ul id="join-team"37 <ul id="join-team"
34 class="super-add-action"38 class="super-add-action"
35 tal:define="link context/menu:overview/join"39 tal:define="link context/menu:overview/join"