launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #16475
[Merge] lp:~wgrant/launchpad/kill-facet-link-titles into lp:launchpad
William Grant has proposed merging lp:~wgrant/launchpad/kill-facet-link-titles into lp:launchpad with lp:~wgrant/launchpad/kill-excessive-facets as a prerequisite.
Commit message:
Drop facet link tooltips and clean up the IFacetMenu implementations across the board.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~wgrant/launchpad/kill-facet-link-titles/+merge/208273
Drop a lot of duplicated facet menu code. Set up sane enablement defaults, and selectively disable irrelevant tabs in a more sensible way. Also drop the tooltips, since most of their value was in the amusement factor: across two pages you could see facet tab tooltips of "The Code Bazaar", "View related
code", "Launchpad Answer Tracker", and "Blueprints and specifications", all of which obviously have the same style and aren't a perfect example of siloisation at all.
--
https://code.launchpad.net/~wgrant/launchpad/kill-facet-link-titles/+merge/208273
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wgrant/launchpad/kill-facet-link-titles into lp:launchpad.
=== modified file 'lib/lp/answers/browser/questiontarget.py'
--- lib/lp/answers/browser/questiontarget.py 2014-02-19 00:35:25 +0000
+++ lib/lp/answers/browser/questiontarget.py 2014-02-26 04:54:32 +0000
@@ -16,7 +16,6 @@
'QuestionCollectionNeedAttentionView',
'QuestionCollectionOpenCountView',
'QuestionCollectionAnswersMenu',
- 'QuestionTargetFacetMixin',
'QuestionTargetPortletAnswerContactsWithDetails',
'QuestionTargetTraversalMixin',
'QuestionTargetAnswersMenu',
@@ -886,16 +885,6 @@
return self.answercontact_data_js
-class QuestionTargetFacetMixin:
- """Mixin for questiontarget facet definition."""
-
- def answers(self):
- """Return the link for Answers."""
- summary = (
- 'Questions for %s' % self.context.displayname)
- return Link('', 'Answers', summary)
-
-
class QuestionTargetTraversalMixin:
"""Navigation mixin for IQuestionTarget."""
=== modified file 'lib/lp/app/browser/launchpad.py'
--- lib/lp/app/browser/launchpad.py 2014-02-25 08:51:21 +0000
+++ lib/lp/app/browser/launchpad.py 2014-02-26 04:54:32 +0000
@@ -457,9 +457,14 @@
class LaunchpadRootFacets(StandardLaunchpadFacets):
usedfor = ILaunchpadRoot
-
- enable_only = ['overview', 'bugs', 'answers', 'specifications',
- 'translations', 'branches']
+ enable_only = [
+ 'overview',
+ 'branches',
+ 'bugs',
+ 'specifications',
+ 'translations',
+ 'answers',
+ ]
def overview(self):
target = ''
@@ -479,20 +484,17 @@
def answers(self):
target = ''
text = 'Answers'
- summary = 'Launchpad Answer Tracker'
- return Link(target, text, summary)
+ return Link(target, text)
def specifications(self):
target = ''
text = 'Blueprints'
- summary = 'Launchpad feature specification tracker.'
- return Link(target, text, summary)
+ return Link(target, text)
def branches(self):
target = ''
text = 'Code'
- summary = 'The Code Bazaar'
- return Link(target, text, summary)
+ return Link(target, text)
class LoginStatus:
=== modified file 'lib/lp/blueprints/browser/sprint.py'
--- lib/lp/blueprints/browser/sprint.py 2014-02-26 04:54:32 +0000
+++ lib/lp/blueprints/browser/sprint.py 2014-02-26 04:54:32 +0000
@@ -84,12 +84,10 @@
"""The links that will appear in the facet menu for an ISprint."""
usedfor = ISprint
- enable_only = ['overview', 'specifications']
-
- def specifications(self):
- text = 'Blueprints'
- summary = 'Topics for discussion at %s' % self.context.title
- return Link('', text, summary)
+ enable_only = [
+ 'overview',
+ 'specifications',
+ ]
class SprintNavigation(Navigation):
=== modified file 'lib/lp/bugs/browser/bug.py'
--- lib/lp/bugs/browser/bug.py 2014-02-26 04:54:32 +0000
+++ lib/lp/bugs/browser/bug.py 2014-02-26 04:54:32 +0000
@@ -208,7 +208,7 @@
usedfor = IBug
- enable_only = []
+ enable_only = ['overview']
class BugSetNavigation(Navigation):
=== modified file 'lib/lp/registry/browser/distribution.py'
--- lib/lp/registry/browser/distribution.py 2014-02-26 04:54:32 +0000
+++ lib/lp/registry/browser/distribution.py 2014-02-26 04:54:32 +0000
@@ -52,10 +52,7 @@
from zope.security.interfaces import Unauthorized
from lp.answers.browser.faqtarget import FAQTargetNavigationMixin
-from lp.answers.browser.questiontarget import (
- QuestionTargetFacetMixin,
- QuestionTargetTraversalMixin,
- )
+from lp.answers.browser.questiontarget import QuestionTargetTraversalMixin
from lp.app.browser.launchpadform import (
action,
custom_widget,
@@ -193,24 +190,10 @@
return self.redirectSubTree(canonical_url(distribution))
-class DistributionFacets(QuestionTargetFacetMixin, StandardLaunchpadFacets):
+class DistributionFacets(StandardLaunchpadFacets):
usedfor = IDistribution
- enable_only = [
- 'overview',
- 'branches',
- 'bugs',
- 'answers',
- 'specifications',
- 'translations',
- ]
-
- def specifications(self):
- text = 'Blueprints'
- summary = 'Feature specifications for %s' % self.context.displayname
- return Link('', text, summary)
-
class DistributionSetBreadcrumb(Breadcrumb):
"""Builds a breadcrumb for an `IDistributionSet`."""
=== modified file 'lib/lp/registry/browser/distributionsourcepackage.py'
--- lib/lp/registry/browser/distributionsourcepackage.py 2014-01-14 03:04:55 +0000
+++ lib/lp/registry/browser/distributionsourcepackage.py 2014-02-26 04:54:32 +0000
@@ -33,7 +33,6 @@
from lp.answers.browser.questiontarget import (
QuestionTargetAnswersMenu,
- QuestionTargetFacetMixin,
QuestionTargetTraversalMixin,
)
from lp.answers.enums import QuestionStatus
@@ -114,27 +113,15 @@
self.context.sourcepackagename.name)
-class DistributionSourcePackageFacets(QuestionTargetFacetMixin,
- StandardLaunchpadFacets):
+class DistributionSourcePackageFacets(StandardLaunchpadFacets):
usedfor = IDistributionSourcePackage
- enable_only = ['overview', 'bugs', 'answers', 'branches']
-
- def overview(self):
- text = 'Overview'
- summary = u'General information about {0}'.format(
- self.context.displayname)
- return Link('', text, summary)
-
- def bugs(self):
- text = 'Bugs'
- summary = u'Bugs reported about {0}'.format(self.context.displayname)
- return Link('', text, summary)
-
- def branches(self):
- text = 'Code'
- summary = u'Branches for {0}'.format(self.context.displayname)
- return Link('', text, summary)
+ enable_only = [
+ 'overview',
+ 'branches',
+ 'bugs',
+ 'answers',
+ ]
class DistributionSourcePackageLinksMixin:
=== modified file 'lib/lp/registry/browser/distroseries.py'
--- lib/lp/registry/browser/distroseries.py 2013-02-14 02:02:58 +0000
+++ lib/lp/registry/browser/distroseries.py 2014-02-26 04:54:32 +0000
@@ -207,8 +207,13 @@
class DistroSeriesFacets(StandardLaunchpadFacets):
usedfor = IDistroSeries
- enable_only = ['overview', 'branches', 'bugs', 'specifications',
- 'translations']
+ enable_only = [
+ 'overview',
+ 'branches',
+ 'bugs',
+ 'specifications',
+ 'translations',
+ ]
class DistroSeriesOverviewMenu(
=== modified file 'lib/lp/registry/browser/person.py'
--- lib/lp/registry/browser/person.py 2013-05-02 00:40:14 +0000
+++ lib/lp/registry/browser/person.py 2014-02-26 04:54:32 +0000
@@ -543,46 +543,6 @@
usedfor = IPerson
- enable_only = ['overview', 'bugs', 'answers', 'specifications',
- 'branches', 'translations']
-
- def overview(self):
- text = 'Overview'
- summary = 'General information about %s' % self.context.displayname
- return Link('', text, summary)
-
- def bugs(self):
- text = 'Bugs'
- summary = (
- 'Bug reports that %s is involved with' % self.context.displayname)
- return Link('', text, summary)
-
- def specifications(self):
- text = 'Blueprints'
- summary = (
- 'Feature specifications that %s is involved with' %
- self.context.displayname)
- return Link('', text, summary)
-
- def branches(self):
- text = 'Code'
- summary = ('Bazaar Branches and revisions registered and authored '
- 'by %s' % self.context.displayname)
- return Link('', text, summary)
-
- def answers(self):
- text = 'Answers'
- summary = (
- 'Questions that %s is involved with' % self.context.displayname)
- return Link('', text, summary)
-
- def translations(self):
- text = 'Translations'
- summary = (
- 'Software that %s is involved in translating' %
- self.context.displayname)
- return Link('', text, summary)
-
class CommonMenuLinks:
=== modified file 'lib/lp/registry/browser/personproduct.py'
--- lib/lp/registry/browser/personproduct.py 2012-09-28 06:15:58 +0000
+++ lib/lp/registry/browser/personproduct.py 2014-02-26 04:54:32 +0000
@@ -65,12 +65,4 @@
"""The links that will appear in the facet menu for an IPerson."""
usedfor = IPersonProduct
-
enable_only = ['branches']
-
- def branches(self):
- text = 'Code'
- summary = ('Bazaar Branches of %s owned by %s' %
- (self.context.product.displayname,
- self.context.person.displayname))
- return Link('', text, summary)
=== modified file 'lib/lp/registry/browser/product.py'
--- lib/lp/registry/browser/product.py 2014-02-26 04:54:32 +0000
+++ lib/lp/registry/browser/product.py 2014-02-26 04:54:32 +0000
@@ -74,10 +74,7 @@
from lp import _
from lp.answers.browser.faqtarget import FAQTargetNavigationMixin
-from lp.answers.browser.questiontarget import (
- QuestionTargetFacetMixin,
- QuestionTargetTraversalMixin,
- )
+from lp.answers.browser.questiontarget import QuestionTargetTraversalMixin
from lp.app.browser.launchpadform import (
action,
custom_widget,
@@ -305,41 +302,11 @@
pass
-class ProductFacets(QuestionTargetFacetMixin, StandardLaunchpadFacets):
+class ProductFacets(StandardLaunchpadFacets):
"""The links that will appear in the facet menu for an IProduct."""
usedfor = IProduct
- enable_only = ['overview', 'bugs', 'answers', 'specifications',
- 'translations', 'branches']
-
- links = StandardLaunchpadFacets.links
-
- def overview(self):
- text = 'Overview'
- summary = 'General information about %s' % self.context.displayname
- return Link('', text, summary)
-
- def bugs(self):
- text = 'Bugs'
- summary = 'Bugs reported about %s' % self.context.displayname
- return Link('', text, summary)
-
- def branches(self):
- text = 'Code'
- summary = 'Branches for %s' % self.context.displayname
- return Link('', text, summary)
-
- def specifications(self):
- text = 'Blueprints'
- summary = 'Feature specifications for %s' % self.context.displayname
- return Link('', text, summary)
-
- def translations(self):
- text = 'Translations'
- summary = 'Translations of %s in Launchpad' % self.context.displayname
- return Link('', text, summary)
-
class ProductInvolvementView(PillarInvolvementView):
"""Encourage configuration of involvement links for projects."""
=== modified file 'lib/lp/registry/browser/productseries.py'
--- lib/lp/registry/browser/productseries.py 2013-05-02 00:40:14 +0000
+++ lib/lp/registry/browser/productseries.py 2014-02-26 04:54:32 +0000
@@ -199,11 +199,17 @@
"""A class that provides the series facets."""
usedfor = IProductSeries
enable_only = [
- 'overview', 'branches', 'bugs', 'specifications', 'translations']
+ 'overview',
+ 'branches',
+ 'bugs',
+ 'specifications',
+ 'translations',
+ ]
def branches(self):
"""Return a link to view the branches related to this series."""
- # Override to go to the branches for the product.
+ # XXX wgrant 2014-02-26 bug=183433: Override to go to the
+ # branches for the product. This is inconsistent and weird. Ew.
text = 'Code'
summary = 'View related branches of code'
link = canonical_url(self.context.product, rootsite='code')
=== modified file 'lib/lp/registry/browser/project.py'
--- lib/lp/registry/browser/project.py 2013-04-10 08:09:05 +0000
+++ lib/lp/registry/browser/project.py 2014-02-26 04:54:32 +0000
@@ -45,10 +45,7 @@
from lp import _
from lp.answers.browser.question import QuestionAddView
-from lp.answers.browser.questiontarget import (
- QuestionCollectionAnswersMenu,
- QuestionTargetFacetMixin,
- )
+from lp.answers.browser.questiontarget import QuestionCollectionAnswersMenu
from lp.app.browser.launchpadform import (
action,
custom_widget,
@@ -113,7 +110,10 @@
)
from lp.services.webapp.authorization import check_permission
from lp.services.webapp.breadcrumb import Breadcrumb
-from lp.services.webapp.menu import NavigationMenu
+from lp.services.webapp.menu import (
+ ALL_LINKS,
+ NavigationMenu,
+ )
class ProjectNavigation(Navigation,
@@ -175,41 +175,21 @@
return Link('+all', text, icon='list')
-class ProjectFacets(QuestionTargetFacetMixin, StandardLaunchpadFacets):
+class ProjectFacets(StandardLaunchpadFacets):
"""The links that will appear in the facet menu for an IProjectGroup."""
usedfor = IProjectGroup
- enable_only = ['overview', 'branches', 'bugs', 'specifications',
- 'answers', 'translations']
-
@cachedproperty
def has_products(self):
return self.context.hasProducts()
- def branches(self):
- text = 'Code'
- return Link('', text, enabled=self.has_products)
-
- def bugs(self):
- site = 'bugs'
- text = 'Bugs'
- return Link('', text, enabled=self.has_products, site=site)
-
- def answers(self):
- site = 'answers'
- text = 'Answers'
- return Link('', text, enabled=self.has_products, site=site)
-
- def specifications(self):
- site = 'blueprints'
- text = 'Blueprints'
- return Link('', text, enabled=self.has_products, site=site)
-
- def translations(self):
- site = 'translations'
- text = 'Translations'
- return Link('', text, enabled=self.has_products, site=site)
+ @property
+ def enable_only(self):
+ if not self.has_products:
+ return ['overview']
+ else:
+ return ALL_LINKS
class ProjectAdminMenuMixin:
=== modified file 'lib/lp/registry/browser/sourcepackage.py'
--- lib/lp/registry/browser/sourcepackage.py 2013-10-01 05:54:09 +0000
+++ lib/lp/registry/browser/sourcepackage.py 2014-02-26 04:54:32 +0000
@@ -202,29 +202,12 @@
class SourcePackageFacets(StandardLaunchpadFacets):
usedfor = ISourcePackage
- enable_only = ['overview', 'bugs', 'branches', 'translations']
-
- def overview(self):
- text = 'Overview'
- summary = u'General information about {0}'.format(
- self.context.displayname)
- return Link('', text, summary)
-
- def bugs(self):
- text = 'Bugs'
- summary = u'Bugs reported about {0}'.format(self.context.displayname)
- return Link('', text, summary)
-
- def branches(self):
- text = 'Code'
- summary = u'Branches for {0}'.format(self.context.displayname)
- return Link('', text, summary)
-
- def translations(self):
- text = 'Translations'
- summary = u'Translations of {0} in Launchpad'.format(
- self.context.displayname)
- return Link('', text, summary)
+ enable_only = [
+ 'overview',
+ 'branches',
+ 'bugs',
+ 'translations',
+ ]
class SourcePackageOverviewMenu(ApplicationMenu):
=== modified file 'lib/lp/services/webapp/__init__.py'
--- lib/lp/services/webapp/__init__.py 2012-11-26 08:40:20 +0000
+++ lib/lp/services/webapp/__init__.py 2014-02-26 04:54:32 +0000
@@ -84,10 +84,14 @@
# provide your own 'usedfor' in subclasses.
# usedfor = IWhatever
- links = ['overview', 'branches', 'bugs', 'specifications', 'translations',
- 'answers']
-
- enable_only = ['overview', 'bugs', 'specifications', 'translations']
+ links = [
+ 'overview',
+ 'branches',
+ 'bugs',
+ 'specifications',
+ 'translations',
+ 'answers',
+ ]
defaultlink = 'overview'
@@ -111,29 +115,22 @@
text = 'Overview'
return Link('', text)
+ def branches(self):
+ text = 'Code'
+ return Link('', text)
+
+ def bugs(self):
+ text = 'Bugs'
+ return Link('', text)
+
+ def specifications(self):
+ text = 'Blueprints'
+ return Link('', text)
+
def translations(self):
text = 'Translations'
return Link('', text)
- def bugs(self):
- text = 'Bugs'
- return Link('', text)
-
def answers(self):
- # This facet is visible but unavailable by default.
- # See the enable_only list above.
text = 'Answers'
- summary = 'Launchpad Answer Tracker'
- return Link('', text, summary)
-
- def specifications(self):
- text = 'Blueprints'
- summary = 'Blueprints and specifications'
- return Link('', text, summary)
-
- def branches(self):
- # this is disabled by default, because relatively few objects have
- # branch views
- text = 'Code'
- summary = 'View related code'
- return Link('', text, summary=summary)
+ return Link('', text)
=== modified file 'lib/lp/services/webapp/menu.py'
--- lib/lp/services/webapp/menu.py 2013-02-06 04:22:43 +0000
+++ lib/lp/services/webapp/menu.py 2014-02-26 04:54:32 +0000
@@ -5,6 +5,7 @@
__metaclass__ = type
__all__ = [
+ 'ALL_LINKS',
'enabled_with_permission',
'get_current_view',
'get_facet',
Follow ups
