Code review comment for lp:~gary/launchpad/launchpad-templates-3

Revision history for this message
Edwin Grubbs (edwin-grubbs) wrote :

Hi Gary,

Thanks for getting rid of all this cruft. I only have a few suggestions for
changes.

merge-conditional

Need to remove default_editform, launchpad_addform, launchpad_editform,
and message_add from pagetitles.py. Since these are just used as macros, they
don't need to have a view class with a page_title either.

Can you open up a bug to see if the few templates that use these the
launchpad_addform and launchpad_editform macros can be updated not to use them?
If these macros are still necessary, the <h1> in these macros needs to be
removed at the same time that the templates using them are converted to UI 3.0.
Otherwise, there will be duplicate <h1> tags.

-Edwin

>=== modified file 'lib/canonical/launchpad/browser/hwdb.py'
>--- lib/canonical/launchpad/browser/hwdb.py 2009-08-14 13:03:36 +0000
>+++ lib/canonical/launchpad/browser/hwdb.py 2009-09-18 21:42:33 +0000
>@@ -35,6 +35,8 @@
> """View class for hardware database submissions."""
>
> schema = IHWSubmissionForm
>+ label = 'Hardware Database Submission'
>+ page_title = 'Submit New Data to the Launchpad Hardware Database'
>
> @action(u'Upload', name='upload')
> def upload_action(self, action, data):
>@@ -150,6 +152,14 @@
> class HWDBPersonSubmissionsView(LaunchpadView):
> """View class for preseting HWDB submissions by a person."""
>
>+ @property
>+ def label(self):
>+ return 'Hardware submissions for %s' % (self.context.title,)
>+
>+ @property
>+ def page_title(self):
>+ return "Hardware Database submissions by %s" % (self.context.title,)

I think I told you something different in previous reviews, but it was
made clear to me that the page_title is only necessary for toplevel pages
that it can't compute breadcrumbs for, so you can get rid of it here.

> def getAllBatched(self):
> """Return the list of HWDB submissions made by this person."""
> hw_submissionset = getUtility(IHWSubmissionSet)
>@@ -255,6 +265,7 @@
> """View class for lists of HWDB submissions for a system fingerprint."""
>
> implements(IBrowserPublisher)
>+ label = page_title = "Hardware Database submissions for a fingerprint"
>
>
> template = ViewPageTemplateFile(
> '../templates/hwdb-fingerprint-submissions.pt')
>
>=== modified file 'lib/canonical/launchpad/templates/hwdb-fingerprint-submissions.pt'
>--- lib/canonical/launchpad/templates/hwdb-fingerprint-submissions.pt 2009-07-17 17:59:07 +0000
>+++ lib/canonical/launchpad/templates/hwdb-fingerprint-submissions.pt 2009-09-18 21:42:33 +0000
>@@ -3,10 +3,7 @@
> 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="view/macro:page/applicationhome"
>+ metal:use-macro="view/macro:page/main_only"
> i18n:domain="launchpad"
> >

This file also fills the "heading" slot, which should be removed.

>=== modified file 'lib/canonical/launchpad/templates/notification-test.pt'
>--- lib/canonical/launchpad/templates/notification-test.pt 2009-07-17 17:59:07 +0000
>+++ lib/canonical/launchpad/templates/notification-test.pt 2009-09-18 21:42:33 +0000
>@@ -3,10 +3,7 @@
> 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"
>+ metal:use-macro="view/macro:page/main_only"
> i18n:domain="launchpad"
> >

This template also fills the 'help' slot, and that needs to be removed.

review: Approve (code ui*)

« Back to merge proposal