Launchpad itself

Merge lp:~deryck/launchpad/comment-add-ui-updates-413611 into lp:launchpad

  1. comment-add-ui-updates-413611
  2. Merge into devel
Proposed by Deryck Hodge
Status: Merged
Merged at revision: not available
Proposed branch: lp:~deryck/launchpad/comment-add-ui-updates-413611
Merge into: lp:launchpad
Diff against target: None lines
To merge this branch: bzr merge lp:~deryck/launchpad/comment-add-ui-updates-413611
Reviewer Review Type Date Requested Status
Eleanor Berger (community) ui Approve
Gavin Panella (community) Approve
Review via email: mp+10383@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Deryck Hodge (deryck) wrote :

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

These are mechanical changes to get the add a comment or attachment page
updated to the 3.0 UI. See bug 413611 for lots of discussion as I
learned my way around what look these changes should ultimately produce.

There are several tests which can verify these changes are clean in
lib/lp/bugs/stories/bugattachments.

./bin/test -vv -t stories/bugattachments

= 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/bugs/browser/bugmessage.py
  lib/canonical/launchpad/pagetitles.py
  lib/lp/bugs/templates/bug-comment-add.pt
  lib/lp/bugs/browser/configure.zcml

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iEYEARECAAYFAkqL+zwACgkQ4glRK0DaE8gQ0QCePOaclGw9wBWuEW2bZVdU17FP
jFEAoN04Kn+0/JlZ3BVoX7fr9fYmOwoH
=4N/z
-----END PGP SIGNATURE-----

Revision history for this message
Gavin Panella (allenap) wrote :
Download full text (3.8 KiB)

Hi Deryck,

Looks good. The layout of the page:

  <h2>Bug #11 in Jokosher: "Make Jokosher use autoaudiosink"</h2>
  <h1>Add a comment or attachment to Bug #11</h1>

is better than, IMO, the layout I did for the bug activity log:

  <h1>Activity log for bug #11: Make Jokosher use autoaudiosink</h1>

That would have been better as:

  <h2>Bug #11 in Jokosher: "Make Jokosher use autoaudiosink"</h2>
  <h1>Activity log for bug #11</h1>

So that there's a consistent look, could you could patch the bug
activity stuff to look the same as here and land it in this branch?
There's a patch at <http://pastebin.ubuntu.com/255760/>.

Other than that, only one tiny comment :)

Gavin.

> === modified file 'lib/canonical/launchpad/pagetitles.py'
> --- lib/canonical/launchpad/pagetitles.py 2009-08-19 02:58:06 +0000
> +++ lib/canonical/launchpad/pagetitles.py 2009-08-19 13:49:59 +0000
> @@ -230,8 +230,6 @@
>
> bug_branch_add = LaunchbagBugID('Bug #%d - Add branch')
>
> -bug_comment_add = LaunchbagBugID('Bug #%d - Add a comment or attachment')
> -
> bug_cve = LaunchbagBugID("Bug #%d - Add CVE reference")
>
> bug_edit = ContextBugId('Bug #%d - Edit')
>
> === modified file 'lib/lp/bugs/browser/bugmessage.py'
> --- lib/lp/bugs/browser/bugmessage.py 2009-08-05 15:05:52 +0000
> +++ lib/lp/bugs/browser/bugmessage.py 2009-08-19 13:49:59 +0000
> @@ -25,6 +25,14 @@
> initial_focus_widget = None
>
> @property
> + def label(self):
> + return 'Add a comment or attachment to Bug #%d' % self.context.bug.id

I think: s/Bug/bug/

> +
> + @property
> + def page_title(self):
> + return self.label
> +
> + @property
> def initial_values(self):
> return dict(subject=self.context.bug.followup_subject())
>
>
> === modified file 'lib/lp/bugs/browser/configure.zcml'
> --- lib/lp/bugs/browser/configure.zcml 2009-07-27 12:39:34 +0000
> +++ lib/lp/bugs/browser/configure.zcml 2009-08-19 13:49:59 +0000
> @@ -524,7 +524,7 @@
> <browser:page
> name="+addcomment"
> for="lp.bugs.interfaces.bugtask.IBugTask"
> - class="lp.bugs.browser.bug.BugView"
> + class="lp.bugs.browser.bugmessage.BugMessageAddFormView"
> permission="launchpad.Edit"
> template="../templates/bug-comment-add.pt"/>
> <browser:page
>
> === modified file 'lib/lp/bugs/templates/bug-comment-add.pt'
> --- lib/lp/bugs/templates/bug-comment-add.pt 2009-07-17 17:59:07 +0000
> +++ lib/lp/bugs/templates/bug-comment-add.pt 2009-08-19 13:49:59 +0000
> @@ -3,33 +3,12 @@
> xmlns:tal="http://xml.zope.org/namespaces/tal"
> xmlns:metal="http://xml.zope.org/namespaces/metal"
> xmlns:i18n="http://xml.zope.org/namespaces/i18n"
> - xml:lang="en"
> - lang="en"
> - dir="ltr"
> - metal:use-macro="context/@@main_template/master"
> - i18n:domain="malone"
> ->
> -
> -<metal:rightportlets fill-slot="portlets_two">
> - <div tal:replace="structure context/bug/@@+portlet-attachments" />
> - <div tal:replace="structure context/@@+portlet-search" />
> -</metal:rightportlets>
> -
> -<body>
> - <metal:heading fill-slot="pageheading"
> - tal:define="bugtask context">
> - <div>
> - ...

Read more...

review: Approve
Revision history for this message
Deryck Hodge (deryck) wrote :

Hi, Gavin.

Thanks for the review!

On Wed, Aug 19, 2009 at 9:17 AM, Gavin
Panella<email address hidden> wrote:
> Looks good. The layout of the page:
>
>  <h2>Bug #11 in Jokosher: "Make Jokosher use autoaudiosink"</h2>
>  <h1>Add a comment or attachment to Bug #11</h1>
>
> is better than, IMO, the layout I did for the bug activity log:

I've updated my branch with your patch to match. Glad you mentioned
that. As I understand it from Curtis, this is the form we should use
for most all the pages, only overriding the heading slot for an
object's index page.

> Other than that, only one tiny comment :)

The "Bug #XXX" capitalization has been updated as well. :)

Cheers,
deryck

Revision history for this message
Eleanor Berger (intellectronica) wrote :

The result doesn't look particularly beautiful. The title repeats the same information as the line above it, and the attachment box is much wider than the comment box. Still, since this is just the fallback option and very few users will actually see it, let's go with that.

review: Approve (ui)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'lib/canonical/launchpad/pagetitles.py'
--- lib/canonical/launchpad/pagetitles.py 2009-08-19 02:58:06 +0000
+++ lib/canonical/launchpad/pagetitles.py 2009-08-19 12:24:35 +0000
@@ -230,8 +230,6 @@
230230
231bug_branch_add = LaunchbagBugID('Bug #%d - Add branch')231bug_branch_add = LaunchbagBugID('Bug #%d - Add branch')
232232
233bug_comment_add = LaunchbagBugID('Bug #%d - Add a comment or attachment')
234
235bug_cve = LaunchbagBugID("Bug #%d - Add CVE reference")233bug_cve = LaunchbagBugID("Bug #%d - Add CVE reference")
236234
237bug_edit = ContextBugId('Bug #%d - Edit')235bug_edit = ContextBugId('Bug #%d - Edit')
238236
=== modified file 'lib/lp/bugs/browser/bugmessage.py'
--- lib/lp/bugs/browser/bugmessage.py 2009-08-05 15:05:52 +0000
+++ lib/lp/bugs/browser/bugmessage.py 2009-08-19 12:23:51 +0000
@@ -25,6 +25,14 @@
25 initial_focus_widget = None25 initial_focus_widget = None
2626
27 @property27 @property
28 def label(self):
29 return 'Add a comment or attachment to Bug #%d' % self.context.bug.id
30
31 @property
32 def page_title(self):
33 return self.label
34
35 @property
28 def initial_values(self):36 def initial_values(self):
29 return dict(subject=self.context.bug.followup_subject())37 return dict(subject=self.context.bug.followup_subject())
3038
3139
=== modified file 'lib/lp/bugs/browser/configure.zcml'
--- lib/lp/bugs/browser/configure.zcml 2009-07-27 12:39:34 +0000
+++ lib/lp/bugs/browser/configure.zcml 2009-08-19 12:23:51 +0000
@@ -524,7 +524,7 @@
524 <browser:page524 <browser:page
525 name="+addcomment"525 name="+addcomment"
526 for="lp.bugs.interfaces.bugtask.IBugTask"526 for="lp.bugs.interfaces.bugtask.IBugTask"
527 class="lp.bugs.browser.bug.BugView"527 class="lp.bugs.browser.bugmessage.BugMessageAddFormView"
528 permission="launchpad.Edit"528 permission="launchpad.Edit"
529 template="../templates/bug-comment-add.pt"/>529 template="../templates/bug-comment-add.pt"/>
530 <browser:page530 <browser:page
531531
=== modified file 'lib/lp/bugs/templates/bug-comment-add.pt'
--- lib/lp/bugs/templates/bug-comment-add.pt 2009-07-17 17:59:07 +0000
+++ lib/lp/bugs/templates/bug-comment-add.pt 2009-08-14 16:35:46 +0000
@@ -3,33 +3,12 @@
3 xmlns:tal="http://xml.zope.org/namespaces/tal"3 xmlns:tal="http://xml.zope.org/namespaces/tal"
4 xmlns:metal="http://xml.zope.org/namespaces/metal"4 xmlns:metal="http://xml.zope.org/namespaces/metal"
5 xmlns:i18n="http://xml.zope.org/namespaces/i18n"5 xmlns:i18n="http://xml.zope.org/namespaces/i18n"
6 xml:lang="en"6 metal:use-macro="view/macro:page/main_only"
7 lang="en"7 i18n:domain="malone">
8 dir="ltr"8 <body>
9 metal:use-macro="context/@@main_template/master"9 <div metal:fill-slot="main">
10 i18n:domain="malone"10 <div tal:content="structure context/@@+addcomment-form" />
11>
12
13<metal:rightportlets fill-slot="portlets_two">
14 <div tal:replace="structure context/bug/@@+portlet-attachments" />
15 <div tal:replace="structure context/@@+portlet-search" />
16</metal:rightportlets>
17
18<body>
19 <metal:heading fill-slot="pageheading"
20 tal:define="bugtask context">
21 <div>
22 <img alt="" src="/@@/bug" />
23 <a tal:attributes="href bugtask/fmt:url"
24 >Bug #<tal:bug replace="context/bug/id">3252543</tal:bug></a>:
25 <tal:title replace="context/bug/title">Foo doesn't work</tal:title>
26 </div>11 </div>
27 <h1>Add a comment or attachment</h1>12 </body>
28 </metal:heading>
29 <div metal:fill-slot="main">
30 <div tal:content="structure context/@@+addcomment-form" />
31 </div>
32</body>
33
34</html>13</html>
3514