launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #02182
[Merge] lp:~sinzui/launchpad/css-ui-0 into lp:launchpad
Curtis Hovey has proposed merging lp:~sinzui/launchpad/css-ui-0 into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
#36287 NOTICE and INFO logging levels should be merged
https://bugs.launchpad.net/bugs/36287
#253558 New message styling obscures useful content
https://bugs.launchpad.net/bugs/253558
#330035 "Please try again" page is crooked
https://bugs.launchpad.net/bugs/330035
Update browser notices and offline pages.
Launchpad bug:
https://bugs.launchpad.net/bugs/36287
https://bugs.launchpad.net/bugs/253558
https://bugs.launchpad.net/bugs/330035
Pre-implementation: no one
Test command: ./bin/test -vv -t notifications -t test_launchpad
Bug #36287 [NOTICE and INFO logging levels should be merged]
NOTICE and INFO messages are shown identically. NOTICE seems to be
custom fluff and should be removed.
Bug #253558 [New message styling obscures useful content]
the message class styles obscure page content in multiple locations.
Bug #330035 ["Please try again" page is crooked]
"Please try again" page <https://launchpad.net/offline.html>, the heading
is correctly horizontally centered with respect to the whole viewport; but
when the browser window is wider than 60em, the body text is way off to
the left.
--------------------------------------------------------------------
RULES
Bug #36287 [NOTICE and INFO logging levels should be merged]
* Remove NOTICE. Update the few callsite that use it to use INFO instead.
Bug #253558 [New message styling obscures useful content]
* Add a top margin of 1em to clear the previous content. Use em units
for margin and padding to keep the distance proportional to the font.
Bug #330035 ["Please try again" page is crooked]
* The offline css needs a margin rule.
* Several of the offline pages are trying to load the mast css file
which cannot be guaranteed to be served. Inline the font and offline
rules to ensure all pages are centered with the correct font.
* Remove the unused .offline style from the style sheet.
QA
Bug #36287 [NOTICE and INFO logging levels should be merged]
* Mark a blueprint as started.
* Verify an info notice is shown. (This will look identical to
product because the template macro converted NOTICE to INFO)
Bug #253558 [New message styling obscures useful content]
* Mark a blueprint as started.
* Verify the info icon is below the registration line.
Bug #330035 ["Please try again" page is crooked]
* Not QAable while the server is up. The html file in the tree can
be looked at with a browser without a running server.
LINT
lib/canonical/launchpad/offline-maintenance-haproxy.html
lib/canonical/launchpad/offline-maintenance.html
lib/canonical/launchpad/offline-staging-code-update.html
lib/canonical/launchpad/offline-staging-db-update.html
lib/canonical/launchpad/offline-unplanned-haproxy.html
lib/canonical/launchpad/offline-unplanned.html
lib/canonical/launchpad/browser/tests/test_launchpad.py
lib/canonical/launchpad/icing/style-3-0.css.in
lib/canonical/launchpad/icing/style.css
lib/canonical/launchpad/webapp/interfaces.py
lib/canonical/launchpad/webapp/login.py
lib/canonical/launchpad/webapp/notifications.py
lib/lp/app/stories/basics/xx-notifications.txt
lib/lp/app/templates/base-layout-macros.pt
lib/lp/blueprints/browser/tests/test_specification.py
lib/lp/registry/browser/person.py
^ There is a lot of lint in these files. I can fix this after the review.
IMPLEMENTATION
Bug #36287 [NOTICE and INFO logging levels should be merged]
Removed NOTICE and and its tests. Updated the account and spec uses
notices to use INFO.
lib/canonical/launchpad/browser/tests/test_launchpad.py
lib/canonical/launchpad/webapp/interfaces.py
lib/canonical/launchpad/webapp/login.py
lib/canonical/launchpad/webapp/notifications.py
lib/lp/app/templates/base-layout-macros.pt
lib/lp/blueprints/browser/tests/test_specification.py
lib/lp/registry/browser/person.py
lib/lp/app/stories/basics/xx-notifications.txt
Bug #253558 [New message styling obscures useful content]
Added 1em to the top margin and updated the padding to be in em units.
lib/canonical/launchpad/icing/style-3-0.css.in
Bug #330035 ["Please try again" page is crooked]
Inlined the CSS rules for the pages and removed the rule that was never
loaded.
lib/canonical/launchpad/offline-maintenance-haproxy.html
lib/canonical/launchpad/offline-maintenance.html
lib/canonical/launchpad/offline-staging-code-update.html
lib/canonical/launchpad/offline-staging-db-update.html
lib/canonical/launchpad/offline-unplanned-haproxy.html
lib/canonical/launchpad/offline-unplanned.html
lib/canonical/launchpad/icing/style.css
--
https://code.launchpad.net/~sinzui/launchpad/css-ui-0/+merge/43936
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~sinzui/launchpad/css-ui-0 into lp:launchpad.
=== modified file 'lib/canonical/launchpad/icing/style-3-0.css.in'
--- lib/canonical/launchpad/icing/style-3-0.css.in 2010-10-31 20:18:45 +0000
+++ lib/canonical/launchpad/icing/style-3-0.css.in 2010-12-16 17:36:13 +0000
@@ -969,8 +969,8 @@
border: solid #666;
border-width: 1px 2px 2px 1px;
color: black;
- margin: 0 auto 10px auto;
- padding: 0 10px 10px 20px;
+ margin: 1em auto 1em auto;
+ padding: 0 1em 1em 2em;
width: 30em;
}
.error.message::before, .warning.message::before,
=== modified file 'lib/canonical/launchpad/icing/style.css'
--- lib/canonical/launchpad/icing/style.css 2010-11-19 20:34:37 +0000
+++ lib/canonical/launchpad/icing/style.css 2010-12-16 17:36:13 +0000
@@ -32,7 +32,6 @@
.exception {color: #cc0000;}
.discreet, .lesser, .duplicate-details {font-size: 90%;}
.extra-discreet {font-size: 75%;}
-.offline {text-align: center; margin: 2em 0;}
.unavailable {color: #999; background: none;}
.duplicate-details p { padding: 0em; margin-top: 0em; margin-bottom: 0.2em; }
=== modified file 'lib/canonical/launchpad/offline-maintenance-haproxy.html'
--- lib/canonical/launchpad/offline-maintenance-haproxy.html 2010-04-01 13:43:01 +0000
+++ lib/canonical/launchpad/offline-maintenance-haproxy.html 2010-12-16 17:36:13 +0000
@@ -6,7 +6,17 @@
<head>
<title>Launchpad is offline for maintenance</title>
<style type="text/css" media="screen, print">
- .offline{text-align:center;margin:2em 0}
+ html, body {
+ font-family: UbuntuBeta, Ubuntu, 'Bitstream Vera Sans', 'DejaVu Sans', Tahoma, sans-serif;
+ }
+ .offline {
+ text-align: center;
+ max-width: 60em;
+ }
+ .offline > * {
+ margin-left: auto;
+ margin-right: auto;
+ }
</style>
</head>
<body>
=== modified file 'lib/canonical/launchpad/offline-maintenance.html'
--- lib/canonical/launchpad/offline-maintenance.html 2010-04-01 13:43:01 +0000
+++ lib/canonical/launchpad/offline-maintenance.html 2010-12-16 17:36:13 +0000
@@ -3,7 +3,17 @@
<head>
<title>Launchpad is offline for maintenance</title>
<style type="text/css" media="screen, print">
- @import url(https://launchpad.net/+icing/rev5/+combo.css);
+ html, body {
+ font-family: UbuntuBeta, Ubuntu, 'Bitstream Vera Sans', 'DejaVu Sans', Tahoma, sans-serif;
+ }
+ .offline {
+ text-align: center;
+ max-width: 60em;
+ }
+ .offline > * {
+ margin-left: auto;
+ margin-right: auto;
+ }
</style>
</head>
<body>
=== modified file 'lib/canonical/launchpad/offline-staging-code-update.html'
--- lib/canonical/launchpad/offline-staging-code-update.html 2010-05-14 15:52:49 +0000
+++ lib/canonical/launchpad/offline-staging-code-update.html 2010-12-16 17:36:13 +0000
@@ -3,7 +3,17 @@
<head>
<title>Please try again</title>
<style type="text/css" media="screen, print">
- @import url(https://launchpad.net/+icing/rev5/combo.css);
+ html, body {
+ font-family: UbuntuBeta, Ubuntu, 'Bitstream Vera Sans', 'DejaVu Sans', Tahoma, sans-serif;
+ }
+ .offline {
+ text-align: center;
+ max-width: 60em;
+ }
+ .offline > * {
+ margin-left: auto;
+ margin-right: auto;
+ }
</style>
</head>
<body>
=== modified file 'lib/canonical/launchpad/offline-staging-db-update.html'
--- lib/canonical/launchpad/offline-staging-db-update.html 2010-05-14 15:52:49 +0000
+++ lib/canonical/launchpad/offline-staging-db-update.html 2010-12-16 17:36:13 +0000
@@ -3,7 +3,17 @@
<head>
<title>Please try again</title>
<style type="text/css" media="screen, print">
- @import url(https://launchpad.net/+icing/rev5/combo.css);
+ html, body {
+ font-family: UbuntuBeta, Ubuntu, 'Bitstream Vera Sans', 'DejaVu Sans', Tahoma, sans-serif;
+ }
+ .offline {
+ text-align: center;
+ max-width: 60em;
+ }
+ .offline > * {
+ margin-left: auto;
+ margin-right: auto;
+ }
</style>
</head>
<body>
=== modified file 'lib/canonical/launchpad/offline-unplanned-haproxy.html'
--- lib/canonical/launchpad/offline-unplanned-haproxy.html 2010-03-02 10:42:06 +0000
+++ lib/canonical/launchpad/offline-unplanned-haproxy.html 2010-12-16 17:36:13 +0000
@@ -6,7 +6,17 @@
<head>
<title>Please try again</title>
<style type="text/css" media="screen, print">
- .offline{text-align:center;margin:2em 0}
+ html, body {
+ font-family: UbuntuBeta, Ubuntu, 'Bitstream Vera Sans', 'DejaVu Sans', Tahoma, sans-serif;
+ }
+ .offline {
+ text-align: center;
+ max-width: 60em;
+ }
+ .offline > * {
+ margin-left: auto;
+ margin-right: auto;
+ }
</style>
</head>
<body>
=== modified file 'lib/canonical/launchpad/offline-unplanned.html'
--- lib/canonical/launchpad/offline-unplanned.html 2010-01-04 12:58:17 +0000
+++ lib/canonical/launchpad/offline-unplanned.html 2010-12-16 17:36:13 +0000
@@ -3,7 +3,17 @@
<head>
<title>Please try again</title>
<style type="text/css" media="screen, print">
- @import url(https://launchpad.net/+icing/rev5/combo.css);
+ html, body {
+ font-family: UbuntuBeta, Ubuntu, 'Bitstream Vera Sans', 'DejaVu Sans', Tahoma, sans-serif;
+ }
+ .offline {
+ text-align: center;
+ max-width: 60em;
+ }
+ .offline > * {
+ margin-left: auto;
+ margin-right: auto;
+ }
</style>
</head>
<body>
=== modified file 'lib/canonical/launchpad/webapp/interfaces.py'
--- lib/canonical/launchpad/webapp/interfaces.py 2010-11-02 19:55:24 +0000
+++ lib/canonical/launchpad/webapp/interfaces.py 2010-12-16 17:36:13 +0000
@@ -568,15 +568,12 @@
"""Matches the standard logging levels, with the addition of notice
(which we should probably add to our log levels as well)
"""
- # XXX mpt 2006-03-22 bugs=36287:
- # NOTICE and INFO should be merged.
DEBUG = logging.DEBUG # A debugging message
INFO = logging.INFO # simple confirmation of a change
- NOTICE = logging.INFO + 5 # action had effects you might not have intended
WARNING = logging.WARNING # action will not be successful unless you ...
ERROR = logging.ERROR # the previous action did not succeed, and why
- ALL_LEVELS = (DEBUG, INFO, NOTICE, WARNING, ERROR)
+ ALL_LEVELS = (DEBUG, INFO, WARNING, ERROR)
class INotification(Interface):
@@ -593,7 +590,7 @@
def __getitem__(index_or_levelname):
"""Retrieve an INotification by index, or a list of INotification
- instances by level name (DEBUG, NOTICE, INFO, WARNING, ERROR).
+ instances by level name (DEBUG, INFO, WARNING, ERROR).
"""
def __iter__():
@@ -616,7 +613,7 @@
have been set when redirect() is called.
"""
- def addNotification(msg, level=BrowserNotificationLevel.NOTICE):
+ def addNotification(msg, level=BrowserNotificationLevel.INFO):
"""Append the given message to the list of notifications.
A plain string message will be CGI escaped. Passing a message
@@ -629,7 +626,7 @@
or an instance of `IStructuredString`.
:param level: One of the `BrowserNotificationLevel` values: DEBUG,
- INFO, NOTICE, WARNING, ERROR.
+ INFO, WARNING, ERROR.
"""
def removeAllNotifications():
@@ -649,9 +646,6 @@
def addInfoNotification(msg):
"""Shortcut to addNotification(msg, INFO)."""
- def addNoticeNotification(msg):
- """Shortcut to addNotification(msg, NOTICE)."""
-
def addWarningNotification(msg):
"""Shortcut to addNotification(msg, WARNING)."""
=== modified file 'lib/canonical/launchpad/webapp/login.py'
--- lib/canonical/launchpad/webapp/login.py 2010-10-26 15:47:24 +0000
+++ lib/canonical/launchpad/webapp/login.py 2010-12-16 17:36:13 +0000
@@ -119,7 +119,7 @@
target = self.getRedirectURL(current_url, query_string)
# A dance to assert that we want to break the rules about no
# unauthenticated sessions. Only after this next line is it safe
- # to use the ``addNoticeNotification`` method.
+ # to use the ``addInfoNotification`` method.
allowUnauthenticatedSession(self.request)
self.request.response.redirect(target)
# Maybe render page with a link to the redirection?
@@ -365,7 +365,7 @@
# The authentication failed (or was canceled), but the user is
# already logged in, so we just add a notification message and
# redirect.
- self.request.response.addNoticeNotification(
+ self.request.response.addInfoNotification(
_(u'Your authentication failed but you were already '
'logged into Launchpad.'))
self._redirect()
=== modified file 'lib/canonical/launchpad/webapp/notifications.py'
--- lib/canonical/launchpad/webapp/notifications.py 2010-08-20 20:31:18 +0000
+++ lib/canonical/launchpad/webapp/notifications.py 2010-12-16 17:36:13 +0000
@@ -16,7 +16,6 @@
from datetime import datetime
-from zope.app.security.interfaces import IUnauthenticatedPrincipal
from zope.interface import implements
from zope.session.interfaces import ISession
@@ -114,7 +113,6 @@
>>> response.addNotification("Whatever", BrowserNotificationLevel.DEBUG)
>>> response.addDebugNotification('Debug')
>>> response.addInfoNotification('Info')
- >>> response.addNoticeNotification('Notice')
>>> response.addWarningNotification('Warning')
And an odd one to test Bug #54987
@@ -131,7 +129,6 @@
10 -- Whatever
10 -- Debug
20 -- Info
- 25 -- Notice
30 -- Warning
40 -- Error
@@ -144,7 +141,7 @@
>>> for notification in ISession(request)[SESSION_KEY]['notifications']:
... print "%d -- %s" % (notification.level, notification.message)
... break
- 25 -- <b><Fnord></b>
+ 20 -- <b><Fnord></b>
If there are no notifications, the session is not touched. This ensures
that we don't needlessly burden the session storage.
@@ -172,7 +169,7 @@
# which would be bad.
_notifications = None
- def addNotification(self, msg, level=BrowserNotificationLevel.NOTICE):
+ def addNotification(self, msg, level=BrowserNotificationLevel.INFO):
"""See `INotificationResponse`."""
self.notifications.append(
Notification(level, escape(msg)))
@@ -237,10 +234,6 @@
"""See `INotificationResponse`."""
self.addNotification(msg, BrowserNotificationLevel.INFO)
- def addNoticeNotification(self, msg):
- """See `INotificationResponse`."""
- self.addNotification(msg, BrowserNotificationLevel.NOTICE)
-
def addWarningNotification(self, msg):
"""See `INotificationResponse`."""
self.addNotification(msg, BrowserNotificationLevel.WARNING)
@@ -334,8 +327,6 @@
structured('Debug notification <b>%d</b>' % count))
response.addInfoNotification(
structured('Info notification <b>%d</b>' % count))
- response.addNoticeNotification(
- structured('Notice notification <b>%d</b>' % count))
response.addWarningNotification(
structured('Warning notification <b>%d</b>' %count))
response.addErrorNotification(
=== renamed file 'lib/canonical/launchpad/browser/tests/test_launchpad.py' => 'lib/lp/app/browser/tests/test_launchpad.py'
--- lib/canonical/launchpad/browser/tests/test_launchpad.py 2010-10-21 04:41:30 +0000
+++ lib/lp/app/browser/tests/test_launchpad.py 2010-12-16 17:36:13 +0000
@@ -127,7 +127,7 @@
def assertDisplaysNotice(self, path, notification):
"""Assert that traversal redirects back with the specified notice."""
self.assertDisplaysNotification(
- path, notification, BrowserNotificationLevel.NOTICE)
+ path, notification, BrowserNotificationLevel.INFO)
def assertDisplaysError(self, path, notification):
"""Assert that traversal redirects back with the specified notice."""
=== renamed file 'lib/canonical/launchpad/pagetests/standalone/xx-notifications.txt' => 'lib/lp/app/stories/basics/xx-notifications.txt'
--- lib/canonical/launchpad/pagetests/standalone/xx-notifications.txt 2009-10-22 18:13:42 +0000
+++ lib/lp/app/stories/basics/xx-notifications.txt 2010-12-16 17:36:13 +0000
@@ -15,8 +15,8 @@
...<div class="error message">Error notification <b>2</b></div>
...<div class="warning message">Warning notification <b>1</b></div>
...<div class="warning message">Warning notification <b>2</b></div>
-...<div class="informational message">Notice notification <b>1</b></div>
-...<div class="informational message">Notice notification <b>2</b></div>
+...<div class="informational message">Info notification <b>1</b></div>
+...<div class="informational message">Info notification <b>2</b></div>
...<div class="debug message">Debug notification <b>1</b></div>
...<div class="debug message">Debug notification <b>2</b></div>
...
@@ -49,8 +49,6 @@
...<div class="error message">Error notification <b>2</b></div>
...<div class="warning message">Warning notification <b>1</b></div>
...<div class="warning message">Warning notification <b>2</b></div>
-...<div class="informational message">Notice notification <b>1</b></div>
-...<div class="informational message">Notice notification <b>2</b></div>
...<div class="informational message">Info notification <b>1</b></div>
...<div class="informational message">Info notification <b>2</b></div>
...<div class="debug message">Debug notification <b>1</b></div>
@@ -90,8 +88,6 @@
...<div class="error message">Error notification <b>2</b></div>
...<div class="warning message">Warning notification <b>1</b></div>
...<div class="warning message">Warning notification <b>2</b></div>
-...<div class="informational message">Notice notification <b>1</b></div>
-...<div class="informational message">Notice notification <b>2</b></div>
...<div class="informational message">Info notification <b>1</b></div>
...<div class="informational message">Info notification <b>2</b></div>
...<div class="debug message">Debug notification <b>1</b></div>
@@ -151,8 +147,6 @@
...<div class="error message">Error notification <b>2</b></div>
...<div class="warning message">Warning notification <b>1</b></div>
...<div class="warning message">Warning notification <b>2</b></div>
-...<div class="informational message">Notice notification <b>1</b></div>
-...<div class="informational message">Notice notification <b>2</b></div>
...<div class="informational message">Info notification <b>1</b></div>
...<div class="informational message">Info notification <b>2</b></div>
...<div class="debug message">Debug notification <b>1</b></div>
=== modified file 'lib/lp/app/templates/base-layout-macros.pt'
--- lib/lp/app/templates/base-layout-macros.pt 2010-12-14 20:53:23 +0000
+++ lib/lp/app/templates/base-layout-macros.pt 2010-12-16 17:36:13 +0000
@@ -23,10 +23,6 @@
tal:content="structure notification/message"
>A warning notification message</div>
<div class="informational message"
- tal:repeat="notification notifications/notice"
- tal:content="structure notification/message"
- >A notice notification message</div>
- <div class="informational message"
tal:repeat="notification notifications/info"
tal:content="structure notification/message"
>An info notification message</div>
=== modified file 'lib/lp/blueprints/browser/tests/test_specification.py'
--- lib/lp/blueprints/browser/tests/test_specification.py 2010-12-01 11:26:57 +0000
+++ lib/lp/blueprints/browser/tests/test_specification.py 2010-12-16 17:36:13 +0000
@@ -129,7 +129,7 @@
SpecificationImplementationStatus.STARTED, spec.implementation_status)
self.assertEqual(spec.owner, spec.starter)
[notification] = view.request.notifications
- self.assertEqual(BrowserNotificationLevel.NOTICE, notification.level)
+ self.assertEqual(BrowserNotificationLevel.INFO, notification.level)
self.assertEqual(
'Blueprint is now considered "Started".', notification.message)
@@ -162,7 +162,7 @@
spec.implementation_status)
self.assertIs(None, spec.starter)
[notification] = view.request.notifications
- self.assertEqual(BrowserNotificationLevel.NOTICE, notification.level)
+ self.assertEqual(BrowserNotificationLevel.INFO, notification.level)
self.assertEqual(
'Blueprint is now considered "Not started".', notification.message)
@@ -182,7 +182,7 @@
spec.implementation_status)
self.assertEqual(spec.owner, spec.completer)
[notification] = view.request.notifications
- self.assertEqual(BrowserNotificationLevel.NOTICE, notification.level)
+ self.assertEqual(BrowserNotificationLevel.INFO, notification.level)
self.assertEqual(
'Blueprint is now considered "Complete".', notification.message)
=== modified file 'lib/lp/registry/browser/person.py'
--- lib/lp/registry/browser/person.py 2010-12-10 23:10:09 +0000
+++ lib/lp/registry/browser/person.py 2010-12-16 17:36:13 +0000
@@ -1559,7 +1559,7 @@
def deactivate_action(self, action, data):
self.context.deactivateAccount(data['comment'])
logoutPerson(self.request)
- self.request.response.addNoticeNotification(
+ self.request.response.addInfoNotification(
_(u'Your account has been deactivated.'))
self.next_url = self.request.getApplicationURL()
@@ -1831,12 +1831,12 @@
# email is sent to the user.
data['password'] = 'invalid'
self.person.setPreferredEmail(None)
- self.request.response.addNoticeNotification(
+ self.request.response.addInfoNotification(
u'The account "%s" has been suspended.' % (
self.context.displayname))
if (data['status'] == AccountStatus.ACTIVE
and self.context.status != AccountStatus.ACTIVE):
- self.request.response.addNoticeNotification(
+ self.request.response.addInfoNotification(
u'The user is reactivated. He must use the '
u'"forgot password" to log in.')
self.updateContextFromData(data)
