launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #08036
[Merge] lp:~jcsackett/launchpad/progressively-enhanced-banners into lp:launchpad
j.c.sackett has proposed merging lp:~jcsackett/launchpad/progressively-enhanced-banners into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~jcsackett/launchpad/progressively-enhanced-banners/+merge/106487
Summary
=======
The privacy banner (and it's close friend, the beta banner) don't work without
JS. This introduces basic html banners into base-layout that are enhanced by
the JS banners.
Preimp
======
Spoke with Curtis Hovey
Implementation
==============
* CSS rules for the banners have been updated to use the YUI css classes.
* The public private css formatter has been renamed to the global css
formatter, and now adds the global-notification-visible class as well when
it's needed.
* The `private` attribute on the LaunchpadView has been changed to a property
method, encapsulating most of the special work in the
public_private_formatter (e.g. checking if the context is private).
* A `beta_features` method has been added that returns the related_features
that are in beta.
* Banner-macros has been added; it encapsulates the privacy banner and beta
banner html, and uses the private and beta_features attributes on
LaunchpadView to determine if the html is presented.
* Some minor js tweaks were made to work with the new banner html.
The close button doesn't get rendered on static beta banners; making this work
would require some extra investment that didn't seem worth it given the
relatively low usage of non-js browsers with beta features. If it's needed, a
followup branch can always be landed.
Tests
=====
bin/test -vvct base-layout -t test_formatters -t tales
bin/test -vvct beta -t privacy --layer=YUI
QA
==
Disable js in your browser. Check the privacy banner and the beta banner,
everything should be fine.
Enable js in your browser. Check it all again. It should all be fine.
LoC
===
This branch is part of the disclosure arc of work.
Lint
====
Checking for conflicts and issues in changed files.
Linting changed files:
lib/lp/app/javascript/banners/beta-notification.js
lib/canonical/launchpad/icing/css/components/sidebar_components.css
lib/canonical/launchpad/icing/css/components/global_notification.css
lib/lp/app/javascript/banners/banner.js
lib/lp/services/webapp/publisher.py
lib/lp/app/templates/banner-macros.pt
lib/lp/app/templates/base-layout.pt
lib/lp/app/javascript/banners/privacy.js
lib/lp/app/browser/configure.zcml
lib/lp/app/browser/tales.py
lib/canonical/launchpad/icing/css/components/beta_banner.css
lib/canonical/launchpad/icing/css/base.css
Lint has been elided; there are some things I will fix before landing, but
most of this was a massive set of css line issues which were introduced by
more css-competent people than me that I'm leery to touch.
--
https://code.launchpad.net/~jcsackett/launchpad/progressively-enhanced-banners/+merge/106487
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jcsackett/launchpad/progressively-enhanced-banners into lp:launchpad.
=== modified file 'lib/canonical/launchpad/icing/css/base.css'
--- lib/canonical/launchpad/icing/css/base.css 2012-03-10 15:33:33 +0000
+++ lib/canonical/launchpad/icing/css/base.css 2012-05-19 01:21:19 +0000
@@ -6,10 +6,6 @@
}
body.private {
/* It must be obvious to the user that the context is private */
- background: url("/@@/private-y-bg") top left repeat-y;
- }
-/* Override for when the feature flag is active */
-body.feature-flag-bugs-private-notification-enabled.private {
background-image: none;
}
body.private .private-disallow {
=== modified file 'lib/canonical/launchpad/icing/css/components/beta_banner.css'
--- lib/canonical/launchpad/icing/css/components/beta_banner.css 2012-03-10 15:43:21 +0000
+++ lib/canonical/launchpad/icing/css/components/beta_banner.css 2012-05-19 01:21:19 +0000
@@ -1,7 +1,7 @@
/* ===========
Beta banner
*/
-.beta-banner {
+.yui3-betabanner-content .global-notification {
position: fixed;
z-index: 9;
top: 0;
@@ -17,6 +17,7 @@
text-shadow: 0 -1px 0 rgba(0, 0, 0, 0.5);
font-size: 14px;
line-height: 21px;
+ text-align: left;
}
.beta-banner .info-link {
color: #4884ef;
@@ -40,8 +41,8 @@
.beta-feature {
font-weight: bold;
}
-.beta-banner .global-notification-close,
-.beta-banner .global-notification-close:active,
-.beta-banner .global-notification-close:visited {
+.yui3-betabanner-content .global-notification-close,
+.yui3-betabanner-content .global-notification-close:active,
+.yui3-betabanner-content .global-notification-close:visited {
color: #fff;
}
=== modified file 'lib/canonical/launchpad/icing/css/components/global_notification.css'
--- lib/canonical/launchpad/icing/css/components/global_notification.css 2012-05-15 17:17:34 +0000
+++ lib/canonical/launchpad/icing/css/components/global_notification.css 2012-05-19 01:21:19 +0000
@@ -21,8 +21,6 @@
font-weight: bold;
box-shadow: 0 0 5px #333;
}
-
-
.global-notification .sprite.notification-private {
float: left;
display: inline-block;
@@ -58,10 +56,11 @@
Privacy
*/
-.privacy-banner {
+.yui3-privacybanner-content .global-notification{
/* Define colour for browsers that don't support transparency */
background-color: #8d1f1f;
/* Set transparent background for browsers that support it */
background-color: rgba(125,0,0,0.9);
color: #fff;
+ text-align: left;
}
=== modified file 'lib/canonical/launchpad/icing/css/components/sidebar_components.css'
--- lib/canonical/launchpad/icing/css/components/sidebar_components.css 2012-03-10 15:33:33 +0000
+++ lib/canonical/launchpad/icing/css/components/sidebar_components.css 2012-05-19 01:21:19 +0000
@@ -24,16 +24,6 @@
.side ul {
background: #fbfbfb;
}
-#privacy.private {
- background: url(/@@/private-bg) top left repeat-x; /* 8px high */
- padding-top: 12px; /* = 8px + the usual 4px top padding */
- }
-/* Override for when the feature flag is active */
-.feature-flag-bugs-private-notification-enabled #privacy.private {
- background-image: none;
- background-color: #FBFBFB;
- padding-top: 0.5em;
- }
.downloads li {
margin: 0;
padding: 2px 0 0;
=== modified file 'lib/lp/app/browser/configure.zcml'
--- lib/lp/app/browser/configure.zcml 2012-04-19 20:37:47 +0000
+++ lib/lp/app/browser/configure.zcml 2012-05-19 01:21:19 +0000
@@ -33,6 +33,13 @@
/>
<browser:page
for="*"
+ name="+banner-macros"
+ template="../templates/banner-macros.pt"
+ permission="zope.Public"
+ class="lp.app.browser.launchpad.Macro"
+ />
+ <browser:page
+ for="*"
name="launchpad_form"
layer="lp.layers.LaunchpadLayer"
permission="zope.Public"
=== modified file 'lib/lp/app/browser/tales.py'
--- lib/lp/app/browser/tales.py 2012-04-20 07:01:18 +0000
+++ lib/lp/app/browser/tales.py 2012-05-19 01:21:19 +0000
@@ -559,7 +559,7 @@
# Names which are allowed but can't be traversed further.
final_traversable_names = {
'pagetitle': 'pagetitle',
- 'public-private-css': 'public_private_css',
+ 'global-css': 'global_css',
}
def __init__(self, context):
@@ -658,22 +658,20 @@
"No link implementation for %r, IPathAdapter implementation "
"for %r." % (self, self._context))
- def public_private_css(self):
- """Return the CSS class that represents the object's privacy."""
- # If a view is marked as private, the context doesn't matter. It will
- # always be displayed as private.
+ def global_css(self):
+ css_classes = set([])
view = self._context
- private_view = getattr(view, 'private', False)
- if private_view:
- return 'private'
-
- # If the view is not marked as private, privacy is determined by the
- # view's context.
- privacy = IPrivacy(getattr(view, 'context', None), None)
- if privacy is not None and privacy.private:
- return 'private'
+ private = getattr(view, 'private', False)
+ if private:
+ css_classes.add('private')
+ css_classes.add('global-notification-visible')
else:
- return 'public'
+ css_classes.add('public')
+ beta = getattr(view, 'beta_features', [])
+ if beta != []:
+ css_classes.add('global-notification-visible')
+ return ' '.join(list(css_classes))
+
def _getSaneBreadcrumbDetail(self, breadcrumb):
text = breadcrumb.detail
=== modified file 'lib/lp/app/javascript/banners/banner.js'
--- lib/lp/app/javascript/banners/banner.js 2012-05-15 17:17:34 +0000
+++ lib/lp/app/javascript/banners/banner.js 2012-05-19 01:21:19 +0000
@@ -23,7 +23,6 @@
slide_out: 0.2,
}
}
-
return anim_times;
},
@@ -98,7 +97,6 @@
var banner_data = {
badge: this.get('banner_icon'),
text: this.get('notification_text'),
- extra_classes: this.get('css_class'),
banner_id: this.get('banner_id')
};
return Y.lp.mustache.to_html(
@@ -117,7 +115,7 @@
this._updateBanner(existing_banner);
} else {
var banner_html = this._createBanner();
- this.get('target').append(banner_html);
+ this.get('contentBox').append(banner_html);
}
},
@@ -135,17 +133,11 @@
banner_id: { value: "base-banner" },
notification_text: { value: "" },
banner_icon: { value: "<span></span>" },
- target: {
- valueFn: function() {
- return Y.one('#maincontent');
- }
- },
banner_template: {
valueFn: function() {
return [
'<div id="{{ banner_id }}"',
- 'class="global-notification transparent hidden',
- ' {{ extra_classes }}">',
+ 'class="global-notification transparent hidden">',
'{{{ badge }}}',
'<span class="banner-text">{{ text }}</span>',
"</div>"].join('')
@@ -153,7 +145,6 @@
},
skip_animation: { value: false },
visible: { value: false },
- css_class: { value: '' }
}
});
=== modified file 'lib/lp/app/javascript/banners/beta-notification.js'
--- lib/lp/app/javascript/banners/beta-notification.js 2012-05-14 22:07:29 +0000
+++ lib/lp/app/javascript/banners/beta-notification.js 2012-05-19 01:21:19 +0000
@@ -19,11 +19,11 @@
renderUI: function () {
baseBanner.prototype.renderUI.apply(this, arguments);
- var banner_node = Y.one('.beta-banner');
+ var beta_node = Y.one('.global-notification');
var close_box = Y.Node.create(
'<a href="#" class="global-notification-close">Hide' +
'<span class="notification-close sprite" /></a>');
- banner_node.appendChild(close_box);
+ beta_node.appendChild(close_box);
},
bindUI: function () {
@@ -45,8 +45,7 @@
valueFn: function() {
return [
'<div id="{{ banner_id }}"',
- 'class="global-notification beta-banner ',
- 'transparent hidden">',
+ 'class="global-notification transparent hidden">',
'{{{ badge }}}',
'<span class="banner-text">',
'{{ text }}{{{ features }}}',
=== modified file 'lib/lp/app/javascript/banners/privacy.js'
--- lib/lp/app/javascript/banners/privacy.js 2012-05-15 17:17:34 +0000
+++ lib/lp/app/javascript/banners/privacy.js 2012-05-19 01:21:19 +0000
@@ -3,15 +3,7 @@
var ns = Y.namespace('lp.app.banner.privacy');
var baseBanner = Y.lp.app.banner.Banner;
-ns.PrivacyBanner = Y.Base.create('privacyBanner', baseBanner, [], {
-
- renderUI: function () {
- var body = Y.one('body');
- body.addClass('feature-flag-bugs-private-notification-enabled');
- baseBanner.prototype.renderUI.apply(this, arguments);
- }
-
-}, {
+ns.PrivacyBanner = Y.Base.create('privacyBanner', baseBanner, [], {}, {
ATTRS: {
banner_id: { value: "privacy-banner" },
notification_text: {
@@ -20,7 +12,6 @@
banner_icon: {
value: '<span class="sprite notification-private"></span>'
},
- css_class: { value: 'privacy-banner' }
}
});
=== added file 'lib/lp/app/templates/banner-macros.pt'
--- lib/lp/app/templates/banner-macros.pt 1970-01-01 00:00:00 +0000
+++ lib/lp/app/templates/banner-macros.pt 2012-05-19 01:21:19 +0000
@@ -0,0 +1,41 @@
+<macros
+ 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";
+ i18n:domain="launchpad"
+ tal:omit-tag=""
+>
+
+<metal:privacy define-macro="privacy-banner">
+ <tal:show-banner condition="view/private">
+ <div class="yui3-privacybanner-content">
+ <div id="privacy-banner" class="global-notification">
+ <span class="sprite notification-private"></span>
+ <span class="banner-text">The information on this page is private.</span>
+ </div>
+ </div>
+ </tal:show-banner>
+</metal:privacy>
+
+<metal:beta define-macro="beta-banner">
+<div class="yui3-betabanner-content">
+ <div id="beta-banner" class="global-notification">
+ <span class="beta-warning">BETA!</span>
+ <span class="banner-text">
+ Some parts of this page are in beta:
+ <span class="beta-feature">
+ <tal:features
+ repeat="feature view/beta_features">
+ <tal:feature replace="feature/title" />
+ <tal:link condition="feature/url">
+ (<a tal:attributes="href feature/url" class="info-link">read more</a>)
+ </tal:link>
+ </tal:features>
+ <span>
+ </span>
+ </div>
+</div>
+</metal:beta>
+
+</macros>
=== modified file 'lib/lp/app/templates/base-layout.pt'
--- lib/lp/app/templates/base-layout.pt 2012-04-12 19:42:35 +0000
+++ lib/lp/app/templates/base-layout.pt 2012-05-19 01:21:19 +0000
@@ -62,7 +62,7 @@
itemtype="http://schema.org/WebPage";
tal:attributes="class string:tab-${view/menu:selectedfacetname}
${view/macro:pagetype}
- ${view/fmt:public-private-css}
+ ${view/fmt:global-css}
yui3-skin-sam">
<script type="text/javascript"
tal:condition="python: is_lpnet">
@@ -97,6 +97,10 @@
<div class="yui-t4"
tal:omit-tag="not: view/macro:pagehas/portlets">
<div id="maincontent" class="yui-main">
+ <metal:privacy-banner
+ use-macro="view/@@+banner-macros/beta-banner"/>
+ <metal:privacy-banner
+ use-macro="view/@@+banner-macros/privacy-banner"/>
<div class="yui-b"
tal:attributes="
lang view/lang|default_language|default;
=== modified file 'lib/lp/services/webapp/publisher.py'
--- lib/lp/services/webapp/publisher.py 2012-04-26 16:21:46 +0000
+++ lib/lp/services/webapp/publisher.py 2012-05-19 01:21:19 +0000
@@ -65,6 +65,7 @@
from zope.traversing.browser.interfaces import IAbsoluteURL
from lp.app.errors import NotFoundError
+from lp.app.interfaces.launchpad import IPrivacy
from lp.app.versioninfo import revno
from lp.layers import (
LaunchpadLayer,
@@ -288,7 +289,14 @@
- publishTraverse() <-- override this to support traversing-through.
"""
- private = False
+ @property
+ def private(self):
+ """A view is private if its context is."""
+ privacy = IPrivacy(self.context, None)
+ if privacy is not None:
+ return privacy.private
+ else:
+ return False
def __init__(self, context, request):
self.context = context
@@ -313,6 +321,15 @@
related_features = cache.setdefault('related_features', {})
related_features.update(self.related_feature_info)
+ def beta_features(self):
+ try:
+ cache = IJSONRequestCache(self.request).objects
+ except TypeError, error:
+ if error.args[0] == 'Could not adapt':
+ return
+ related_features = cache.setdefault('related_features', {}).values()
+ return [f for f in related_features if f['is_beta']]
+
def initialize(self):
"""Override this in subclasses.
Follow ups
