Launchpad itself

Code review comment for lp:~deryck/launchpad/comment-add-ui-updates-413611

  1. comment-add-ui-updates-413611
  2. Merge into devel
Revision history for this message
Gavin Panella (allenap) wrote :

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>
> - <img alt="" src="/@@/bug" />
> - <a tal:attributes="href bugtask/fmt:url"
> - >Bug #<tal:bug replace="context/bug/id">3252543</tal:bug></a>:
> - <tal:title replace="context/bug/title">Foo doesn't work</tal:title>
> + metal:use-macro="view/macro:page/main_only"
> + i18n:domain="malone">
> + <body>
> + <div metal:fill-slot="main">
> + <div tal:content="structure context/@@+addcomment-form" />
> </div>
> - <h1>Add a comment or attachment</h1>
> - </metal:heading>
> - <div metal:fill-slot="main">
> - <div tal:content="structure context/@@+addcomment-form" />
> - </div>
> -</body>
> -
> + </body>
> </html>
>
>

review: Approve

« Back to merge proposal