Merge lp:~sinzui/launchpad/front-page-layout into lp:launchpad

Proposed by Curtis Hovey on 2012-03-23
Status: Merged
Approved by: Curtis Hovey on 2012-03-26
Approved revision: no longer in the source branch.
Merged at revision: 15014
Proposed branch: lp:~sinzui/launchpad/front-page-layout
Merge into: lp:launchpad
Diff against target: 143 lines (+66/-1)
6 files modified
lib/lp/app/browser/root.py (+1/-0)
lib/lp/app/browser/tales.py (+11/-0)
lib/lp/app/browser/tests/test_launchpadroot.py (+20/-0)
lib/lp/app/browser/tests/test_page_macro.py (+25/-0)
lib/lp/app/templates/base-layout.pt (+2/-1)
lib/lp/app/templates/root-index.pt (+7/-0)
To merge this branch: bzr merge lp:~sinzui/launchpad/front-page-layout
Reviewer Review Type Date Requested Status
Benji York (community) code 2012-03-23 Approve on 2012-03-26
Review via email: mp+99121@code.launchpad.net

Commit Message

Replace the launchpad watermark with the logo.

Description of the Change

Pre-implementation: no one

Launchpad root was switched to main_only layout when the locationless
layout was removed. The watermark does not align with the page content
because the root page defines its own styles :(

--------------------------------------------------------------------

RULES

    * Add a hack to permit the Lp front page to say STFU to the watermark.
    * Add the Lp logo and style hack beneath it to restore the previous
      layout (removed in revno 14867).

QA

    See https://launchpadlibrarian.net/98063949/lp-logo-front-page.png
    for an example of the reverted change.

    * Visit https://qastaging.launchpad.net/
    * Verify the application links are not shown.
    * Verify the Lp logo is shown above the two columns
      with a dotted line below it.

LINT

    lib/lp/app/browser/root.py
    lib/lp/app/browser/tales.py
    lib/lp/app/browser/tests/test_launchpadroot.py
    lib/lp/app/browser/tests/test_page_macro.py
    lib/lp/app/templates/base-layout.pt
    lib/lp/app/templates/root-index.pt

TEST

    ./bin/test -vv -t page_macro -t base_layout -t root lp.app.browser.tests

IMPLEMENTATION

Added a hack to allow a view to say it does not have the watermark. We
do not want to make this easy for developers to do because the locationless
layout was abused causing lots of breadcrumb and heading issues.
    lib/lp/app/browser/tales.py
    lib/lp/app/browser/tests/test_page_macro.py
    lib/lp/app/templates/base-layout.pt

Updated the launchpadRootView to state is does not have a watermark and
reverted the change to the page template made in revno 14867.
    lib/lp/app/browser/root.py
    lib/lp/app/browser/tests/test_launchpadroot.py
    lib/lp/app/templates/root-index.pt

To post a comment you must log in.
Benji York (benji) wrote :

This looks good.

review: Approve (code)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/app/browser/root.py'
2--- lib/lp/app/browser/root.py 2011-12-30 08:13:14 +0000
3+++ lib/lp/app/browser/root.py 2012-03-26 13:27:19 +0000
4@@ -65,6 +65,7 @@
5
6 # Used by the footer to display the lp-arcana section.
7 is_root_page = True
8+ has_watermark = False
9
10 @staticmethod
11 def _get_day_of_year():
12
13=== modified file 'lib/lp/app/browser/tales.py'
14--- lib/lp/app/browser/tales.py 2012-03-02 16:17:46 +0000
15+++ lib/lp/app/browser/tales.py 2012-03-26 13:27:19 +0000
16@@ -2520,6 +2520,7 @@
17 view/macro:pagetype
18
19 view/macro:is-page-contentless
20+ view/macro:has-watermark
21 """
22
23 implements(ITraversable)
24@@ -2554,6 +2555,8 @@
25 return self.pagetype()
26 elif name == 'is-page-contentless':
27 return self.isPageContentless()
28+ elif name == 'has-watermark':
29+ return self.hasWatermark()
30 else:
31 raise TraversalError(name)
32
33@@ -2569,6 +2572,14 @@
34 pagetype = 'unset'
35 return self._pagetypes[pagetype][layoutelement]
36
37+ def hasWatermark(self):
38+ """Does the page havethe watermark block.
39+
40+ The default value is True, but the view can provide has_watermark
41+ to force the page not render the standard location information.
42+ """
43+ return getattr(self.context, 'has_watermark', True)
44+
45 def isPageContentless(self):
46 """Should the template avoid rendering detailed information.
47
48
49=== modified file 'lib/lp/app/browser/tests/test_launchpadroot.py'
50--- lib/lp/app/browser/tests/test_launchpadroot.py 2012-01-01 02:58:52 +0000
51+++ lib/lp/app/browser/tests/test_launchpadroot.py 2012-03-26 13:27:19 +0000
52@@ -108,3 +108,23 @@
53 self.assertEqual(
54 'https://help.launchpad.net/Feedback',
55 request.response.getHeader('location'))
56+
57+
58+class LaunchpadRootIndexViewTestCase(TestCaseWithFactory):
59+
60+ layer = DatabaseFunctionalLayer
61+
62+ def test_has_logo_without_watermark(self):
63+ root = getUtility(ILaunchpadRoot)
64+ user = self.factory.makePerson()
65+ login_person(user)
66+ view = create_initialized_view(root, 'index.html', principal=user)
67+ # Replace the blog posts so the view does not make a network request.
68+ view.getRecentBlogPosts = lambda: []
69+ markup = BeautifulSoup(
70+ view(), parseOnlyThese=SoupStrainer(id='document'))
71+ self.assertIs(False, view.has_watermark)
72+ self.assertIs(None, markup.find(True, id='watermark'))
73+ logo = markup.find(True, id='launchpad-logo-and-name')
74+ self.assertIsNot(None, logo)
75+ self.assertEqual('/@@/launchpad-logo-and-name.png', logo['src'])
76
77=== modified file 'lib/lp/app/browser/tests/test_page_macro.py'
78--- lib/lp/app/browser/tests/test_page_macro.py 2012-01-01 02:58:52 +0000
79+++ lib/lp/app/browser/tests/test_page_macro.py 2012-03-26 13:27:19 +0000
80@@ -123,6 +123,31 @@
81 KeyError, "'fnord'",
82 self._call_test_tales, 'view/macro:pagehas/fnord')
83
84+ def test_has_watermark_default(self):
85+ # All pages have a watermark if the view does not provide the attr.
86+ has_watermark = test_tales('view/macro:has-watermark', view=self.view)
87+ self.assertIs(True, has_watermark)
88+
89+ def test_has_watermark_false(self):
90+ # A view cand define has_watermark as False.
91+ class NoWatermarkView(TestView):
92+ has_watermark = False
93+
94+ self.registerBrowserViewAdapter(NoWatermarkView, ITest, '+test')
95+ view = create_view(TestObject(), name='+test')
96+ has_watermark = test_tales('view/macro:has-watermark', view=view)
97+ self.assertIs(False, has_watermark)
98+
99+ def test_has_watermark_true(self):
100+ # A view cand define has_watermark as True.
101+ class NoWatermarkView(TestView):
102+ has_watermark = True
103+
104+ self.registerBrowserViewAdapter(NoWatermarkView, ITest, '+test')
105+ view = create_view(TestObject(), name='+test')
106+ has_watermark = test_tales('view/macro:has-watermark', view=view)
107+ self.assertIs(True, has_watermark)
108+
109
110 class PageMacroDispatcherInteractionTestCase(TestPageMacroDispatcherMixin,
111 TestCaseWithFactory):
112
113=== modified file 'lib/lp/app/templates/base-layout.pt'
114--- lib/lp/app/templates/base-layout.pt 2012-03-14 06:58:58 +0000
115+++ lib/lp/app/templates/base-layout.pt 2012-03-26 13:27:19 +0000
116@@ -79,7 +79,8 @@
117 <tal:login replace="structure context/@@login_status" />
118 </div><!--id="locationbar"-->
119
120- <div id="watermark" class="watermark-apps-portlet">
121+ <div id="watermark" class="watermark-apps-portlet"
122+ tal:condition="view/macro:has-watermark">
123 <div>
124 <span tal:replace="structure view/watermark:logo"></span>
125 </div>
126
127=== modified file 'lib/lp/app/templates/root-index.pt'
128--- lib/lp/app/templates/root-index.pt 2012-03-06 11:19:22 +0000
129+++ lib/lp/app/templates/root-index.pt 2012-03-26 13:27:19 +0000
130@@ -57,6 +57,13 @@
131 <!-- Is your project registered yet? -->
132
133 <div id="homepage" class="homepage">
134+
135+ <div class="top-portlet" style="border-bottom: 1px dotted #999;">
136+ <img src="/@@/launchpad-logo-and-name.png"
137+ id="launchpad-logo-and-name" alt=""
138+ style="margin: 0 9em 1em 0"/>
139+ </div>
140+
141 <div class="yui-g">
142 <div class="yui-u first" style="margin-top: 1.5em;">
143 <div class="homepage-whatslaunchpad"