Launchpad itself

Merge lp:~deryck/launchpad/person-bug-page-ui-update-434794 into lp:launchpad

  1. person-bug-page-ui-update-434794
  2. Merge into devel
Proposed by Deryck Hodge
Status: Merged
Approved by: Brad Crittenden
Approved revision: no longer in the source branch.
Merged at revision: not available
Proposed branch: lp:~deryck/launchpad/person-bug-page-ui-update-434794
Merge into: lp:launchpad
Diff against target: None lines
To merge this branch: bzr merge lp:~deryck/launchpad/person-bug-page-ui-update-434794
Reviewer Review Type Date Requested Status
Brad Crittenden (community) release-critical Approve
Graham Binns (community) code Approve
Jeroen T. Vermeulen (community) Needs Fixing
Review via email: mp+12275@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Deryck Hodge (deryck) wrote :

The final template converted! w00t!

This converts the template for displaying a person's bug page and
related bug pages (assigned bugs, commented bugs, etc.) These are just
mechanical changes.

This is tracked at bug 434794.

To demo, visit https://bugs.launchpad.dev/~name16
(or any other user bug page).

= 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/stories/foaf/xx-person-bugs.txt
  lib/lp/bugs/templates/buglisting-embedded-advanced-search.pt
  lib/lp/bugs/browser/bugtask.py
  lib/lp/registry/browser/person.py

== Pylint notices ==

lib/lp/bugs/browser/bugtask.py
    73: [F0401] Unable to import 'lazr.delegates' (No module named
delegates)
    74: [F0401] Unable to import 'lazr.enum' (No module named enum)
    76: [F0401] Unable to import 'lazr.lifecycle.event' (No module named
lifecycle)
    77: [F0401] Unable to import 'lazr.lifecycle.snapshot' (No module
named lifecycle)
    78: [F0401] Unable to import 'lazr.restful.interface' (No module
named restful)
    79: [F0401] Unable to import 'lazr.restful.interfaces' (No module
named restful)
    149: [F0401] Unable to import 'lazr.restful.interfaces' (No module
named restful)

lib/lp/registry/browser/person.py
    117: [F0401] Unable to import 'lazr.delegates' (No module named
delegates)
    118: [F0401] Unable to import 'lazr.config' (No module named config)
    119: [F0401] Unable to import 'lazr.restful.interface' (No module
named restful)

Revision history for this message
Jeroen T. Vermeulen (jtv) wrote :

Several problems, as discussed on IRC:

 * Don't fill the heading slot. Let the h1 be generated from your view's label, and the title from your breadcrumbs.

 * getSearchPageHeading now provides completely wrong page_title values, so don't use those strings for that. The page_title now goes into the breadcrumbs, so it shouldn't repeat the context it's in. Usually the page_title should be a name ("Deryck H") *or* an action/page type ("Bugs"). It should not repeat anything, so if your breadcrumbs already say "Deryck H » Bugs" then say "Assigned," not "Bugs assigned to Deryck H" in the next one.

 * The getSearchPageHeading strings do look good for labels.

 * I guess there's no time to change this now, but the actions menu is basically a related-pages menu. I don't think it belongs in the sidebar in the new design; it probably ought to be a horizontal line of links at the top or bottom, with an info icon for each.

review: Needs Fixing
Revision history for this message
Deryck Hodge (deryck) wrote :
Download full text (6.6 KiB)

On Wed, Sep 23, 2009 at 7:50 AM, Jeroen T. Vermeulen <email address hidden> wrote:
>  * I guess there's no time to change this now, but the actions menu is basically a related-pages menu.  I don't think it belongs in the sidebar in the new design; it probably ought to be a horizontal line of links at the top or bottom, with an info icon for each.

I thought I followed examples in registry templates for this. If this
needs to be in a grid above the results I can file a bug, but it seems
fine to me as a side bar set. I do know people pages have related
pages in the side in other templates.

Here's an updated diff for everything else:

=== modified file 'lib/lp/bugs/browser/bugtask.py'
--- lib/lp/bugs/browser/bugtask.py 2009-09-23 12:13:38 +0000
+++ lib/lp/bugs/browser/bugtask.py 2009-09-23 13:01:20 +0000
@@ -3325,6 +3325,7 @@
                        "importance", "status"]
     schema = IFrontPageBugTaskSearch
     custom_widget('scope', ProjectScopeWidget)
+ page_title = 'Search'

     def initialize(self):
         """Initialize the view for the request."""
@@ -3359,7 +3360,7 @@
         return "Search all bug reports"

     @property
- def page_title(self):
+ def label(self):
         return self.getSearchPageHeading()

=== modified file 'lib/lp/bugs/templates/buglisting-embedded-advanced-search.pt'
--- lib/lp/bugs/templates/buglisting-embedded-advanced-search.pt 2009-09-23
04:31:16 +0000
+++ lib/lp/bugs/templates/buglisting-embedded-advanced-search.pt 2009-09-23
12:45:16 +0000
@@ -7,8 +7,6 @@
   i18n:domain="launchpad">

 <body>
- <h1 metal:fill-slot="heading" tal:content="view/page_title" />
-
   <div metal:fill-slot="side">
     <tal:menu replace="structure context/@@+global-actions" />
     <div tal:condition="view/shouldShowAssignedToTeamPortlet|nothing"

=== modified file 'lib/lp/registry/browser/person.py'
--- lib/lp/registry/browser/person.py 2009-09-23 04:38:00 +0000
+++ lib/lp/registry/browser/person.py 2009-09-23 13:04:10 +0000
@@ -1820,6 +1820,7 @@
     """Bugs reported on packages for a bug subscriber."""

     columns_to_show = ["id", "summary", "importance", "status"]
+ page_title = 'Package bugs'

     @property
     def current_package(self):
@@ -2013,6 +2014,10 @@
     def getSimpleSearchURL(self):
         return self.getBugSubscriberPackageSearchURL()

+ @property
+ def label(self):
+ return self.getSearchPageHeading()
+

 class RelevantMilestonesMixin:
     """Mixin to narrow the milestone list to only relevant milestones."""
@@ -2033,6 +2038,7 @@

     columns_to_show = ["id", "summary", "bugtargetdisplayname",
                        "importance", "status"]
+ page_title = 'Related bugs'

     def searchUnbatched(self, searchtext=None, context=None,
                         extra_params=None):
@@ -2082,7 +2088,7 @@
         return canonical_url(self.context, view_name="+bugs")

     @property
- def page_title(self):
+ def label(self):
         return self.getSearchPageHeading()

@@ -2092,6 +2098,7 @@

     columns_to_show = ["id", "summary", "bugtargetdisplayname",
                        "importance", "status"]
+ page_title = 'Assigned bugs'

     def searchUnbatched(self, search...

Read more...

Revision history for this message
Graham Binns (gmb) :
review: Approve (code)
Revision history for this message
Brad Crittenden (bac) wrote :

Hi Deryck,

Thanks for getting this template converted. The changes Martin suggested are ones we definitely need to include but there is no time to make the current release schedule.

r9566 of your branch is approved for release-critical submission. We'll tackle the other issues immediately afterwards.

Thanks for the hard work.

review: Approve (release-critical)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/bugs/browser/bugtask.py'
2--- lib/lp/bugs/browser/bugtask.py 2009-09-20 20:46:39 +0000
3+++ lib/lp/bugs/browser/bugtask.py 2009-09-23 12:13:38 +0000
4@@ -155,8 +155,8 @@
5 NewLineToSpacesWidget, NominationReviewActionWidget)
6 from canonical.widgets.itemswidgets import LabeledMultiCheckBoxWidget
7 from canonical.widgets.lazrjs import (
8- InlineEditPickerWidget, TextAreaEditorWidget,
9- TextLineEditorWidget, vocabulary_to_choice_edit_items)
10+ TextAreaEditorWidget, TextLineEditorWidget,
11+ vocabulary_to_choice_edit_items)
12 from canonical.widgets.project import ProjectScopeWidget
13
14 from lp.registry.vocabularies import MilestoneVocabulary
15@@ -1961,7 +1961,8 @@
16
17 @enabled_with_permission('launchpad.Edit')
18 def securitycontact(self):
19- return Link('+securitycontact', 'Change security contact', icon='edit')
20+ return Link(
21+ '+securitycontact', 'Change security contact', icon='edit')
22
23 def subscribe(self):
24 return Link('+subscribe', 'Subscribe to bug mail', icon='edit')
25@@ -3357,6 +3358,10 @@
26 """Return the heading to search all Bugs."""
27 return "Search all bug reports"
28
29+ @property
30+ def page_title(self):
31+ return self.getSearchPageHeading()
32+
33
34 class BugTaskPrivacyAdapter:
35 """Provides `IObjectPrivacy` for `IBugTask`."""
36
37=== modified file 'lib/lp/bugs/templates/buglisting-embedded-advanced-search.pt'
38--- lib/lp/bugs/templates/buglisting-embedded-advanced-search.pt 2009-07-17 17:59:07 +0000
39+++ lib/lp/bugs/templates/buglisting-embedded-advanced-search.pt 2009-09-23 04:31:16 +0000
40@@ -3,23 +3,17 @@
41 xmlns:tal="http://xml.zope.org/namespaces/tal"
42 xmlns:metal="http://xml.zope.org/namespaces/metal"
43 xmlns:i18n="http://xml.zope.org/namespaces/i18n"
44- xml:lang="en"
45- lang="en"
46- dir="ltr"
47- metal:use-macro="context/@@main_template/master"
48- i18n:domain="malone"
49->
50+ metal:use-macro="view/macro:page/main_side"
51+ i18n:domain="launchpad">
52
53 <body>
54-
55- <metal:heading fill-slot="pageheading">
56- <h1 tal:content="view/getSearchPageHeading"></h1>
57- </metal:heading>
58-
59- <metal:leftportlets fill-slot="portlets_one">
60- <div tal:condition="view/shouldShowAssignedToTeamPortlet|nothing"
61+ <h1 metal:fill-slot="heading" tal:content="view/page_title" />
62+
63+ <div metal:fill-slot="side">
64+ <tal:menu replace="structure context/@@+global-actions" />
65+ <div tal:condition="view/shouldShowAssignedToTeamPortlet|nothing"
66 tal:content="structure context/@@+portlet-team-assignedbugs" />
67- </metal:leftportlets>
68+ </div>
69
70 <div metal:fill-slot="main">
71 <tal:do_not_show_advanced_form
72@@ -36,8 +30,6 @@
73 use-macro="context/@@+bugtask-macros-tableview/advanced_search_form" />
74 </tal:show_advanced>
75 <div class="visualClear">&nbsp;</div>
76-
77 </div>
78-
79 </body>
80 </html>
81
82=== modified file 'lib/lp/registry/browser/person.py'
83--- lib/lp/registry/browser/person.py 2009-09-22 15:02:41 +0000
84+++ lib/lp/registry/browser/person.py 2009-09-23 04:38:00 +0000
85@@ -2081,6 +2081,10 @@
86 def getSimpleSearchURL(self):
87 return canonical_url(self.context, view_name="+bugs")
88
89+ @property
90+ def page_title(self):
91+ return self.getSearchPageHeading()
92+
93
94 class PersonAssignedBugTaskSearchListingView(RelevantMilestonesMixin,
95 BugTaskSearchListingView):
96@@ -2133,6 +2137,10 @@
97 """Return a URL that can be usedas an href to the simple search."""
98 return canonical_url(self.context, view_name="+assignedbugs")
99
100+ @property
101+ def page_title(self):
102+ return self.getSearchPageHeading()
103+
104
105 class PersonCommentedBugTaskSearchListingView(RelevantMilestonesMixin,
106 BugTaskSearchListingView):
107@@ -2173,6 +2181,10 @@
108 """Return a URL that can be used as an href to the simple search."""
109 return canonical_url(self.context, view_name="+commentedbugs")
110
111+ @property
112+ def page_title(self):
113+ return self.getSearchPageHeading()
114+
115
116 class PersonReportedBugTaskSearchListingView(RelevantMilestonesMixin,
117 BugTaskSearchListingView):
118@@ -2224,6 +2236,10 @@
119 """Should the tags combinator widget show on the search page?"""
120 return False
121
122+ @property
123+ def page_title(self):
124+ return self.getSearchPageHeading()
125+
126
127 class PersonSubscribedBugTaskSearchListingView(RelevantMilestonesMixin,
128 BugTaskSearchListingView):
129@@ -2264,6 +2280,10 @@
130 """Return a URL that can be used as an href to the simple search."""
131 return canonical_url(self.context, view_name="+subscribedbugs")
132
133+ @property
134+ def page_title(self):
135+ return self.getSearchPageHeading()
136+
137
138 class PersonVouchersView(LaunchpadFormView):
139 """Form for displaying and redeeming commercial subscription vouchers."""
140
141=== modified file 'lib/lp/registry/stories/foaf/xx-person-bugs.txt'
142--- lib/lp/registry/stories/foaf/xx-person-bugs.txt 2009-06-12 16:36:02 +0000
143+++ lib/lp/registry/stories/foaf/xx-person-bugs.txt 2009-09-23 04:36:10 +0000
144@@ -60,7 +60,7 @@
145
146 >>> anon_browser.getLink('List assigned bugs').click()
147 >>> print anon_browser.title
148- Bugs assigned to Sample Person
149+ Bugs assigned to Sample Person : Sample Person
150 >>> print anon_browser.url
151 http://bugs.launchpad.dev/~name12/+assignedbugs
152
153@@ -77,7 +77,7 @@
154
155 >>> anon_browser.getLink('List commented bugs').click()
156 >>> print anon_browser.title
157- Bugs commented on by Sample Person
158+ Bugs commented on by Sample Person : Sample Person
159 >>> print anon_browser.url
160 http://bugs.launchpad.dev/~name12/+commentedbugs
161
162@@ -98,7 +98,7 @@
163
164 >>> anon_browser.getLink('List reported bugs').click()
165 >>> print anon_browser.title
166- Bugs reported by Sample Person
167+ Bugs reported by Sample Person : Sample Person
168 >>> print anon_browser.url
169 http://bugs.launchpad.dev/~name12/+reportedbugs
170
171@@ -127,7 +127,7 @@
172
173 >>> anon_browser.getLink('List subscribed bugs').click()
174 >>> print anon_browser.title
175- Bugs Sample Person is subscribed to
176+ Bugs Sample Person is subscribed to : Sample Person
177 >>> print anon_browser.url
178 http://bugs.launchpad.dev/~name12/+subscribedbugs
179