Launchpad itself

Merge lp:~sinzui/launchpad/full-page-width into lp:launchpad/db-devel

  1. full-page-width
  2. Merge into db-devel
Proposed by Curtis Hovey
Status: Merged
Merged at revision: not available
Proposed branch: lp:~sinzui/launchpad/full-page-width
Merge into: lp:launchpad/db-devel
Diff against target: 36 lines
2 files modified
lib/canonical/launchpad/icing/style-3-0.css (+4/-0)
lib/lp/registry/templates/productrelease-portlet-data.pt (+2/-2)
To merge this branch: bzr merge lp:~sinzui/launchpad/full-page-width
Reviewer Review Type Date Requested Status
Francis J. Lacoste (community) release-critical code Approve
Michael Nelson Pending
Canonical Launchpad Engineering code Pending
Review via email: mp+14427@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 the milestone page width problem.

    lp:~sinzui/launchpad/full-page-width
    Diff size: 36
    Launchpad bug: https://bugs.launchpad.net/bugs/473738
    Test command: ./bin/test -vvt "stories/(milestone|productrelease)"
    Pre-implementation: No one
    Target release: 3.1.10

= Fix the milestone page width problem =

bug 473738 [milestone content exceeds the browser viewport]
    There seems to be an odd resizing bug on the +milestone page.
    The bugs and blueprints tables scroll to the right, even after
    the browser is maximised.

== Rules ==

This problem is only visible when there is a release. The release data
portlet is creating bad markup.

The <div style="clear:both" /> are well formed but invalid HTML. FF saw two
open divs, so it rendered the side portlets below the main content. This then
exposed a flaw in nested .full-page-width that caused the content to exceed
the browser viewport. Fixing the markup is all that is required, but added an
additional rule for nested .full-page-width is the right thing to do since the
content should never exceed the view port.

== QA ==

Before: http://people.canonical.com/~curtis/broken-page-width.png
    The side porlet is below the main content; the bug is wider than the page.

After: http://people.canonical.com/~curtis/fixed-page-width.png
    The side portlet is to the side. the bug fits in the page width.

== Lint ==

Linting changed files:
  lib/canonical/launchpad/icing/style-3-0.css
  lib/lp/registry/templates/productrelease-portlet-data.pt

== Test ==

No tests changed. The changes are CSS and layout markup

== Implementation ==

    * lib/canonical/launchpad/icing/style-3-0.css
      * Added a rule to ensure nested .full-page-width classes do not exceed
        the browser view port.
    * lib/lp/registry/templates/productrelease-portlet-data.pt
      * Replace the invalid markup with valid HTML markup.

Revision history for this message
Francis J. Lacoste (flacoste) wrote :

Please add a comment above the CSS class pointing at a template that should be use if this rule is modified/dropped.

Otherwise, that's fine.

review: Approve (release-critical code)

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-11-03 16:36:15 +0000
+++ lib/canonical/launchpad/icing/style-3-0.css 2009-11-04 17:42:13 +0000
@@ -250,6 +250,10 @@
250 z-index: 10;250 z-index: 10;
251 width: 131%;251 width: 131%;
252 }252 }
253/* The content is already full width; the content should remain full width. */
254.full-page-width .full-page-width {
255 width: 100%;
256 }
253.warning.message {257.warning.message {
254 margin-top: 17px;258 margin-top: 17px;
255 }259 }
256260
=== modified file 'lib/lp/registry/templates/productrelease-portlet-data.pt'
--- lib/lp/registry/templates/productrelease-portlet-data.pt 2009-10-22 13:35:25 +0000
+++ lib/lp/registry/templates/productrelease-portlet-data.pt 2009-11-04 17:42:13 +0000
@@ -69,7 +69,7 @@
69 <div class="top-portlet">69 <div class="top-portlet">
70 <h2 style="float:left">Release notes &nbsp;</h2>70 <h2 style="float:left">Release notes &nbsp;</h2>
71 <a tal:replace="structure view/release/menu:context/edit/fmt:icon" />71 <a tal:replace="structure view/release/menu:context/edit/fmt:icon" />
72 <div style="clear:both" />72 <div style="clear:both"></div>
7373
74 <div id="release-notes"74 <div id="release-notes"
75 tal:content="structure view/release/release_notes/fmt:text-to-html"75 tal:content="structure view/release/release_notes/fmt:text-to-html"
@@ -83,7 +83,7 @@
8383
84 <h2 style="float:left">Changelog &nbsp;</h2>84 <h2 style="float:left">Changelog &nbsp;</h2>
85 <a tal:replace="structure view/release/menu:context/edit/fmt:icon" />85 <a tal:replace="structure view/release/menu:context/edit/fmt:icon" />
86 <div style="clear:both" />86 <div style="clear:both"></div>
8787
88 <fieldset class="collapsible collapsed" style="padding: 0px;"88 <fieldset class="collapsible collapsed" style="padding: 0px;"
89 tal:condition="view/release/changelog">89 tal:condition="view/release/changelog">

Subscribers

People subscribed via source and target branches

to status/vote changes: