Merge into devel : milestone-design-oops : Code : Launchpad itself

Status:	Merged
Merged at revision:	not available
Proposed branch:	lp:~sinzui/launchpad/milestone-design-oops
Merge into:	lp:launchpad
Diff against target:	None lines
To merge this branch:	bzr merge lp:~sinzui/launchpad/milestone-design-oops
Related bugs:	Link a bug report

Reviewer	Review Type	Date Requested	Status
Michael Nelson (community)	code ui*	2009-09-17	Approve on 2009-09-17
Review via email: mp+11946@code.launchpad.net

Revision history for this message

Curtis Hovey (sinzui) wrote on 2009-09-17:

#

This is my branch to fix timeout issues and IProject issues with the
milestone page and improve the layout.

    lp:~sinzui/launchpad/milestone-design-oops
    Diff size: 155
    Launchpad bug: https://bugs.launchpad.net/bugs/430726
    Test command: ./bin/test -vv -t "milestone-views|object-milestones"
    Pre-implementation: wgrant
    Target release: 3.1.0

= Fix timeout issues and IProject issues with the milestone page =

There is a content and a design problem with the milestone page.

1. There are many more timeouts because of the cost of showing the bug tags.
Remove them, they do not work well are they are in the design.

2. For most users, the side bar has one link, and if the context is an
   IProject, there is no side bar. The content does not fill the page well.
   Remove the side bar from the IProject case, consider moving the three
   action links inline or float a fake side bar so that the content can fill
   the page.

== Rules ==

1. Removed the tags from the page.

2. This is hard to do. I had several failures before settling on the CSS
   approach.
   a) I could not conditional set the page macro
   b) I could not reliably make a mock side bar in a main_only template --
      If someone changed the yui-* definition in the CSS, the template would
      be out of sync.
   c) I tried using CSS to collapse the yui-* widths to reclaim the space,
      but Firefox ignored them
   d) I decided to give up a just add a conditional CSS block that hides the
      empty background of the side bar.

3. I then saw that the Project milestone table is still very compressed
because it has a "Product" column. Well that is not right. It should
read "Project"

4. The I experimented with a new class that was on a different z-index and
very wide. The bug and spec work items filled the space very well.

5. I decided to a .wide to the release files portlets so that its description
was not compressed.

== QA ==

The side bar and wide tables show look like these images.
http://people.canonical.com/~curtis/milestones/

== Lint ==

Linting changed files:
  lib/canonical/launchpad/icing/style-3-0.css
  lib/lp/registry/browser/tests/milestone-views.txt
  lib/lp/registry/stories/milestone/object-milestones.txt
  lib/lp/registry/templates/milestone-index.pt

== Test ==

    * lib/lp/registry/browser/tests/milestone-views.txt
      * Added a test to verify that CSS is added to hide the project group
        milestone side bar.
    * lib/lp/registry/stories/milestone/object-milestones.txt
      * Updated the test to verify that the UI uses "Project", not "Product".

== Implementation ==

    * lib/canonical/launchpad/icing/style-3-0.css
      * Added a wide rule for content to reclaim the space lost to the
        side bar.
    * lib/lp/registry/templates/milestone-index.pt
      * Added CSS for the is_project_milestone condition
      * Added .wide to the product release files portlet and the bug/blueprint
        work items portlet
      * Removed the bug tags that were cause a zillion queries.

This is my branch to fix timeout issues and IProject issues with the
milestone page and improve the layout.

lp:~sinzui/launchpad/milestone-design-oops
    Diff size: 155
    Launchpad bug: https://bugs.launchpad.net/bugs/430726
    Test command: ./bin/test -vv -t "milestone-views|object-milestones"
    Pre-implementation: wgrant
    Target release: 3.1.0

= Fix timeout issues and IProject issues with the milestone page =

There is a content and a design problem with the milestone page.

1. There are many more timeouts because of the cost of showing the bug tags.
    Remove them, they do not work well are they are in the design.

2. For most users, the side bar has one link, and if the context is an
   IProject, there is no side bar. The content does not fill the page well.
   Remove the side bar from the IProject case, consider moving the three
   action links inline or float a fake side bar so that the content can fill
   the page.

== Rules ==

1. Removed the tags from the page.

2. This is hard to do. I had several failures before settling on the CSS
   approach.
   a) I could not conditional set the page macro
   b) I could not reliably make a mock side bar in a main_only template --
      If someone changed the yui-* definition in the CSS, the template would
      be out of sync.
   c) I tried using CSS to collapse the yui-* widths to reclaim the space,
      but Firefox ignored them
   d) I decided to give up a just add a conditional CSS block that hides the
      empty background of the side bar.

3. I then saw that the Project milestone table is still very compressed
   because it has a "Product" column. Well that is not right. It should
   read "Project"

4. The I experimented with a new class that was on a different z-index and
   very wide. The bug and spec work items filled the space very well.

5. I decided to a .wide to the release files portlets so that its description
   was not compressed.

== QA ==

The side bar and wide tables show look like these images.
    http://people.canonical.com/~curtis/milestones/

== Lint ==

Linting changed files:
  lib/canonical/launchpad/icing/style-3-0.css
  lib/lp/registry/browser/tests/milestone-views.txt
  lib/lp/registry/stories/milestone/object-milestones.txt
  lib/lp/registry/templates/milestone-index.pt

== Test ==

* lib/lp/registry/browser/tests/milestone-views.txt
      * Added a test to verify that CSS is added to hide the project group
        milestone side bar.
    * lib/lp/registry/stories/milestone/object-milestones.txt
      * Updated the test to verify that the UI uses "Project", not "Product".

== Implementation ==

* lib/canonical/launchpad/icing/style-3-0.css
      * Added a wide rule for content to reclaim the space lost to the
        side bar.
    * lib/lp/registry/templates/milestone-index.pt
      * Added CSS for the is_project_milestone condition
      * Added .wide to the product release files portlet and the bug/blueprint
        work items portlet
      * Removed the bug tags that were cause a zillion queries.

Revision history for this message

Michael Nelson (michael.nelson) wrote on 2009-09-17:

#

Download full text (10.9 KiB)

> This is my branch to fix timeout issues and IProject issues with the
> milestone page and improve the layout.
>

Hi Curtis, thanks for all the details here - and for finding a solution
to a common problem!

r=me (code/ui*)

I've got some questions below, but I'll leave it up to you whether they
are worth considering.

> lp:~sinzui/launchpad/milestone-design-oops
> Diff size: 155
> Launchpad bug: https://bugs.launchpad.net/bugs/430726
> Test command: ./bin/test -vv -t "milestone-views|object-milestones"
> Pre-implementation: wgrant
> Target release: 3.1.0

Did I miss 3.0?

>
>
> = Fix timeout issues and IProject issues with the milestone page =
>
> There is a content and a design problem with the milestone page.
>
> 1. There are many more timeouts because of the cost of showing the bug tags.
> Remove them, they do not work well are they are in the design.
>
> 2. For most users, the side bar has one link, and if the context is an
> IProject, there is no side bar. The content does not fill the page well.
> Remove the side bar from the IProject case, consider moving the three
> action links inline or float a fake side bar so that the content can fill
> the page.
>
>
> == Rules ==
>
> 1. Removed the tags from the page.

Great.

>
> 2. This is hard to do. I had several failures before settling on the CSS
> approach.
> a) I could not conditional set the page macro

I haven't need to do this but, if it's helpful, I found an example at:

http://docs.zope.org/zope2/zope2book/source/ZPT.html#macro-details

I assume this should work the same for us?

> b) I could not reliably make a mock side bar in a main_only template --
> If someone changed the yui-* definition in the CSS, the template would
> be out of sync.

Yep, good catch.

> c) I tried using CSS to collapse the yui-* widths to reclaim the space,
> but Firefox ignored them
> d) I decided to give up a just add a conditional CSS block that hides the
> empty background of the side bar.

This seems to work well.

>
> 3. I then saw that the Project milestone table is still very compressed
> because it has a "Product" column. Well that is not right. It should
> read "Project"
>
> 4. The I experimented with a new class that was on a different z-index and
> very wide. The bug and spec work items filled the space very well.

Great.

>
> 5. I decided to a .wide to the release files portlets so that its description
> was not compressed.

Yes, I agree it looks better.

>
>
> == QA ==
>
> The side bar and wide tables show look like these images.
> http://people.canonical.com/~curtis/milestones/
>
>
> == Lint ==
>
> Linting changed files:
> lib/canonical/launchpad/icing/style-3-0.css
> lib/lp/registry/browser/tests/milestone-views.txt
> lib/lp/registry/stories/milestone/object-milestones.txt
> lib/lp/registry/templates/milestone-index.pt
>
>
> == Test ==
>
> * lib/lp/registry/browser/tests/milestone-views.txt
> * Added a test to verify that CSS is added to hide the project group
> milestone side bar.
> * lib/lp/registry/stories/milestone/object-milestones.txt
> ...

> This is my branch to fix timeout issues and IProject issues with the
> milestone page and improve the layout.
>

Hi Curtis, thanks for all the details here - and for finding a solution
to a common problem!

r=me (code/ui*)

I've got some questions below, but I'll leave it up to you whether they
are worth considering.

>     lp:~sinzui/launchpad/milestone-design-oops
>     Diff size: 155
>     Launchpad bug: https://bugs.launchpad.net/bugs/430726
>     Test command: ./bin/test -vv -t "milestone-views|object-milestones"
>     Pre-implementation: wgrant
>     Target release: 3.1.0

Did I miss 3.0?

> 
> 
> = Fix timeout issues and IProject issues with the milestone page =
> 
> There is a content and a design problem with the milestone page.
> 
> 1. There are many more timeouts because of the cost of showing the bug tags.
>     Remove them, they do not work well are they are in the design.
> 
> 2. For most users, the side bar has one link, and if the context is an
>    IProject, there is no side bar. The content does not fill the page well.
>    Remove the side bar from the IProject case, consider moving the three
>    action links inline or float a fake side bar so that the content can fill
>    the page.
> 
> 
> == Rules ==
> 
> 1. Removed the tags from the page.

Great.

> 
> 2. This is hard to do. I had several failures before settling on the CSS
>    approach.
>    a) I could not conditional set the page macro

I haven't need to do this but, if it's helpful, I found an example at:

http://docs.zope.org/zope2/zope2book/source/ZPT.html#macro-details

I assume this should work the same for us?

>    b) I could not reliably make a mock side bar in a main_only template --
>       If someone changed the yui-* definition in the CSS, the template would
>       be out of sync.

Yep, good catch.

>    c) I tried using CSS to collapse the yui-* widths to reclaim the space,
>       but Firefox ignored them
>    d) I decided to give up a just add a conditional CSS block that hides the
>       empty background of the side bar.

This seems to work well.

> 
> 3. I then saw that the Project milestone table is still very compressed
>    because it has a "Product" column. Well that is not right. It should
>    read "Project"
> 
> 4. The I experimented with a new class that was on a different z-index and
>    very wide. The bug and spec work items filled the space very well.

Great.

> 
> 5. I decided to a .wide to the release files portlets so that its description
>    was not compressed.

Yes, I agree it looks better.

> 
> 
> == QA ==
> 
> The side bar and wide tables show look like these images.
>     http://people.canonical.com/~curtis/milestones/
> 
> 
> == Lint ==
> 
> Linting changed files:
>   lib/canonical/launchpad/icing/style-3-0.css
>   lib/lp/registry/browser/tests/milestone-views.txt
>   lib/lp/registry/stories/milestone/object-milestones.txt
>   lib/lp/registry/templates/milestone-index.pt
> 
> 
> == Test ==
> 
>     * lib/lp/registry/browser/tests/milestone-views.txt
>       * Added a test to verify that CSS is added to hide the project group
>         milestone side bar.
>     * lib/lp/registry/stories/milestone/object-milestones.txt
>       * Updated the test to verify that the UI uses "Project", not "Product".
> 
> 
> == Implementation ==
> 
>     * lib/canonical/launchpad/icing/style-3-0.css
>       * Added a wide rule for content to reclaim the space lost to the
>         side bar.
>     * lib/lp/registry/templates/milestone-index.pt
>       * Added CSS for the is_project_milestone condition
>       * Added .wide to the product release files portlet and the bug/blueprint
>         work items portlet
>       * Removed the bug tags that were cause a zillion queries.

Heh.

> === modified file 'lib/canonical/launchpad/icing/style-3-0.css'
> --- lib/canonical/launchpad/icing/style-3-0.css	2009-09-14 10:13:55 +0000
> +++ lib/canonical/launchpad/icing/style-3-0.css	2009-09-17 07:26:10 +0000
> @@ -237,7 +237,10 @@
>      padding: 0 0 0.5em 0;
>      margin: 0 0 1em;
>      }
> -
> +.wide {
> +    z-index: 10;
> +    width: 131%;
> +    }

I wonder if we should give this a more meaningful name, perhaps something
like 'full-width-main-side'? Maybe that' too verbose. See what you think.

>  .warning.message {
>      margin-top: 17px;
>      }
> 
> === modified file 'lib/lp/registry/browser/tests/milestone-views.txt'
> --- lib/lp/registry/browser/tests/milestone-views.txt	2009-09-11 20:38:21 +0000
> +++ lib/lp/registry/browser/tests/milestone-views.txt	2009-09-17 07:26:10 +0000
> @@ -259,6 +259,29 @@
>  
>      >>> login_person(person)
>  
> +
> +Project milestones
> +------------------
> +
> +The projectgroup milestones are virtual and cannot be modified. The template
> +generates CSS that hides the space occupied by the side portlets.
> +
> +    >>> projectgroup = factory.makeProject(name='flock')
> +    >>> product.project = projectgroup
> +    >>> project_milestone = projectgroup.getMilestone('kakapo')
> +    >>> view = create_initialized_view(
> +    ...     project_milestone, '+index', principal=person)
> +    >>> print find_tag_by_id(view.render(), 'hide-side-portlets')['type']
> +    text/css
> +
> +A normal milestone does not have the CSS rule.
> +
> +    >>> view = create_initialized_view(
> +    ...     milestone, '+index', principal=person)
> +    >>> print find_tag_by_id(content, 'hide-side-portlets')
> +    None

The test is fine - I've got a more general question/discussion point
that I'd be keen to chat with you about some other time:

I remember you mentioning this way of testing the template as part of
the view test - and I can understand that it makes the stories much easier
to read by removing the unnecessary detail, but I'm not convinced that
the view test is the place for it. I've always assumed the dependencies
are:

model -> view -> template

That is, if I update a model, all tests could fail. If I update a view,
the model test cannot fail, but the view and template tests might. If I
update a template, the model and view tests should not fail, but the
template tests might.

I think what I'm trying to get at is, perhaps we should have some separate
template tests that do exactly what you've done here (or alternatively as
a unittest)? That way the stories stay clean, but the view tests would
remain focused on view logic?

> +
> +
>  Editing milestones
>  ------------------
>  
> 
> === modified file 'lib/lp/registry/stories/milestone/object-milestones.txt'
> --- lib/lp/registry/stories/milestone/object-milestones.txt	2009-09-14 15:03:45 +0000
> +++ lib/lp/registry/stories/milestone/object-milestones.txt	2009-09-17 07:26:10 +0000
> @@ -122,13 +122,13 @@
>  
>      >>> specs = find_tag_by_id(anon_browser.contents, 'milestone_specs')
>      >>> print extract_text(specs)
> -    Blueprint Product Priority Assignee Delivery
> +    Blueprint Project Priority Assignee Delivery
>      Title evolution specification   Evolution  High  Unknown
>      Title gnomebaker specification  gnomebaker High  Unknown
>  
>      >>> bugtasks = find_tag_by_id(anon_browser.contents, 'milestone_bugtasks')
>      >>> print extract_text(bugtasks)
> -    Bug report Product Importance Assignee Status ...
> +    Bug report Project Importance Assignee Status ...
>      Milestone test bug for evolution  Evolution  Undecided Confirmed ...
>      Milestone test bug for gnomebaker gnomebaker Undecided Confirmed ...
>      Milestone test bug for evolution series trunk Undecided Confirmed
> 
> === modified file 'lib/lp/registry/templates/milestone-index.pt'
> --- lib/lp/registry/templates/milestone-index.pt	2009-09-14 19:28:26 +0000
> +++ lib/lp/registry/templates/milestone-index.pt	2009-09-17 07:26:10 +0000
> @@ -6,6 +6,15 @@
>    metal:use-macro="view/macro:page/main_side"
>    i18n:domain="launchpad">
>  
> +  <tal:css metal:fill-slot="head_epilogue"
> +    condition="view/is_project_milestone">
> +    <style id="hide-side-portlets" type="text/css">
> +        .side {
> +            background: #fff;
> +            }
> +    </style>
> +  </tal:css>

The link above to the zope page about conditional macros *may* help - but
otherwise, this is fine.

> +
>    <body>
>      <tal:heading metal:fill-slot="heading">
>        <h1 tal:content="view/milestone/title" >project 1.x series</h1>
> @@ -183,11 +192,11 @@
>          </div>
>        </div>
>  
> -    <tal:release
> -      condition="view/release"
> -      replace="structure view/milestone/@@+productrelease-data" />
> +    <div class="wide"
> +      tal:condition="view/release"
> +      tal:content="structure view/milestone/@@+productrelease-data" />
>  
> -    <div class="portlet">
> +    <div class="portlet wide">
>          <h2>
>            <span id="specification-count" style="color: #3594bb;"
>              tal:content="structure view/specification_count_text">2</span>
> @@ -205,7 +214,7 @@
>                  <th colspan="2">Blueprint</th>
>                  <th style="width: 14em"
>                    tal:condition="view/is_project_milestone">
> -                  Product
> +                  Project
>                  </th>
>                  <th style="width: 9em;">Priority</th>
>                  <th style="width: 14em;">Assignee</th>
> @@ -261,7 +270,7 @@
>                  <th colspan="4">Bug report</th>
>                  <th style="width: 14em"
>                    tal:condition="view/is_project_milestone">
> -                  Product
> +                  Project
>                  </th>
>                  <th style="width: 9em;">Importance</th>
>                  <th style="width: 14em;">Assignee</th>
> @@ -280,20 +289,6 @@
>                       tal:content="bugtask/bug/title/fmt:shorten/65">
>                       Bug Title Here
>                    </a>
> -                  <tal:has-tags
> -                    condition="bugtask/bug/tags|nothing">
> -                    <br />
> -                    <span id="tag-list">
> -                      <a tal:repeat="tag bugtask/@@+index/official_tags"
> -                         tal:content="tag"
> -                         class="official-tag"
> -                         tal:attributes="href string:${context/target/fmt:url:bugs}/+bugs?field.tag=${tag}">tag</a>
> -                      <a tal:repeat="tag bugtask/@@+index/unofficial_tags"
> -                         tal:content="tag"
> -                         class="unofficial-tag"
> -                         tal:attributes="href string:${context/target/fmt:url:bugs}/+bugs?field.tag=${tag}">tag</a>
> -                    </span>
> -                  </tal:has-tags>
>                  </td>
>                  <td style="padding-right: 5px; text-align:right">
>                    <img src="/@@/info" tal:replace="structure bugtask/image:badges" />
> @@ -355,8 +350,7 @@
>        </div>
>      </div>
>  
> -    <tal:side metal:fill-slot="side"
> -      define="overview_menu context/menu:overview">
> +    <tal:side metal:fill-slot="side">
>        <tal:menu replace="structure view/milestone/@@+global-actions" />
>      </tal:side>
>    </body>

Great! Thanks Curtis.

review: Approve (code ui*)

Revision history for this message

Curtis Hovey (sinzui) wrote on 2009-09-17:

#

Download full text (8.4 KiB)

On Thu, 2009-09-17 at 08:30 +0000, Michael Nelson wrote:
> Review: Approve code ui*
> > This is my branch to fix timeout issues and IProject issues with the
> > milestone page and improve the layout.
> >
>
> Hi Curtis, thanks for all the details here - and for finding a solution
> to a common problem!
>
> r=me (code/ui*)
>
> I've got some questions below, but I'll leave it up to you whether they
> are worth considering.
>
> > lp:~sinzui/launchpad/milestone-design-oops
> > Diff size: 155
> > Launchpad bug: https://bugs.launchpad.net/bugs/430726
> > Test command: ./bin/test -vv -t "milestone-views|object-milestones"
> > Pre-implementation: wgrant
> > Target release: 3.1.0
>
> Did I miss 3.0?

3.0 is mis named. The series is 3.1.

> >
> >
> > = Fix timeout issues and IProject issues with the milestone page =
> >
> > There is a content and a design problem with the milestone page.
> >
> > 1. There are many more timeouts because of the cost of showing the bug tags.
> > Remove them, they do not work well are they are in the design.
> >
> > 2. For most users, the side bar has one link, and if the context is an
> > IProject, there is no side bar. The content does not fill the page well.
> > Remove the side bar from the IProject case, consider moving the three
> > action links inline or float a fake side bar so that the content can fill
> > the page.
> >
> >
> > == Rules ==
> >
> > 1. Removed the tags from the page.
>
> Great.
>
> >
> > 2. This is hard to do. I had several failures before settling on the CSS
> > approach.
> > a) I could not conditional set the page macro
>
> I haven't need to do this but, if it's helpful, I found an example at:
>
> http://docs.zope.org/zope2/zope2book/source/ZPT.html#macro-details
>
> I assume this should work the same for us?

I am very familiar with metal and tal. The problem is the we are *way*
to clever for our own good. There is some crazy abstraction going on
when we call macro: that ensures we call the whole macro...getting part
of the object is a no-no. I thought I could create two <html> nodes for
each condition:

But once we call the first one, the state is set.

The object state cannot be monkeyed with because we are talking to a
cached proxy that guards access to the real object. So I cannot get the
the object in python and pass it as a variable.

I think we could dismantle the insanity when we tear out the 1.0 and 2.0
support from the macro.

> > b) I could not reliably make a mock side bar in a main_only template --
> > If someone changed the yui-* definition in the CSS, the template would
> > be out of sync.
>
> Yep, good catch....

On Thu, 2009-09-17 at 08:30 +0000, Michael Nelson wrote:
> Review: Approve code ui*
> > This is my branch to fix timeout issues and IProject issues with the
> > milestone page and improve the layout.
> > 
> 
> Hi Curtis, thanks for all the details here - and for finding a solution
> to a common problem!
> 
> r=me (code/ui*)
> 
> I've got some questions below, but I'll leave it up to you whether they
> are worth considering.
> 
> >     lp:~sinzui/launchpad/milestone-design-oops
> >     Diff size: 155
> >     Launchpad bug: https://bugs.launchpad.net/bugs/430726
> >     Test command: ./bin/test -vv -t "milestone-views|object-milestones"
> >     Pre-implementation: wgrant
> >     Target release: 3.1.0
> 
> Did I miss 3.0?

3.0 is mis named. The series is 3.1.

> > 
> > 
> > = Fix timeout issues and IProject issues with the milestone page =
> > 
> > There is a content and a design problem with the milestone page.
> > 
> > 1. There are many more timeouts because of the cost of showing the bug tags.
> >     Remove them, they do not work well are they are in the design.
> > 
> > 2. For most users, the side bar has one link, and if the context is an
> >    IProject, there is no side bar. The content does not fill the page well.
> >    Remove the side bar from the IProject case, consider moving the three
> >    action links inline or float a fake side bar so that the content can fill
> >    the page.
> > 
> > 
> > == Rules ==
> > 
> > 1. Removed the tags from the page.
> 
> Great.
> 
> > 
> > 2. This is hard to do. I had several failures before settling on the CSS
> >    approach.
> >    a) I could not conditional set the page macro
> 
> I haven't need to do this but, if it's helpful, I found an example at:
> 
> http://docs.zope.org/zope2/zope2book/source/ZPT.html#macro-details
> 
> I assume this should work the same for us?

I am very familiar with metal and tal. The problem is the we are *way*
to clever for our own good. There is some crazy abstraction going on
when we call macro: that ensures we call the whole macro...getting part
of the object is a no-no. I thought I could create two <html> nodes for
each condition:

But once we call the first one, the state is set.

The object state cannot be monkeyed with because we are talking to a
cached proxy that guards access to the real object. So I cannot get the
the object in python and pass it as a variable.

I think we could dismantle the insanity when we tear out the 1.0 and 2.0
support from the macro.

> >    b) I could not reliably make a mock side bar in a main_only template --
> >       If someone changed the yui-* definition in the CSS, the template would
> >       be out of sync.
> 
> Yep, good catch.
> 
> >    c) I tried using CSS to collapse the yui-* widths to reclaim the space,
> >       but Firefox ignored them
> >    d) I decided to give up a just add a conditional CSS block that hides the
> >       empty background of the side bar.
> 
> This seems to work well.
> 
> > 
> > 3. I then saw that the Project milestone table is still very compressed
> >    because it has a "Product" column. Well that is not right. It should
> >    read "Project"
> > 
> > 4. The I experimented with a new class that was on a different z-index and
> >    very wide. The bug and spec work items filled the space very well.
> 
> Great.
> 
> > 
> > 5. I decided to a .wide to the release files portlets so that its description
> >    was not compressed.
> 
> Yes, I agree it looks better.
> 
> > 
> > 
> > == QA ==
> > 
> > The side bar and wide tables show look like these images.
> >     http://people.canonical.com/~curtis/milestones/
> > 
> > 
> > == Lint ==
> > 
> > Linting changed files:
> >   lib/canonical/launchpad/icing/style-3-0.css
> >   lib/lp/registry/browser/tests/milestone-views.txt
> >   lib/lp/registry/stories/milestone/object-milestones.txt
> >   lib/lp/registry/templates/milestone-index.pt
> > 
> > 
> > == Test ==
> > 
> >     * lib/lp/registry/browser/tests/milestone-views.txt
> >       * Added a test to verify that CSS is added to hide the project group
> >         milestone side bar.
> >     * lib/lp/registry/stories/milestone/object-milestones.txt
> >       * Updated the test to verify that the UI uses "Project", not "Product".
> > 
> > 
> > == Implementation ==
> > 
> >     * lib/canonical/launchpad/icing/style-3-0.css
> >       * Added a wide rule for content to reclaim the space lost to the
> >         side bar.
> >     * lib/lp/registry/templates/milestone-index.pt
> >       * Added CSS for the is_project_milestone condition
> >       * Added .wide to the product release files portlet and the bug/blueprint
> >         work items portlet
> >       * Removed the bug tags that were cause a zillion queries.
> 
> Heh.
> 
> > === modified file 'lib/canonical/launchpad/icing/style-3-0.css'
> > --- lib/canonical/launchpad/icing/style-3-0.css	2009-09-14 10:13:55 +0000
> > +++ lib/canonical/launchpad/icing/style-3-0.css	2009-09-17 07:26:10 +0000
> > @@ -237,7 +237,10 @@
> >      padding: 0 0 0.5em 0;
> >      margin: 0 0 1em;
> >      }
> > -
> > +.wide {
> > +    z-index: 10;
> > +    width: 131%;
> > +    }
> 
> I wonder if we should give this a more meaningful name, perhaps something
> like 'full-width-main-side'? Maybe that' too verbose. See what you think.

Maybe. I do not link wide, but I also want to avoid a meaningless name
in 1 year when we change the design again. I'll use full-page-width.

...

> > === modified file 'lib/lp/registry/browser/tests/milestone-views.txt'
> > --- lib/lp/registry/browser/tests/milestone-views.txt	2009-09-11 20:38:21 +0000
> > +++ lib/lp/registry/browser/tests/milestone-views.txt	2009-09-17 07:26:10 +0000
> > @@ -259,6 +259,29 @@
> >  
> >      >>> login_person(person)
> >  
> > +
> > +Project milestones
> > +------------------
> > +
> > +The projectgroup milestones are virtual and cannot be modified. The template
> > +generates CSS that hides the space occupied by the side portlets.
> > +
> > +    >>> projectgroup = factory.makeProject(name='flock')
> > +    >>> product.project = projectgroup
> > +    >>> project_milestone = projectgroup.getMilestone('kakapo')
> > +    >>> view = create_initialized_view(
> > +    ...     project_milestone, '+index', principal=person)
> > +    >>> print find_tag_by_id(view.render(), 'hide-side-portlets')['type']
> > +    text/css
> > +
> > +A normal milestone does not have the CSS rule.
> > +
> > +    >>> view = create_initialized_view(
> > +    ...     milestone, '+index', principal=person)
> > +    >>> print find_tag_by_id(content, 'hide-side-portlets')
> > +    None
> 
> The test is fine - I've got a more general question/discussion point
> that I'd be keen to chat with you about some other time:
> 
> I remember you mentioning this way of testing the template as part of
> the view test - and I can understand that it makes the stories much easier
> to read by removing the unnecessary detail, but I'm not convinced that
> the view test is the place for it. I've always assumed the dependencies
> are:
> 
> model -> view -> template
> 
> That is, if I update a model, all tests could fail. If I update a view,
> the model test cannot fail, but the view and template tests might. If I
> update a template, the model and view tests should not fail, but the
> template tests might.
> 
> I think what I'm trying to get at is, perhaps we should have some separate
> template tests that do exactly what you've done here (or alternatively as
> a unittest)? That way the stories stay clean, but the view tests would
> remain focused on view logic?

I understand you point. The problem is that stories are not the place
for testing conditional logic. They only verify a user can perform a
basic action moving from one page to the next. The user does not care
how we make the side bar appear or disappear.

We could have a separate file to test that the template responds the of
the view and context state. There are some in fact because the test of
the view was getting large.

--

__C U R T I S  C.  H O V E Y_______
Guilty of stealing everything I am.

Launchpad itself

Merge lp:~sinzui/launchpad/milestone-design-oops into lp:launchpad

Commit message

Description of the change

Preview Diff

Subscribers

 === modified file 'lib/canonical/launchpad/icing/style-3-0.css'
 --- lib/canonical/launchpad/icing/style-3-0.css	2009-09-14 10:13:55 +0000
 +++ lib/canonical/launchpad/icing/style-3-0.css	2009-09-17 03:28:21 +0000
@@ -237,7 +237,10 @@
      padding: 0 0 0.5em 0;
      margin: 0 0 1em;
+     }
--
++.wide {
++    z-index: 10;
++    width: 131%;
++    }
  .warning.message {
      margin-top: 17px;
+     }
 === modified file 'lib/lp/registry/browser/tests/milestone-views.txt'
 --- lib/lp/registry/browser/tests/milestone-views.txt	2009-09-11 20:38:21 +0000
 +++ lib/lp/registry/browser/tests/milestone-views.txt	2009-09-17 02:38:00 +0000
@@ -259,6 +259,29 @@
      >>> login_person(person)
++
++Project milestones
++------------------
++
++The projectgroup milestones are virtual and cannot be modified. The template
++generates CSS that hides the space occupied by the side portlets.
++
++    >>> projectgroup = factory.makeProject(name='flock')
++    >>> product.project = projectgroup
++    >>> project_milestone = projectgroup.getMilestone('kakapo')
++    >>> view = create_initialized_view(
++    ...     project_milestone, '+index', principal=person)
++    >>> print find_tag_by_id(view.render(), 'hide-side-portlets')['type']
++    text/css
++
++A normal milestone does not have the CSS rule.
++
++    >>> view = create_initialized_view(
++    ...     milestone, '+index', principal=person)
++    >>> print find_tag_by_id(content, 'hide-side-portlets')
++    None
++
++
  Editing milestones
  ------------------
 === modified file 'lib/lp/registry/stories/milestone/object-milestones.txt'
 --- lib/lp/registry/stories/milestone/object-milestones.txt	2009-09-14 15:03:45 +0000
 +++ lib/lp/registry/stories/milestone/object-milestones.txt	2009-09-17 03:54:06 +0000
@@ -122,13 +122,13 @@
      >>> specs = find_tag_by_id(anon_browser.contents, 'milestone_specs')
      >>> print extract_text(specs)
--    Blueprint Product Priority Assignee Delivery
++    Blueprint Project Priority Assignee Delivery
      Title evolution specification   Evolution  High  Unknown
      Title gnomebaker specification  gnomebaker High  Unknown
      >>> bugtasks = find_tag_by_id(anon_browser.contents, 'milestone_bugtasks')
      >>> print extract_text(bugtasks)
--    Bug report Product Importance Assignee Status ...
++    Bug report Project Importance Assignee Status ...
      Milestone test bug for evolution  Evolution  Undecided Confirmed ...
      Milestone test bug for gnomebaker gnomebaker Undecided Confirmed ...
      Milestone test bug for evolution series trunk Undecided Confirmed
 === modified file 'lib/lp/registry/templates/milestone-index.pt'
 --- lib/lp/registry/templates/milestone-index.pt	2009-09-14 19:28:26 +0000
 +++ lib/lp/registry/templates/milestone-index.pt	2009-09-17 03:54:06 +0000
@@ -6,6 +6,15 @@
    metal:use-macro="view/macro:page/main_side"
    i18n:domain="launchpad">
++  <tal:css metal:fill-slot="head_epilogue"
++    condition="view/is_project_milestone">
++    <style id="hide-side-portlets" type="text/css">
++        .side {
++            background: #fff;
++            }
++    </style>
++  </tal:css>
++
    <body>
      <tal:heading metal:fill-slot="heading">
        <h1 tal:content="view/milestone/title" >project 1.x series</h1>
@@ -183,11 +192,11 @@
          </div>
        </div>
--    <tal:release
--      condition="view/release"
--      replace="structure view/milestone/@@+productrelease-data" />
++    <div class="wide"
++      tal:condition="view/release"
++      tal:content="structure view/milestone/@@+productrelease-data" />
--    <div class="portlet">
++    <div class="portlet wide">
          <h2>
            <span id="specification-count" style="color: #3594bb;"
              tal:content="structure view/specification_count_text">2</span>
@@ -205,7 +214,7 @@
                  <th colspan="2">Blueprint</th>
                  <th style="width: 14em"
                    tal:condition="view/is_project_milestone">
--                  Product
++                  Project
                  </th>
                  <th style="width: 9em;">Priority</th>
                  <th style="width: 14em;">Assignee</th>
@@ -261,7 +270,7 @@
                  <th colspan="4">Bug report</th>
                  <th style="width: 14em"
                    tal:condition="view/is_project_milestone">
--                  Product
++                  Project
                  </th>
                  <th style="width: 9em;">Importance</th>
                  <th style="width: 14em;">Assignee</th>
@@ -280,20 +289,6 @@
                       tal:content="bugtask/bug/title/fmt:shorten/65">
                       Bug Title Here
                    </a>
--                  <tal:has-tags
--                    condition="bugtask/bug/tags|nothing">
--                    <br />
--                    <span id="tag-list">
--                      <a tal:repeat="tag bugtask/@@+index/official_tags"
--                         tal:content="tag"
--                         class="official-tag"
--                         tal:attributes="href string:${context/target/fmt:url:bugs}/+bugs?field.tag=${tag}">tag</a>
--                      <a tal:repeat="tag bugtask/@@+index/unofficial_tags"
--                         tal:content="tag"
--                         class="unofficial-tag"
--                         tal:attributes="href string:${context/target/fmt:url:bugs}/+bugs?field.tag=${tag}">tag</a>
--                    </span>
--                  </tal:has-tags>
                  </td>
                  <td style="padding-right: 5px; text-align:right">
                    <img src="/@@/info" tal:replace="structure bugtask/image:badges" />
@@ -355,8 +350,7 @@
        </div>
      </div>
--    <tal:side metal:fill-slot="side"
--      define="overview_menu context/menu:overview">
++    <tal:side metal:fill-slot="side">
        <tal:menu replace="structure view/milestone/@@+global-actions" />
      </tal:side>
    </body>