Launchpad itself

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

  1. milestone-design-oops
  2. Merge into devel
Proposed by Curtis Hovey
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
Reviewer Review Type Date Requested Status
Michael Nelson (community) code ui* Approve
Review via email: mp+11946@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Curtis Hovey (sinzui) wrote :

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 :
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
> ...

review: Approve (code ui*)
Revision history for this message
Curtis Hovey (sinzui) wrote :
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:

        <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"
          tal:omit-tag="not: view/is_project_milestone"
          metal:use-macro="view/macro:page/main_side"
          i18n:domain="launchpad">
        <html
          tal:omit-tag="view/is_project_milestone"
          metal:use-macro="view/macro:page/main_only">

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....

Read more...

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== 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 @@
237 padding: 0 0 0.5em 0;237 padding: 0 0 0.5em 0;
238 margin: 0 0 1em;238 margin: 0 0 1em;
239 }239 }
240240.wide {
241 z-index: 10;
242 width: 131%;
243 }
241.warning.message {244.warning.message {
242 margin-top: 17px;245 margin-top: 17px;
243 }246 }
244247
=== 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 @@
259259
260 >>> login_person(person)260 >>> login_person(person)
261261
262
263Project milestones
264------------------
265
266The projectgroup milestones are virtual and cannot be modified. The template
267generates CSS that hides the space occupied by the side portlets.
268
269 >>> projectgroup = factory.makeProject(name='flock')
270 >>> product.project = projectgroup
271 >>> project_milestone = projectgroup.getMilestone('kakapo')
272 >>> view = create_initialized_view(
273 ... project_milestone, '+index', principal=person)
274 >>> print find_tag_by_id(view.render(), 'hide-side-portlets')['type']
275 text/css
276
277A normal milestone does not have the CSS rule.
278
279 >>> view = create_initialized_view(
280 ... milestone, '+index', principal=person)
281 >>> print find_tag_by_id(content, 'hide-side-portlets')
282 None
283
284
262Editing milestones285Editing milestones
263------------------286------------------
264287
265288
=== 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 @@
122122
123 >>> specs = find_tag_by_id(anon_browser.contents, 'milestone_specs')123 >>> specs = find_tag_by_id(anon_browser.contents, 'milestone_specs')
124 >>> print extract_text(specs)124 >>> print extract_text(specs)
125 Blueprint Product Priority Assignee Delivery125 Blueprint Project Priority Assignee Delivery
126 Title evolution specification Evolution High Unknown126 Title evolution specification Evolution High Unknown
127 Title gnomebaker specification gnomebaker High Unknown127 Title gnomebaker specification gnomebaker High Unknown
128128
129 >>> bugtasks = find_tag_by_id(anon_browser.contents, 'milestone_bugtasks')129 >>> bugtasks = find_tag_by_id(anon_browser.contents, 'milestone_bugtasks')
130 >>> print extract_text(bugtasks)130 >>> print extract_text(bugtasks)
131 Bug report Product Importance Assignee Status ...131 Bug report Project Importance Assignee Status ...
132 Milestone test bug for evolution Evolution Undecided Confirmed ...132 Milestone test bug for evolution Evolution Undecided Confirmed ...
133 Milestone test bug for gnomebaker gnomebaker Undecided Confirmed ...133 Milestone test bug for gnomebaker gnomebaker Undecided Confirmed ...
134 Milestone test bug for evolution series trunk Undecided Confirmed134 Milestone test bug for evolution series trunk Undecided Confirmed
135135
=== 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 @@
6 metal:use-macro="view/macro:page/main_side"6 metal:use-macro="view/macro:page/main_side"
7 i18n:domain="launchpad">7 i18n:domain="launchpad">
88
9 <tal:css metal:fill-slot="head_epilogue"
10 condition="view/is_project_milestone">
11 <style id="hide-side-portlets" type="text/css">
12 .side {
13 background: #fff;
14 }
15 </style>
16 </tal:css>
17
9 <body>18 <body>
10 <tal:heading metal:fill-slot="heading">19 <tal:heading metal:fill-slot="heading">
11 <h1 tal:content="view/milestone/title" >project 1.x series</h1>20 <h1 tal:content="view/milestone/title" >project 1.x series</h1>
@@ -183,11 +192,11 @@
183 </div>192 </div>
184 </div>193 </div>
185194
186 <tal:release195 <div class="wide"
187 condition="view/release"196 tal:condition="view/release"
188 replace="structure view/milestone/@@+productrelease-data" />197 tal:content="structure view/milestone/@@+productrelease-data" />
189198
190 <div class="portlet">199 <div class="portlet wide">
191 <h2>200 <h2>
192 <span id="specification-count" style="color: #3594bb;"201 <span id="specification-count" style="color: #3594bb;"
193 tal:content="structure view/specification_count_text">2</span>202 tal:content="structure view/specification_count_text">2</span>
@@ -205,7 +214,7 @@
205 <th colspan="2">Blueprint</th>214 <th colspan="2">Blueprint</th>
206 <th style="width: 14em"215 <th style="width: 14em"
207 tal:condition="view/is_project_milestone">216 tal:condition="view/is_project_milestone">
208 Product217 Project
209 </th>218 </th>
210 <th style="width: 9em;">Priority</th>219 <th style="width: 9em;">Priority</th>
211 <th style="width: 14em;">Assignee</th>220 <th style="width: 14em;">Assignee</th>
@@ -261,7 +270,7 @@
261 <th colspan="4">Bug report</th>270 <th colspan="4">Bug report</th>
262 <th style="width: 14em"271 <th style="width: 14em"
263 tal:condition="view/is_project_milestone">272 tal:condition="view/is_project_milestone">
264 Product273 Project
265 </th>274 </th>
266 <th style="width: 9em;">Importance</th>275 <th style="width: 9em;">Importance</th>
267 <th style="width: 14em;">Assignee</th>276 <th style="width: 14em;">Assignee</th>
@@ -280,20 +289,6 @@
280 tal:content="bugtask/bug/title/fmt:shorten/65">289 tal:content="bugtask/bug/title/fmt:shorten/65">
281 Bug Title Here290 Bug Title Here
282 </a>291 </a>
283 <tal:has-tags
284 condition="bugtask/bug/tags|nothing">
285 <br />
286 <span id="tag-list">
287 <a tal:repeat="tag bugtask/@@+index/official_tags"
288 tal:content="tag"
289 class="official-tag"
290 tal:attributes="href string:${context/target/fmt:url:bugs}/+bugs?field.tag=${tag}">tag</a>
291 <a tal:repeat="tag bugtask/@@+index/unofficial_tags"
292 tal:content="tag"
293 class="unofficial-tag"
294 tal:attributes="href string:${context/target/fmt:url:bugs}/+bugs?field.tag=${tag}">tag</a>
295 </span>
296 </tal:has-tags>
297 </td>292 </td>
298 <td style="padding-right: 5px; text-align:right">293 <td style="padding-right: 5px; text-align:right">
299 <img src="/@@/info" tal:replace="structure bugtask/image:badges" />294 <img src="/@@/info" tal:replace="structure bugtask/image:badges" />
@@ -355,8 +350,7 @@
355 </div>350 </div>
356 </div>351 </div>
357352
358 <tal:side metal:fill-slot="side"353 <tal:side metal:fill-slot="side">
359 define="overview_menu context/menu:overview">
360 <tal:menu replace="structure view/milestone/@@+global-actions" />354 <tal:menu replace="structure view/milestone/@@+global-actions" />
361 </tal:side>355 </tal:side>
362 </body>356 </body>
363357