launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #03000
[Merge] lp:~bac/launchpad/hidden_links into lp:launchpad
Brad Crittenden has proposed merging lp:~bac/launchpad/hidden_links into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~bac/launchpad/hidden_links/+merge/53911
= Summary =
We have a need to have a link enabled but hidden which will subsequently
be made visible via JavaScript. If it were not hidden then browsers
which don't support JavaScript would see the link which would be a dead-end.
That said, this branch modifies the Link mechanism to add a 'hidden'
attribute. Links that are hidden but enabled will be rendered with the
class 'invisible-link'.
== Proposed fix ==
As above.
== Pre-implementation notes ==
Talk with Curtis.
== Implementation details ==
As above.
== Tests ==
bin/test -vvt doc/menus.txt
== Demo and Q/A ==
None
= Launchpad lint =
Checking for conflicts and issues in changed files.
I may fix these pre-existing problems but I may not.
Linting changed files:
lib/canonical/launchpad/webapp/interfaces.py
lib/canonical/launchpad/webapp/menu.py
lib/canonical/launchpad/templates/launchpad-inline-link.pt
lib/canonical/launchpad/doc/menus.txt
./lib/canonical/launchpad/webapp/interfaces.py
879: redefinition of unused 'StartRequestEvent' from line 871
42: 'UnexpectedFormData' imported but unused
279: E301 expected 1 blank line, found 0
281: E202 whitespace before ')'
289: E302 expected 2 blank lines, found 1
309: E202 whitespace before ')'
314: E301 expected 1 blank line, found 0
317: E301 expected 1 blank line, found 0
319: E301 expected 1 blank line, found 0
321: E301 expected 1 blank line, found 0
428: E301 expected 1 blank line, found 0
435: E301 expected 1 blank line, found 0
443: E301 expected 1 blank line, found 0
458: E301 expected 1 blank line, found 0
502: E302 expected 2 blank lines, found 1
549: E302 expected 2 blank lines, found 1
611: E202 whitespace before ')'
644: E202 whitespace before ')'
756: E301 expected 1 blank line, found 0
826: E301 expected 1 blank line, found 0
873: E301 expected 1 blank line, found 0
879: E301 expected 1 blank line, found 2
200: Line exceeds 78 characters.
./lib/canonical/launchpad/doc/menus.txt
1: narrative uses a moin header.
6: narrative uses a moin header.
55: narrative uses a moin header.
102: source exceeds 78 characters.
164: narrative uses a moin header.
253: narrative uses a moin header.
333: source exceeds 78 characters.
370: narrative uses a moin header.
429: narrative uses a moin header.
501: narrative uses a moin header.
568: narrative uses a moin header.
576: source exceeds 78 characters.
688: narrative uses a moin header.
878: narrative uses a moin header.
897: narrative uses a moin header.
909: narrative uses a moin header.
--
https://code.launchpad.net/~bac/launchpad/hidden_links/+merge/53911
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~bac/launchpad/hidden_links into lp:launchpad.
=== modified file 'lib/canonical/launchpad/doc/menus.txt'
--- lib/canonical/launchpad/doc/menus.txt 2010-11-07 21:15:07 +0000
+++ lib/canonical/launchpad/doc/menus.txt 2011-03-17 21:21:04 +0000
@@ -192,7 +192,7 @@
... target = '+foo'
... text = 'Foo'
... return Link(target, text)
- ...
+ ...
... def bar(self):
... target = '+bar'
... text = 'Bar'
@@ -215,6 +215,7 @@
--- link foo ---
enabled: True
escapedtext: Foo
+ hidden: False
icon: None
icon_url: None
linked: True
@@ -232,6 +233,7 @@
--- link bar ---
enabled: True
escapedtext: Bar
+ hidden: False
icon: None
icon_url: None
linked: True
@@ -357,6 +359,13 @@
# Clean up our special login.
>>> login(ANONYMOUS)
+A menu item can be marked as hidden even though it is enabled.
+
+ >>> link = Link('z', 'text', 'summary', icon='icon', hidden=True)
+ >>> print ILink(link).render() #doctest: +NORMALIZE_WHITESPACE
+ <a class="menu-link-None sprite icon invisible-link" title="summary"
+ style="line-height: 20px;">text</a>
+
== How do we tell which link from a facetmenu is the selected one? ==
@@ -448,10 +457,10 @@
adapter. For this part of the test, we won't worry about that.
>>> class FooApplicationMenu(ApplicationMenu):
- ...
+ ...
... links = ['first']
... facet = 'foo'
- ...
+ ...
... def first(self):
... target = '+first'
... text = 'First menu'
@@ -473,6 +482,7 @@
--- link first ---
enabled: True
escapedtext: First menu
+ hidden: False
icon: None
icon_url: None
linked: True
@@ -515,9 +525,9 @@
For this part of the test, we won't worry about that.
>>> class MyContextMenu(ContextMenu):
- ...
+ ...
... links = ['first']
- ...
+ ...
... def first(self):
... target = '+firstcontext'
... text = 'First context menu item'
@@ -539,6 +549,7 @@
--- link first ---
enabled: True
escapedtext: First context menu item
+ hidden: False
icon: None
icon_url: None
linked: True
@@ -583,9 +594,9 @@
>>> class FacetsForThing(Facets):
... usedfor = IThingHavingFacets
- ...
+ ...
... links = ['foo', 'bar', 'baz']
- ...
+ ...
... def baz(self):
... target = ''
... text = 'baz'
@@ -694,16 +705,16 @@
>>> from canonical.launchpad.webapp.vhosts import allvhosts
>>> class FakeRequest:
... implements(IHTTPApplicationRequest, IBrowserRequest)
- ...
+ ...
... interaction = None
- ...
+ ...
... def __init__(self, url, query=None, url1=None):
... self.url = url
... self.query = query
... self.url1 = url1 # returned from getURL(1)
... self.method = 'GET'
... self.annotations = {}
- ...
+ ...
... def getURL(self, level=0):
... assert 0 <= level <=1, 'level must be 0 or 1'
... if level == 0:
@@ -712,7 +723,7 @@
... assert self.url1 is not None, (
... 'Must set url1 in FakeRequest')
... return self.url1
- ...
+ ...
... def getRootURL(self, rootsite):
... if rootsite is not None:
... return allvhosts.configs[rootsite].rooturl
@@ -722,14 +733,14 @@
... def getApplicationURL(self):
... # Just the http://place:port part, so stop at the 3rd slash.
... return '/'.join(self.url.split('/', 3)[:3])
- ...
+ ...
... def get(self, key, default=None):
... assert key == 'QUERY_STRING', 'we handle only QUERY_STRING'
... if self.query is None:
... return default
... else:
... return self.query
- ...
+ ...
... def setPrincipal(self, principal):
... self.principal = principal
>>> request = FakeRequest('http://launchpad.dev/sesamestreet/+bar')
@@ -906,7 +917,7 @@
>>> from canonical.launchpad.webapp import enabled_with_permission
>>> class SomeMenu(ContextMenu):
- ...
+ ...
... @enabled_with_permission('launchpad.Admin')
... def foo(self):
... return Link('+admin', 'Admin the foo')
@@ -930,4 +941,3 @@
>>> foolink = somemenu.foo()
>>> foolink.enabled
True
-
=== modified file 'lib/canonical/launchpad/templates/launchpad-inline-link.pt'
--- lib/canonical/launchpad/templates/launchpad-inline-link.pt 2010-08-02 16:36:09 +0000
+++ lib/canonical/launchpad/templates/launchpad-inline-link.pt 2011-03-17 21:21:04 +0000
@@ -2,7 +2,7 @@
xmlns:tal="http://xml.zope.org/namespaces/tal";
condition="context/enabled">
<tal:link-linked
- condition="context/linked"><a
+ condition="python: context.linked and not context.hidden"><a
href="" class="" title=""
tal:condition="context/icon"
tal:attributes="
@@ -22,6 +22,29 @@
"
tal:content="structure context/escapedtext"
/></tal:link-linked>
+ <tal:link-linked-hidden
+ condition="python: context.linked and context.hidden"><a
+ href="" class="" title=""
+ style="line-height: 20px;"
+ tal:condition="context/icon"
+ tal:attributes="
+ class string:menu-link-${context/name} ${view/sprite_class}
+ ${context/icon} invisible-link;
+ href context/url;
+ title context/summary;
+ "
+ tal:content="structure context/escapedtext"
+ /><a
+ tal:condition="not: context/icon"
+ style="display: none"
+ href="" class="" title=""
+ tal:attributes="
+ class string:menu-link-${context/name};
+ href context/url;
+ title context/summary;
+ "
+ tal:content="structure context/escapedtext"
+ /></tal:link-linked-hidden>
<tal:link-not-linked
condition="not: context/linked"><span
tal:condition="context/icon"
=== modified file 'lib/canonical/launchpad/webapp/interfaces.py'
--- lib/canonical/launchpad/webapp/interfaces.py 2011-01-20 23:55:37 +0000
+++ lib/canonical/launchpad/webapp/interfaces.py 2011-03-17 21:21:04 +0000
@@ -187,6 +187,12 @@
sort_key = Attribute(
"The sort key to use when rendering it with a group of links.")
+ hidden = Attribute(
+ "Boolean to say whether this link is hidden. This is separate from "
+ "being enabled and is used to support links which need to be be "
+ "enabled but not viewable in the rendered HTML. The link may be "
+ "changed to visible by JavaScript or some other means.")
+
class ILink(ILinkData):
"""An object that represents a link in a menu.
=== modified file 'lib/canonical/launchpad/webapp/menu.py'
--- lib/canonical/launchpad/webapp/menu.py 2011-03-01 05:00:01 +0000
+++ lib/canonical/launchpad/webapp/menu.py 2011-03-17 21:21:04 +0000
@@ -128,7 +128,7 @@
implements(ILinkData)
def __init__(self, target, text, summary=None, icon=None, enabled=True,
- site=None, menu=None):
+ site=None, menu=None, hidden=False):
"""Create a new link to 'target' with 'text' as the link text.
'target' is a relative path, an absolute path, or an absolute url.
@@ -158,6 +158,7 @@
self.enabled = enabled
self.site = site
self.menu = menu
+ self.hidden = hidden
Link = LinkData
