Launchpad itself

Code review comment for lp://staging/~cprov/launchpad/bug-412715-registration-info-in-base-layout

  1. bug-412715-registration-info-in-base-layout
  2. Merge into devel
Revision history for this message
Edwin Grubbs (edwin-grubbs) wrote :

Hi Celso,

This branch looks like a nice improvement. Just a couple of comments below.

merge-conditional

-Edwin

>=== modified file 'lib/lp/app/browser/tests/base-layout.txt'
>--- lib/lp/app/browser/tests/base-layout.txt 2009-09-12 01:39:04 +0000
>+++ lib/lp/app/browser/tests/base-layout.txt 2009-09-16 20:19:40 +0000
>@@ -93,7 +93,10 @@
> ...
> <div class="yui-b" dir="ltr">
> <div>
>- <h2>Heading</h2>
>+ <div class="registering">
>+ <div />
>+ </div>
>+ <h2>Heading</h2>
> <ol class="breadcrumbs" xmlns="http://www.w3.org/1999/xhtml">
> ...
>
>@@ -189,6 +192,9 @@
> <BLANKLINE>
> <div class="yui-b" dir="ltr">
> <div>
>+ <div class="registering">
>+ <div />
>+ </div>
> <BLANKLINE>
> <ol class="breadcrumbs" xmlns="http://www.w3.org/1999/xhtml">
> <BLANKLINE>
>@@ -307,6 +313,9 @@
> <BLANKLINE>
> <div class="yui-b" dir="ltr">
> <div>
>+ <div class="registering">
>+ <div />
>+ </div>
> <BLANKLINE>
> <ol class="breadcrumbs" xmlns="http://www.w3.org/1999/xhtml">
> <BLANKLINE>
>@@ -463,6 +472,34 @@
> <h1>Heading</h1>
>
>
>+Page registering
>+----------------
>+
>+The 'registering' slot is presented on the right side of the 'heading'
>+and can be filled with the context registering information (registrant
>+and date_created, normally).
>+
>+ >>> class RegisteringView(LaunchpadView):
>+ ... """A simple view to test base-layout."""
>+ ... __launchpad_facetname__ = 'overview'
>+ ... template = ViewPageTemplateFile('testfiles/main-registering.pt')
>+ ... page_title = 'Test base-layout: registering'

What is the purpose of this page_title variable that doesn't appear to be used
anywhere?

>+
>+ >>> view = RegisteringView(user, request)
>+ >>> html = view.render()
>+
>+ >>> from canonical.launchpad.testing.pages import (
>+ ... first_tag_by_class)
>+
>+ >>> print first_tag_by_class(html, 'registering')
>+ <div class="registering">
>+ <p>something nice about the context registering.</p>
>+ </div>
>+
>+Note that the slot itself will be already 'styled' and it rarely has
>+to be changed.
>+
>+
> Public and private presentation
> -------------------------------
>
>
>=== added file 'lib/lp/app/browser/tests/testfiles/main-registering.pt'
>--- lib/lp/app/browser/tests/testfiles/main-registering.pt 1970-01-01 00:00:00 +0000
>+++ lib/lp/app/browser/tests/testfiles/main-registering.pt 2009-09-16 20:19:40 +0000
>@@ -0,0 +1,19 @@
>+<html
>+ xmlns="http://www.w3.org/1999/xhtml"
>+ xmlns:tal="http://xml.zope.org/namespaces/tal"
>+ xmlns:metal="http://xml.zope.org/namespaces/metal"
>+ xmlns:i18n="http://xml.zope.org/namespaces/i18n"
>+ metal:use-macro="view/macro:page/main_only">
>+
>+ <body>
>+
>+ <tal:registering metal:fill-slot="registering">
>+ <p>something nice about the context registering.</p>
>+ </tal:registering>
>+
>+ <tal:main metal:fill-slot="main">
>+ ANYTHING
>+ </tal:main>
>+
>+ </body>
>+</html>
>
>=== modified file 'lib/lp/soyuz/templates/build-index.pt'
>--- lib/lp/soyuz/templates/build-index.pt 2009-09-16 12:31:06 +0000
>+++ lib/lp/soyuz/templates/build-index.pt 2009-09-16 20:19:40 +0000
>@@ -10,19 +10,18 @@
> <body>
>
> <div metal:fill-slot="heading">
>- <div style="float: left;">
> <h1 tal:content="CONTEXTS/fmt:pagetitle">i386 build of
> alsa-utils 1.0.9a-4ubuntu1 in ubuntu hoary RELEASE</h1>
>- </div>
>- <div style="float: right;">
>- <p class="discreet">
>- Created <span
>- tal:attributes="title context/datecreated/fmt:datetime"
>- tal:content="context/datecreated/fmt:displaydate"
>- >2005-10-05 5:45</span>
>+ </div> <!-- heading -->

CONTEXTS/fmt:pagetitle is dependent on lib/c/l/pagetitles.py, which we
want to get rid of. The preferred way to move the <h1> out of the template
is to add a view.label attribute. Since, the BuildView is shared by both
build-index.pt and build-retry.pt, subclasses would need to be created to
provide them with the distinct label properties that match the build_index
and build_retry variables, which need to be removed from lib/c/l/pagetitles.py.

>+
>+ <tal:registering metal:fill-slot="registering">
>+ <p>
>+ created
>+ <span tal:content="context/datecreated/fmt:displaydate"
>+ tal:attributes="title context/datecreated/fmt:datetime"
>+ >on 2005-01-01</span>
> </p>
>- </div>
>- </div> <!-- heading -->
>+ </tal:registering>
>
> <div metal:fill-slot="main">
>
>

review: Approve

« Back to merge proposal