Merge lp:~allenap/launchpad/ui-convert-bug-activity-bug-412523 into lp:launchpad
- ui-convert-bug-activity-bug-412523
- Merge into devel
| Status: | Merged | ||||
|---|---|---|---|---|---|
| Approved by: | Gavin Panella | ||||
| Approved revision: | no longer in the source branch. | ||||
| Merged at revision: | not available | ||||
| Proposed branch: | lp:~allenap/launchpad/ui-convert-bug-activity-bug-412523 | ||||
| Merge into: | lp:launchpad | ||||
| Diff against target: | None lines | ||||
| To merge this branch: | bzr merge lp:~allenap/launchpad/ui-convert-bug-activity-bug-412523 | ||||
| Related bugs: |
|
| Reviewer | Review Type | Date Requested | Status |
|---|---|---|---|
| Celso Providelo (community) | code | Approve | |
| Michael Nelson (community) | ui | Approve | |
| Martin Albisetti | Pending | ||
|
Review via email:
|
|||
Commit message
Description of the change
| Gavin Panella (allenap) wrote : | # |
| Michael Nelson (michael.nelson) wrote : | # |
Hi Gavin,
Great start! It's a nice simple informational page without actions so a good one to begin with!
I've marked it as needs fixing, just to sort out the portlet divs etc.
> This branch converts bug-activity.pt to the new 3.0 base template according to
> the instructions at
> <https:/
> have to make more changes arising from this review, but that's what I want: it
> is the first bugs template to be converted, so it's also so I can figure out
> the process. Once I understand it better I will split up the work so the whole
> team can get to work on it.
Great idea!
>
> I have before and after screenshots (also linked from the bug report):
>
> http://
>
> http://
Thanks for that! So much easier... and great to enable discussion on the bug too!
I'll respond to your thoughts below, and then summarise more generally at the bottom...
>
> My comments on the changes so far, taken from
> <https:/
>
> > The changes so far have been almost exclusively mechanical. I've
> > chosen the main_only base template, and tried to apply the CSS
> > rules. I can see the following issues with the new page:
> >
> > * "Mozilla Firefox" is repeated in the breadcrumbs and in the
> > location-portlet.
The breadcrumbs will be moving to directly below your h1 heading, and will be less prominent. I personally don't see the repetition as a problem in the context of breadcrumbs (ie. they will always display the path of your traversal by necessity).
> >
> > * The bug activity log has always been shown in bugtask
> > context. The new application tabs block is much more prominent
> > than the old and looks a bit daft in the activity log which is
> > bug-related, not bugtask related.
Hmmm... But the application tabs block is showing the user the related pillar-or-person - in this case the product. So I'm not sure why you think it's confusing in either bug/bugtask context? But I'm not sure I've properly understood your point here.
> >
> >
> > * Repetition of "bug #1" close together.
Yes, but as we chatted, I really think that 'bug #1' link should be part of the breadcrumb...
Firefox > Bug #1
Perhaps you should chat with salgado and find out if it will indeed be included in the new breadcrumb (or if he can ensure that it is). In which case, I'd be for removing it.
> >
> > * The table border butts against the divider at the bottom of the
> > page, and creates the appearance of a double thickness border. A
> > margin-bottom on that table would fix it, but that smells a
> > but. Is there already a way to handle this "nicely"?
I think this is just because you don't have the table in a portlet div. You seem to have just the top-portlet which has the link.
I think you should either have one single top-portlet (my preference), or if you need to leave the link in and you think it should be in a separate portlet, then you'd need to wrap your table in a portlet div too, but I really think you hav...
| Gavin Panella (allenap) wrote : | # |
Thanks for the review Michael!
I've add a BugTask breadcrumb, that just says "Bug #X", and tests in
lp/bugs/
work on breadcrumbs.
> > > * The table border butts against the divider at the bottom of the
> > > page, and creates the appearance of a double thickness border. A
> > > margin-bottom on that table would fix it, but that smells a
> > > but. Is there already a way to handle this "nicely"?
>
> I think this is just because you don't have the table in a portlet div. You
> seem to have just the top-portlet which has the link.
>
> I think you should either have one single top-portlet (my preference), or if
> you need to leave the link in and you think it should be in a separate
> portlet, then you'd need to wrap your table in a portlet div too, but I really
> think you have just one piece of information on that page (the activity log).
I've added a single top-portlet div.
> So, other general thoughts:
> * The date formatting (which beuno mentioned)
Fixed, though I'm not using fmt:datetime because of bug 412963
(fmt:datetime displays date and time in current timezone, even when
UTC only is desired). I've added an XXX to the template.
> * Assuming the breadcrumb will include Bug #1, would it make sense to update
> the H1 to instead include the name of the bug rather than the number (or in
> addition to)?
Yes, the bug title is now in the heading.
> * Given that this page is traversable by other products (and the activity is
> related to other products as you said), I'd expect to see "Other related
> products" or something on this page, when there are some. I realise this
> wouldn't be part of the mechanical update, but it would be great to do. At
> least an XXX for a bug?
This is a weird one. The presence of the product heading doesn't
always make sense at first on the activity log page (or many other
pages that are found below the bug page). Once the breadcrumbs are
moved to below the product heading it will make a bit more sense. But
I'm not sure what value there is in drawing attention to the fact that
this activity log page can be seen in the context of another product;
the main page content will be identical. This is definitely something
the bugs team are going to have to talk about as part of our 3.0
redesign.
I've attached a couple of diffs showing the changes I've made. I had
to merge devel in the middle of my changes because the tree would no
longer build (and the bin/buildout workaround was also not working), I
made some whitespace changes, and I also fixed some lint, none of
which is interesting here.
| 1 | === modified file 'lib/lp/bugs/browser/bugtask.py' | |||
| 2 | --- lib/lp/bugs/browser/bugtask.py 2009-08-07 23:20:33 +0000 | |||
| 3 | +++ lib/lp/bugs/browser/bugtask.py 2009-08-13 10:14:48 +0000 | |||
| 4 | @@ -14,6 +14,7 @@ | |||
| 5 | 14 | 'BugTargetView', | 14 | 'BugTargetView', |
| 6 | 15 | 'BugTaskContextMenu', | 15 | 'BugTaskContextMenu', |
| 7 | 16 | 'BugTaskCreateQuestionView', | 16 | 'BugTaskCreateQuestionView', |
| 8 | 17 | 'BugTaskBreadcrumbBuilder', | ||
| 9 | 17 | 'BugTaskEditView', | 18 | 'BugTaskEditView', |
| 10 | 18 | 'BugTaskExpirableListingView', | 19 | 'BugTaskExpirableListingView', |
| 11 | 19 | 'BugTaskListingItem', | 20 | 'BugTaskListingItem', |
| 12 | @@ -116,6 +117,7 @@ | |||
| 13 | 116 | from lp.registry.interfaces.sourcepackage import ISourcePackage | 117 | from lp.registry.interfaces.sourcepackage import ISourcePackage |
| 14 | 117 | from canonical.launchpad.interfaces.validation import ( | 118 | from canonical.launchpad.interfaces.validation import ( |
| 15 | 118 | valid_upstreamtask, validate_distrotask) | 119 | valid_upstreamtask, validate_distrotask) |
| 16 | 120 | from canonical.launchpad.webapp.breadcrumb import BreadcrumbBuilder | ||
| 17 | 119 | from canonical.launchpad.webapp.interfaces import ( | 121 | from canonical.launchpad.webapp.interfaces import ( |
| 18 | 120 | ILaunchBag, NotFoundError, UnexpectedFormData) | 122 | ILaunchBag, NotFoundError, UnexpectedFormData) |
| 19 | 121 | 123 | ||
| 20 | @@ -3476,3 +3478,12 @@ | |||
| 21 | 3476 | return_dict[key] = cgi.escape(return_dict[key]) | 3478 | return_dict[key] = cgi.escape(return_dict[key]) |
| 22 | 3477 | 3479 | ||
| 23 | 3478 | return "%(old_value)s → %(new_value)s" % return_dict | 3480 | return "%(old_value)s → %(new_value)s" % return_dict |
| 24 | 3481 | |||
| 25 | 3482 | |||
| 26 | 3483 | class BugTaskBreadcrumbBuilder(BreadcrumbBuilder): | ||
| 27 | 3484 | """Builds a breadcrumb for an `IBugTask`.""" | ||
| 28 | 3485 | @property | ||
| 29 | 3486 | def text(self): | ||
| 30 | 3487 | return "%s: %s" % ( | ||
| 31 | 3488 | self.context.bug.displayname, | ||
| 32 | 3489 | self.context.bug.title) | ||
| 33 | 3479 | 3490 | ||
| 34 | === modified file 'lib/lp/bugs/configure.zcml' | |||
| 35 | --- lib/lp/bugs/configure.zcml 2009-07-23 13:44:13 +0000 | |||
| 36 | +++ lib/lp/bugs/configure.zcml 2009-08-13 10:14:48 +0000 | |||
| 37 | @@ -263,6 +263,12 @@ | |||
| 38 | 263 | <adapter | 263 | <adapter |
| 39 | 264 | factory="lp.bugs.browser.bugtask.assignee_renderer" | 264 | factory="lp.bugs.browser.bugtask.assignee_renderer" |
| 40 | 265 | name="assignee"/> | 265 | name="assignee"/> |
| 41 | 266 | <adapter | ||
| 42 | 267 | provides="canonical.launchpad.webapp.interfaces.IBreadcrumbBuilder" | ||
| 43 | 268 | for="lp.bugs.interfaces.bugtask.IBugTask" | ||
| 44 | 269 | factory="lp.bugs.browser.bugtask.BugTaskBreadcrumbBuilder" | ||
| 45 | 270 | permission="zope.Public"/> | ||
| 46 | 271 | |||
| 47 | 266 | 272 | ||
| 48 | 267 | <!-- NullBugTask --> | 273 | <!-- NullBugTask --> |
| 49 | 268 | 274 | ||
| 50 | 269 | 275 | ||
| 51 | === modified file 'lib/lp/bugs/templates/bug-activity.pt' | |||
| 52 | --- lib/lp/bugs/templates/bug-activity.pt 2009-08-12 13:02:26 +0000 | |||
| 53 | +++ lib/lp/bugs/templates/bug-activity.pt 2009-08-13 10:14:13 +0000 | |||
| 54 | @@ -10,10 +10,8 @@ | |||
| 55 | 10 | </div> | 10 | </div> |
| 56 | 11 | 11 | ||
| 57 | 12 | <div metal:fill-slot="main"> | 12 | <div metal:fill-slot="main"> |
| 58 | 13 | <div class="yui-g"> | ||
| 59 | 13 | <div class="top-portlet"> | 14 | <div class="top-portlet"> |
| 60 | 14 | <p tal:content="structure context/fmt:link" /> | ||
| 61 | 15 | </div> | ||
| 62 | 16 | <div class="yui-g"> | ||
| 63 | 17 | <table class="listing"> | 15 | <table class="listing"> |
| 64 | 18 | <thead> | 16 | <thead> |
| 65 | 19 | <tr> | 17 | <tr> |
| 66 | @@ -39,5 +37,6 @@ | |||
| 67 | 39 | </tbody> | 37 | </tbody> |
| 68 | 40 | </table> | 38 | </table> |
| 69 | 41 | </div> | 39 | </div> |
| 70 | 40 | </div> | ||
| 71 | 42 | </div> | 41 | </div> |
| 72 | 43 | </bug-activity> | 42 | </bug-activity> |
| 1 | === modified file 'lib/lp/bugs/browser/bugtask.py' | |||
| 2 | --- lib/lp/bugs/browser/bugtask.py 2009-08-13 10:15:14 +0000 | |||
| 3 | +++ lib/lp/bugs/browser/bugtask.py 2009-08-13 12:41:32 +0000 | |||
| 4 | @@ -3498,6 +3498,4 @@ | |||
| 5 | 3498 | """Builds a breadcrumb for an `IBugTask`.""" | 3498 | """Builds a breadcrumb for an `IBugTask`.""" |
| 6 | 3499 | @property | 3499 | @property |
| 7 | 3500 | def text(self): | 3500 | def text(self): |
| 11 | 3501 | return "%s: %s" % ( | 3501 | return self.context.bug.displayname |
| 9 | 3502 | self.context.bug.displayname, | ||
| 10 | 3503 | self.context.bug.title) | ||
| 12 | 3504 | 3502 | ||
| 13 | === modified file 'lib/lp/bugs/stories/bugs/xx-bug-activity.txt' | |||
| 14 | --- lib/lp/bugs/stories/bugs/xx-bug-activity.txt 2009-08-06 13:51:38 +0000 | |||
| 15 | +++ lib/lp/bugs/stories/bugs/xx-bug-activity.txt 2009-08-13 12:59:41 +0000 | |||
| 16 | @@ -7,16 +7,19 @@ | |||
| 17 | 7 | ... 'mozilla-firefox/+bug/3/+activity') | 7 | ... 'mozilla-firefox/+bug/3/+activity') |
| 18 | 8 | >>> main_content = find_main_content(anon_browser.contents) | 8 | >>> main_content = find_main_content(anon_browser.contents) |
| 19 | 9 | 9 | ||
| 21 | 10 | The page heading contains a link back to the bug page. | 10 | The page contains a link back to the bug page: |
| 22 | 11 | 11 | ||
| 26 | 12 | >>> bug_link = main_content.p.a | 12 | >>> document = find_tag_by_id(anon_browser.contents, 'document') |
| 27 | 13 | >>> print '%s --> %s' % (extract_text(bug_link), bug_link['href']) | 13 | >>> bug_link = document.find( |
| 28 | 14 | Bug #3: Bug Title Test --> /debian/+source/mozilla-firefox/+bug/3 | 14 | ... 'a', href=lambda href: href.endswith('/+bug/3')) |
| 29 | 15 | >>> print '%s\n--> %s' % (extract_text(bug_link), bug_link['href']) | ||
| 30 | 16 | Bug #3 | ||
| 31 | 17 | --> http://bugs.launchpad.dev/debian/+source/mozilla-firefox/+bug/3 | ||
| 32 | 15 | 18 | ||
| 33 | 16 | The title repeats the bug number for clarity. | 19 | The title repeats the bug number for clarity. |
| 34 | 17 | 20 | ||
| 35 | 18 | >>> print extract_text(main_content.h1) | 21 | >>> print extract_text(main_content.h1) |
| 37 | 19 | Activity log for bug #3 | 22 | Activity log for bug #3 - Bug Title Test |
| 38 | 20 | 23 | ||
| 39 | 21 | The activity log itself is presented as a table. | 24 | The activity log itself is presented as a table. |
| 40 | 22 | 25 | ||
| 41 | @@ -27,9 +30,9 @@ | |||
| 42 | 27 | >>> for row in main_content.table('tr'): | 30 | >>> for row in main_content.table('tr'): |
| 43 | 28 | ... print_row(row) | 31 | ... print_row(row) |
| 44 | 29 | Date | Who | What changed | Old value | New value | Message | 32 | Date | Who | What changed | Old value | New value | Message |
| 48 | 30 | 10 Aug 05 16:30 | Sample Person | bug | | | assigned to source ... | 33 | 2005-08-10 16:30:32 | Sample Person | bug | | | assigned to source ... |
| 49 | 31 | 10 Aug 05 16:30 | Sample Person | bug | | | assigned to source ... | 34 | 2005-08-10 16:30:47 | Sample Person | bug | | | assigned to source ... |
| 50 | 32 | 24 Feb 06 21:34 | Foo Bar | mozilla-firefox: statusexplanation | | | | 35 | 2006-02-24 21:34:52 | Foo Bar | mozilla-firefox: statusexplanation | | | |
| 51 | 33 | 36 | ||
| 52 | 34 | 37 | ||
| 53 | 35 | == Activity interleaved with comments == | 38 | == Activity interleaved with comments == |
| 54 | 36 | 39 | ||
| 55 | === modified file 'lib/lp/bugs/templates/bug-activity.pt' | |||
| 56 | --- lib/lp/bugs/templates/bug-activity.pt 2009-08-13 10:14:13 +0000 | |||
| 57 | +++ lib/lp/bugs/templates/bug-activity.pt 2009-08-13 12:59:41 +0000 | |||
| 58 | @@ -6,7 +6,10 @@ | |||
| 59 | 6 | metal:use-macro="view/macro:page/main_only"> | 6 | metal:use-macro="view/macro:page/main_only"> |
| 60 | 7 | 7 | ||
| 61 | 8 | <div metal:fill-slot="heading"> | 8 | <div metal:fill-slot="heading"> |
| 63 | 9 | <h1>Activity log for bug #<span tal:replace="context/bug/id" /></h1> | 9 | <h1> |
| 64 | 10 | Activity log for bug #<span tal:replace="context/bug/id" /> - | ||
| 65 | 11 | <span tal:replace="context/bug/title" /> | ||
| 66 | 12 | </h1> | ||
| 67 | 10 | </div> | 13 | </div> |
| 68 | 11 | 14 | ||
| 69 | 12 | <div metal:fill-slot="main"> | 15 | <div metal:fill-slot="main"> |
| 70 | @@ -25,8 +28,13 @@ | |||
| 71 | 25 | </thead> | 28 | </thead> |
| 72 | 26 | <tbody> | 29 | <tbody> |
| 73 | 27 | <tr tal:repeat="log context/bug/activity"> | 30 | <tr tal:repeat="log context/bug/activity"> |
| 76 | 28 | <td tal:content="python:log.datechanged.strftime('%d %b %y %H:%M')"> | 31 | <tal:comment condition="nothing"> |
| 77 | 29 | 13 Sep 04 14:15 | 32 | XXX: Gavin Panella 2009-08-12 bug=412963: Using strftime() |
| 78 | 33 | here because fmt:datetime changes timezone, even though we | ||
| 79 | 34 | always want to show only UTC. | ||
| 80 | 35 | </tal:comment> | ||
| 81 | 36 | <td tal:content="python:log.datechanged.strftime('%Y-%m-%d %T')"> | ||
| 82 | 37 | 2004-09-24 12:04:43 | ||
| 83 | 30 | </td> | 38 | </td> |
| 84 | 31 | <td><tal:person tal:replace="structure log/person/fmt:link">person</tal:person></td> | 39 | <td><tal:person tal:replace="structure log/person/fmt:link">person</tal:person></td> |
| 85 | 32 | <td tal:content="log/whatchanged">description</td> | 40 | <td tal:content="log/whatchanged">description</td> |
| 86 | 33 | 41 | ||
| 87 | === added file 'lib/lp/bugs/tests/test_breadcrumbs.py' | |||
| 88 | --- lib/lp/bugs/tests/test_breadcrumbs.py 1970-01-01 00:00:00 +0000 | |||
| 89 | +++ lib/lp/bugs/tests/test_breadcrumbs.py 2009-08-13 13:57:47 +0000 | |||
| 90 | @@ -0,0 +1,71 @@ | |||
| 91 | 1 | # Copyright 2009 Canonical Ltd. This software is licensed under the | ||
| 92 | 2 | # GNU Affero General Public License version 3 (see the file LICENSE). | ||
| 93 | 3 | |||
| 94 | 4 | __metaclass__ = type | ||
| 95 | 5 | |||
| 96 | 6 | import unittest | ||
| 97 | 7 | |||
| 98 | 8 | from zope.component import getMultiAdapter | ||
| 99 | 9 | |||
| 100 | 10 | from canonical.lazr.testing.menus import make_fake_request | ||
| 101 | 11 | from canonical.launchpad.webapp.publisher import RootObject | ||
| 102 | 12 | from canonical.testing import DatabaseFunctionalLayer | ||
| 103 | 13 | from lp.testing import TestCaseWithFactory | ||
| 104 | 14 | |||
| 105 | 15 | |||
| 106 | 16 | class TestBugTaskBreadcrumbBuilder(TestCaseWithFactory): | ||
| 107 | 17 | |||
| 108 | 18 | layer = DatabaseFunctionalLayer | ||
| 109 | 19 | |||
| 110 | 20 | def setUp(self): | ||
| 111 | 21 | TestCaseWithFactory.setUp(self) | ||
| 112 | 22 | self.product = self.factory.makeProduct( | ||
| 113 | 23 | name='crumb-tester', displayname="Crumb Tester") | ||
| 114 | 24 | self.bug = self.factory.makeBug(product=self.product) | ||
| 115 | 25 | self.root = RootObject() | ||
| 116 | 26 | |||
| 117 | 27 | def test_bugtask_on_mainsite(self): | ||
| 118 | 28 | request = make_fake_request( | ||
| 119 | 29 | 'http://launchpad.dev/%s/+bug/%d' % ( | ||
| 120 | 30 | self.product.name, self.bug.id), | ||
| 121 | 31 | [self.root, self.product, self.bug.default_bugtask]) | ||
| 122 | 32 | hierarchy = getMultiAdapter((self.root, request), name='+hierarchy') | ||
| 123 | 33 | self.assertEquals( | ||
| 124 | 34 | ['http://launchpad.dev/crumb-tester', | ||
| 125 | 35 | 'http://launchpad.dev/crumb-tester/+bug/%d' % self.bug.id], | ||
| 126 | 36 | [crumb.url for crumb in hierarchy.items()]) | ||
| 127 | 37 | self.assertEquals( | ||
| 128 | 38 | ["Crumb Tester", "Bug #%d" % self.bug.id], | ||
| 129 | 39 | [crumb.text for crumb in hierarchy.items()]) | ||
| 130 | 40 | |||
| 131 | 41 | def test_bugtask_on_vhost(self): | ||
| 132 | 42 | request = make_fake_request( | ||
| 133 | 43 | 'http://bugs.launchpad.dev/%s/+bug/%d' % ( | ||
| 134 | 44 | self.product.name, self.bug.id), | ||
| 135 | 45 | [self.root, self.product, self.bug.default_bugtask]) | ||
| 136 | 46 | hierarchy = getMultiAdapter((self.root, request), name='+hierarchy') | ||
| 137 | 47 | self.assertEquals( | ||
| 138 | 48 | ['http://bugs.launchpad.dev/crumb-tester', | ||
| 139 | 49 | 'http://bugs.launchpad.dev/crumb-tester/+bug/%d' % self.bug.id], | ||
| 140 | 50 | [crumb.url for crumb in hierarchy.items()]) | ||
| 141 | 51 | self.assertEquals( | ||
| 142 | 52 | ["Crumb Tester", "Bug #%d" % self.bug.id], | ||
| 143 | 53 | [crumb.text for crumb in hierarchy.items()]) | ||
| 144 | 54 | |||
| 145 | 55 | def test_bugtask_child_on_vhost(self): | ||
| 146 | 56 | request = make_fake_request( | ||
| 147 | 57 | 'http://bugs.launchpad.dev/%s/+bug/%d/+activity' % ( | ||
| 148 | 58 | self.product.name, self.bug.id), | ||
| 149 | 59 | [self.root, self.product, self.bug.default_bugtask]) | ||
| 150 | 60 | hierarchy = getMultiAdapter((self.root, request), name='+hierarchy') | ||
| 151 | 61 | self.assertEquals( | ||
| 152 | 62 | ['http://bugs.launchpad.dev/crumb-tester', | ||
| 153 | 63 | 'http://bugs.launchpad.dev/crumb-tester/+bug/%d' % self.bug.id], | ||
| 154 | 64 | [crumb.url for crumb in hierarchy.items()]) | ||
| 155 | 65 | self.assertEquals( | ||
| 156 | 66 | ["Crumb Tester", "Bug #%d" % self.bug.id], | ||
| 157 | 67 | [crumb.text for crumb in hierarchy.items()]) | ||
| 158 | 68 | |||
| 159 | 69 | |||
| 160 | 70 | def test_suite(): | ||
| 161 | 71 | return unittest.TestLoader().loadTestsFromName(__name__) | ||
| Gavin Panella (allenap) wrote : | # |
Also see the screenshot of what it looks like now:
| Michael Nelson (michael.nelson) wrote : | # |
> Also see the screenshot of what it looks like now:
>
> http://
Thanks Gavin, for the screenshot and for adding the breadcrumb - looks great!
So, as we discussed, r=me assuming that:
1. The lack of padding between the heading and the table will be fixed by either the placement of the new breadcrumbs or a general css rule to apply padding to tables within portlets (as you've correctly got it in a top-portlet now), and,
2. The table itself will be redesigned as part of the bugs ui work later, and will include a link to the relevant product's bugtask for an activity log entry when it is different from the product displayed in the traversal (and above the app. tabs).
-M.
| Celso Providelo (cprov) wrote : | # |
> === modified file 'lib/lp/
> --- lib/lp/
> +++ lib/lp/
> @@ -14,6 +14,7 @@
> 'BugTargetView',
> 'BugTaskContext
> 'BugTaskCreateQ
> + 'BugTaskBreadcr
> 'BugTaskEditView',
> 'BugTaskExpirab
> 'BugTaskListing
> @@ -45,7 +46,6 @@
> import re
> from simplejson import dumps
> import urllib
> -from urlparse import urlparse, urlunparse
> from operator import attrgetter, itemgetter
>
> from zope import component
> @@ -116,6 +116,7 @@
> from lp.registry.
> from canonical.
> valid_upstreamtask, validate_
> +from canonical.
> from canonical.
> ILaunchBag, NotFoundError, UnexpectedFormData)
>
> @@ -3490,3 +3491,10 @@
> return_dict[key] = cgi.escape(
>
> return "%(old_value)s → %(new_value)s" % return_dict
> +
> +
> +class BugTaskBreadcru
> + """Builds a breadcrumb for an `IBugTask`."""
> + @property
> + def text(self):
> + return self.context.
>
Nice! It will look much nicer when we have the breadcrumbs below the
heading section.
> === modified file 'lib/lp/
> --- lib/lp/
> +++ lib/lp/
> @@ -263,6 +263,12 @@
> <adapter
> factory=
> name="assignee"/>
> + <adapter
> + provides=
> + for="lp.
> + factory=
> + permission=
> +
>
> <!-- NullBugTask -->
>
>
It looks like you have TABs before and after this comment.
> === modified file 'lib/lp/
> --- lib/lp/
> +++ lib/lp/
> @@ -7,16 +7,19 @@
> ... 'mozilla-
> >>> main_content = find_main_
>
> -The page heading contains a link back to the bug page.
> +The page contains a link back to the bug page:
>
> - >>> bug_link = main_content.p.a
> - >>> print '%s --> %s' % (extract_
> - Bug #3: Bug Title Test --> /debian/
> + >>> document = find_tag_
> + >>> bug_link = document.find(
> + ... 'a', href=lambda href: href.endswith(
> + >>> print '%s\n--> %s' % (extract_
> + Bug #3
> + --> http://
>
I wonder if there would be any value in using print_locatio...
| Gavin Panella (allenap) wrote : | # |
On Thu, 13 Aug 2009 17:09:06 -0000
Celso Providelo <email address hidden> wrote:
> Review: Approve
Woohoo! Thank you :)
...
> > === modified file 'lib/lp/
> > --- lib/lp/
> > +++ lib/lp/
> > @@ -263,6 +263,12 @@
> > <adapter
> > factory=
> > name="assignee"/>
> > + <adapter
> > + provides=
> > + for="lp.
> > + factory=
> > + permission=
> > +
> >
> > <!-- NullBugTask -->
> >
> >
>
> It looks like you have TABs before and after this comment.
All of the configure.zcml files generated during the move from c/l to
lp/* are chock full of weird whitespace. I started a utility to clean
this up a while back, but have not finished it. If I fix this comment,
I need to fix all the whitespace in this file, so I'd like to leave it
for now, until I have an automated solution.
>
> > === modified file 'lib/lp/
> > --- lib/lp/
> > +++ lib/lp/
> > @@ -7,16 +7,19 @@
> > ... 'mozilla-
> > >>> main_content = find_main_
> >
> > -The page heading contains a link back to the bug page.
> > +The page contains a link back to the bug page:
> >
> > - >>> bug_link = main_content.p.a
> > - >>> print '%s --> %s' % (extract_
> > - Bug #3: Bug Title Test --> /debian/
> > + >>> document = find_tag_
> > + >>> bug_link = document.find(
> > + ... 'a', href=lambda href: href.endswith(
> > + >>> print '%s\n--> %s' % (extract_
> > + Bug #3
> > + --> http://
> >
>
> I wonder if there would be any value in using print_location() helper
> here. it prints the hierarchy, not exactly the links, but we know the
> links are properly rendered by BreadcrumbBuilder (we have specific
> tests for that).
>
> What do you think ?
I've never used print_location() before, but you're right, it fits the
bill nicely. I've had to modify it to work with the new application
tabs, so maybe you could have a look at the attached diff just to make
sure it's okay?
>
> > The title repeats the bug number for clarity.
> >
> > >>> print extract_
> > - Activity log for bug #3
> > + Activity log for bug #3 - Bug Title Test
> >
> > The activity log itself is presented as a table.
> >
> > @@ -27,9 +30,9 @@
> > >>> for row in main_content.
> > ... print_row(row)
> > Date | Who | What changed | Old value | New value | Message
> > - 10 Aug 05 16:30 | Sample Person | bug | | | assigne...
| 1 | === modified file 'lib/canonical/launchpad/testing/pages.py' | |||
| 2 | --- lib/canonical/launchpad/testing/pages.py 2009-06-25 05:30:52 +0000 | |||
| 3 | +++ lib/canonical/launchpad/testing/pages.py 2009-08-14 09:19:59 +0000 | |||
| 4 | @@ -542,15 +542,26 @@ | |||
| 5 | 542 | def print_location_apps(contents): | 542 | def print_location_apps(contents): |
| 6 | 543 | """Print the application tabs' text and URL.""" | 543 | """Print the application tabs' text and URL.""" |
| 7 | 544 | location_apps = find_tag_by_id(contents, 'lp-apps') | 544 | location_apps = find_tag_by_id(contents, 'lp-apps') |
| 17 | 545 | for tab in location_apps.findAll('span'): | 545 | if location_apps is None: |
| 18 | 546 | tab_text = extract_text(tab) | 546 | location_apps = first_tag_by_class(contents, 'location-portlet') |
| 19 | 547 | if tab['class'].find('active') != -1: | 547 | if location_apps is not None: |
| 20 | 548 | tab_text += ' (selected)' | 548 | location_apps = location_apps.ul.findAll('li') |
| 21 | 549 | if tab.a: | 549 | else: |
| 22 | 550 | link = tab.a['href'] | 550 | location_apps = location_apps.findAll('span') |
| 23 | 551 | else: | 551 | if location_apps is None: |
| 24 | 552 | link = 'not linked' | 552 | print "(Application tabs omitted)" |
| 25 | 553 | print "* %s - %s" % (tab_text, link) | 553 | elif len(location_apps) == 0: |
| 26 | 554 | print "(No application tabs)" | ||
| 27 | 555 | else: | ||
| 28 | 556 | for tab in location_apps: | ||
| 29 | 557 | tab_text = extract_text(tab) | ||
| 30 | 558 | if tab['class'].find('active') != -1: | ||
| 31 | 559 | tab_text += ' (selected)' | ||
| 32 | 560 | if tab.a: | ||
| 33 | 561 | link = tab.a['href'] | ||
| 34 | 562 | else: | ||
| 35 | 563 | link = 'not linked' | ||
| 36 | 564 | print "* %s - %s" % (tab_text, link) | ||
| 37 | 554 | 565 | ||
| 38 | 555 | 566 | ||
| 39 | 556 | def print_tag_with_id(contents, id): | 567 | def print_tag_with_id(contents, id): |
| 40 | 557 | 568 | ||
| 41 | === modified file 'lib/lp/bugs/stories/bugs/xx-bug-activity.txt' | |||
| 42 | --- lib/lp/bugs/stories/bugs/xx-bug-activity.txt 2009-08-13 12:59:41 +0000 | |||
| 43 | +++ lib/lp/bugs/stories/bugs/xx-bug-activity.txt 2009-08-14 09:20:08 +0000 | |||
| 44 | @@ -7,19 +7,20 @@ | |||
| 45 | 7 | ... 'mozilla-firefox/+bug/3/+activity') | 7 | ... 'mozilla-firefox/+bug/3/+activity') |
| 46 | 8 | >>> main_content = find_main_content(anon_browser.contents) | 8 | >>> main_content = find_main_content(anon_browser.contents) |
| 47 | 9 | 9 | ||
| 61 | 10 | The page contains a link back to the bug page: | 10 | |
| 62 | 11 | 11 | The page contains a link back to the bug page in the breadcrumbs and | |
| 63 | 12 | >>> document = find_tag_by_id(anon_browser.contents, 'document') | 12 | the main heading repeats the bug number for clarity: |
| 64 | 13 | >>> bug_link = document.find( | 13 | |
| 65 | 14 | ... 'a', href=lambda href: href.endswith('/+bug/3')) | 14 | >>> print_location(anon_browser.contents) |
| 66 | 15 | >>> print '%s\n--> %s' % (extract_text(bug_link), bug_link['href']) | 15 | Hierarchy: Launchpad > Debian > ?mozilla-firefox? package > Bug #3 |
| 67 | 16 | Bug #3 | 16 | Tabs: |
| 68 | 17 | --> http://bugs.launchpad.dev/debian/+source/mozilla-firefox/+bug/3 | 17 | * Overview - http://launchpad.dev/debian/+source/mozilla-firefox |
| 69 | 18 | 18 | * Code - http://code.launchpad.dev/debian/+source/mozilla-firefox | |
| 70 | 19 | The title repeats the bug number for clarity. | 19 | * Bugs (selected) - http://bugs.launchpad.dev/debian/+source/mozilla-firefox |
| 71 | 20 | 20 | * Blueprints - not linked | |
| 72 | 21 | >>> print extract_text(main_content.h1) | 21 | * Translations - not linked |
| 73 | 22 | Activity log for bug #3 - Bug Title Test | 22 | * Answers - http://answers.launchpad.dev/debian/+source/mozilla-firefox |
| 74 | 23 | Main heading: Activity log for bug #3: Bug Title Test | ||
| 75 | 23 | 24 | ||
| 76 | 24 | The activity log itself is presented as a table. | 25 | The activity log itself is presented as a table. |
| 77 | 25 | 26 | ||
| 78 | 26 | 27 | ||
| 79 | === modified file 'lib/lp/bugs/templates/bug-activity.pt' | |||
| 80 | --- lib/lp/bugs/templates/bug-activity.pt 2009-08-13 14:06:31 +0000 | |||
| 81 | +++ lib/lp/bugs/templates/bug-activity.pt 2009-08-13 14:25:35 +0000 | |||
| 82 | @@ -7,7 +7,7 @@ | |||
| 83 | 7 | 7 | ||
| 84 | 8 | <div metal:fill-slot="heading"> | 8 | <div metal:fill-slot="heading"> |
| 85 | 9 | <h1> | 9 | <h1> |
| 87 | 10 | Activity log for bug #<span tal:replace="context/bug/id" /> - | 10 | Activity log for bug #<span tal:replace="context/bug/id" />: |
| 88 | 11 | <span tal:replace="context/bug/title" /> | 11 | <span tal:replace="context/bug/title" /> |
| 89 | 12 | </h1> | 12 | </h1> |
| 90 | 13 | </div> | 13 | </div> |
| Gavin Panella (allenap) wrote : | # |
Several tests broke, and one of them required quite a lot of work to
fix. Basically, breadcrumbs were still being generated when viewing a
bug that the user did not have permission to view. I've added a test to
test_breadcrumbs and changed the adapter to cope with this scenario.
I've attached an incremental diff.
Thanks in advance.
| 1 | === modified file 'lib/lp/bugs/browser/bugtask.py' | |||
| 2 | --- lib/lp/bugs/browser/bugtask.py 2009-08-13 14:07:18 +0000 | |||
| 3 | +++ lib/lp/bugs/browser/bugtask.py 2009-08-14 16:30:27 +0000 | |||
| 4 | @@ -62,6 +62,7 @@ | |||
| 5 | 62 | from zope.schema.interfaces import IContextSourceBinder, IList | 62 | from zope.schema.interfaces import IContextSourceBinder, IList |
| 6 | 63 | from zope.schema.vocabulary import ( | 63 | from zope.schema.vocabulary import ( |
| 7 | 64 | getVocabularyRegistry, SimpleVocabulary, SimpleTerm) | 64 | getVocabularyRegistry, SimpleVocabulary, SimpleTerm) |
| 8 | 65 | from zope.security.interfaces import Unauthorized | ||
| 9 | 65 | from zope.security.proxy import ( | 66 | from zope.security.proxy import ( |
| 10 | 66 | isinstance as zope_isinstance, removeSecurityProxy) | 67 | isinstance as zope_isinstance, removeSecurityProxy) |
| 11 | 67 | from zope.traversing.interfaces import IPathAdapter | 68 | from zope.traversing.interfaces import IPathAdapter |
| 12 | @@ -3495,6 +3496,18 @@ | |||
| 13 | 3495 | 3496 | ||
| 14 | 3496 | class BugTaskBreadcrumbBuilder(BreadcrumbBuilder): | 3497 | class BugTaskBreadcrumbBuilder(BreadcrumbBuilder): |
| 15 | 3497 | """Builds a breadcrumb for an `IBugTask`.""" | 3498 | """Builds a breadcrumb for an `IBugTask`.""" |
| 16 | 3499 | |||
| 17 | 3498 | @property | 3500 | @property |
| 18 | 3499 | def text(self): | 3501 | def text(self): |
| 19 | 3500 | return self.context.bug.displayname | 3502 | return self.context.bug.displayname |
| 20 | 3503 | |||
| 21 | 3504 | def make_breadcrumb(self): | ||
| 22 | 3505 | """Return a breadcrumb for this `BugTask`. | ||
| 23 | 3506 | |||
| 24 | 3507 | If the user does not have permission to view the bug for | ||
| 25 | 3508 | whatever reason, don't return a breadcrumb. | ||
| 26 | 3509 | """ | ||
| 27 | 3510 | try: | ||
| 28 | 3511 | return super(BugTaskBreadcrumbBuilder, self).make_breadcrumb() | ||
| 29 | 3512 | except Unauthorized: | ||
| 30 | 3513 | return None | ||
| 31 | 3501 | 3514 | ||
| 32 | === modified file 'lib/lp/bugs/stories/bug-release-management/nomination-navigation.txt' | |||
| 33 | --- lib/lp/bugs/stories/bug-release-management/nomination-navigation.txt 2009-07-01 13:16:44 +0000 | |||
| 34 | +++ lib/lp/bugs/stories/bug-release-management/nomination-navigation.txt 2009-08-14 14:44:07 +0000 | |||
| 35 | @@ -11,7 +11,7 @@ | |||
| 36 | 11 | ... 'http://bugs.launchpad.dev/ubuntu/+source/mozilla-firefox/+bug/1' | 11 | ... 'http://bugs.launchpad.dev/ubuntu/+source/mozilla-firefox/+bug/1' |
| 37 | 12 | ... '/nominations/2/+editstatus') | 12 | ... '/nominations/2/+editstatus') |
| 38 | 13 | >>> print_location(admin_browser.contents) | 13 | >>> print_location(admin_browser.contents) |
| 40 | 14 | Hierarchy: Launchpad > Ubuntu > ?mozilla-firefox? package | 14 | Hierarchy: Launchpad > Ubuntu > ?mozilla-firefox? package > Bug #1 |
| 41 | 15 | Tabs: | 15 | Tabs: |
| 42 | 16 | * Overview - http://launchpad.dev/ubuntu/+source/mozilla-firefox | 16 | * Overview - http://launchpad.dev/ubuntu/+source/mozilla-firefox |
| 43 | 17 | * Code - http://code.launchpad.dev/ubuntu/+source/mozilla-firefox | 17 | * Code - http://code.launchpad.dev/ubuntu/+source/mozilla-firefox |
| 44 | 18 | 18 | ||
| 45 | === modified file 'lib/lp/bugs/stories/bugs/xx-bug-index.txt' | |||
| 46 | --- lib/lp/bugs/stories/bugs/xx-bug-index.txt 2009-07-31 08:12:47 +0000 | |||
| 47 | +++ lib/lp/bugs/stories/bugs/xx-bug-index.txt 2009-08-14 15:11:58 +0000 | |||
| 48 | @@ -10,9 +10,25 @@ | |||
| 49 | 10 | >>> anon_browser.title | 10 | >>> anon_browser.title |
| 50 | 11 | '...Blackhole Trash folder...' | 11 | '...Blackhole Trash folder...' |
| 51 | 12 | 12 | ||
| 55 | 13 | The bug id in the upper left corner of the page is linked to the bug itself. | 13 | The breadcrumbs and other navigation include a link to the bug itself. |
| 56 | 14 | 14 | ||
| 57 | 15 | >>> anon_browser.getLink('Bug #2').click() | 15 | >>> print_location(anon_browser.contents) |
| 58 | 16 | Hierarchy: Launchpad > Debian > ?mozilla-firefox? package > Bug #2 (blackhole) | ||
| 59 | 17 | Tabs: | ||
| 60 | 18 | * Overview - http://launchpad.dev/debian/+source/mozilla-firefox | ||
| 61 | 19 | * Code - http://code.launchpad.dev/debian/+source/mozilla-firefox | ||
| 62 | 20 | * Bugs (selected) - http://bugs.launchpad.dev/debian/+source/mozilla-firefox | ||
| 63 | 21 | * Blueprints - not linked | ||
| 64 | 22 | * Translations - not linked | ||
| 65 | 23 | * Answers - http://answers.launchpad.dev/debian/+source/mozilla-firefox | ||
| 66 | 24 | Main heading: Blackhole Trash folder | ||
| 67 | 25 | |||
| 68 | 26 | The bug id in the upper left corner of the main page content is also | ||
| 69 | 27 | linked to the bug itself. | ||
| 70 | 28 | |||
| 71 | 29 | >>> bug_link = first_tag_by_class( | ||
| 72 | 30 | ... anon_browser.contents, 'object identifier').a | ||
| 73 | 31 | >>> anon_browser.open(bug_link.get('href')) | ||
| 74 | 16 | >>> print anon_browser.title | 32 | >>> print anon_browser.title |
| 75 | 17 | Bug #2 in Tomcat:...Blackhole Trash folder... | 33 | Bug #2 in Tomcat:...Blackhole Trash folder... |
| 76 | 18 | 34 | ||
| 77 | 19 | 35 | ||
| 78 | === modified file 'lib/lp/bugs/tests/test_breadcrumbs.py' | |||
| 79 | --- lib/lp/bugs/tests/test_breadcrumbs.py 2009-08-13 13:57:47 +0000 | |||
| 80 | +++ lib/lp/bugs/tests/test_breadcrumbs.py 2009-08-14 16:32:02 +0000 | |||
| 81 | @@ -10,7 +10,8 @@ | |||
| 82 | 10 | from canonical.lazr.testing.menus import make_fake_request | 10 | from canonical.lazr.testing.menus import make_fake_request |
| 83 | 11 | from canonical.launchpad.webapp.publisher import RootObject | 11 | from canonical.launchpad.webapp.publisher import RootObject |
| 84 | 12 | from canonical.testing import DatabaseFunctionalLayer | 12 | from canonical.testing import DatabaseFunctionalLayer |
| 86 | 13 | from lp.testing import TestCaseWithFactory | 13 | from lp.testing import ( |
| 87 | 14 | ANONYMOUS, TestCaseWithFactory, login) | ||
| 88 | 14 | 15 | ||
| 89 | 15 | 16 | ||
| 90 | 16 | class TestBugTaskBreadcrumbBuilder(TestCaseWithFactory): | 17 | class TestBugTaskBreadcrumbBuilder(TestCaseWithFactory): |
| 91 | @@ -18,7 +19,7 @@ | |||
| 92 | 18 | layer = DatabaseFunctionalLayer | 19 | layer = DatabaseFunctionalLayer |
| 93 | 19 | 20 | ||
| 94 | 20 | def setUp(self): | 21 | def setUp(self): |
| 96 | 21 | TestCaseWithFactory.setUp(self) | 22 | super(TestBugTaskBreadcrumbBuilder, self).setUp() |
| 97 | 22 | self.product = self.factory.makeProduct( | 23 | self.product = self.factory.makeProduct( |
| 98 | 23 | name='crumb-tester', displayname="Crumb Tester") | 24 | name='crumb-tester', displayname="Crumb Tester") |
| 99 | 24 | self.bug = self.factory.makeBug(product=self.product) | 25 | self.bug = self.factory.makeBug(product=self.product) |
| 100 | @@ -66,6 +67,23 @@ | |||
| 101 | 66 | ["Crumb Tester", "Bug #%d" % self.bug.id], | 67 | ["Crumb Tester", "Bug #%d" % self.bug.id], |
| 102 | 67 | [crumb.text for crumb in hierarchy.items()]) | 68 | [crumb.text for crumb in hierarchy.items()]) |
| 103 | 68 | 69 | ||
| 104 | 70 | def test_bugtask_private_bug(self): | ||
| 105 | 71 | # A breadcrumb is not generated for a bug that the user does | ||
| 106 | 72 | # not have permission to view. | ||
| 107 | 73 | login('foo.bar@canonical.com') | ||
| 108 | 74 | self.bug.setPrivate(True, self.bug.owner) | ||
| 109 | 75 | login(ANONYMOUS) | ||
| 110 | 76 | request = make_fake_request( | ||
| 111 | 77 | 'http://bugs.launchpad.dev/%s/+bug/%d/+activity' % ( | ||
| 112 | 78 | self.product.name, self.bug.id), | ||
| 113 | 79 | [self.root, self.product, self.bug.default_bugtask]) | ||
| 114 | 80 | hierarchy = getMultiAdapter((self.root, request), name='+hierarchy') | ||
| 115 | 81 | self.assertEquals( | ||
| 116 | 82 | ['http://bugs.launchpad.dev/crumb-tester'], | ||
| 117 | 83 | [crumb.url for crumb in hierarchy.items()]) | ||
| 118 | 84 | self.assertEquals( | ||
| 119 | 85 | ["Crumb Tester"], [crumb.text for crumb in hierarchy.items()]) | ||
| 120 | 86 | |||
| 121 | 69 | 87 | ||
| 122 | 70 | def test_suite(): | 88 | def test_suite(): |
| 123 | 71 | return unittest.TestLoader().loadTestsFromName(__name__) | 89 | return unittest.TestLoader().loadTestsFromName(__name__) |
| Celso Providelo (cprov) wrote : | # |
> Several tests broke, and one of them required quite a lot of work to
> fix. Basically, breadcrumbs were still being generated when viewing a
> bug that the user did not have permission to view. I've added a test to
> test_breadcrumbs and changed the adapter to cope with this scenario.
>
> I've attached an incremental diff.
>
> Thanks in advance.
Gavin.
I'm happy with the way you fixed breadcrumbs for private bugs, thanks for catching this problem before landing.
Regarding the previous diff, thanks for fixing print_location() and you are right that having 'UTC' repeated on each row would be bad ... the bug report is enough to sort this problem in the future.
All good!
Preview Diff
| 1 | === modified file 'lib/lp/bugs/templates/bug-activity.pt' | |||
| 2 | --- lib/lp/bugs/templates/bug-activity.pt 2009-07-17 17:59:07 +0000 | |||
| 3 | +++ lib/lp/bugs/templates/bug-activity.pt 2009-08-12 13:02:26 +0000 | |||
| 4 | @@ -1,20 +1,19 @@ | |||
| 17 | 1 | <html | 1 | <bug-activity |
| 18 | 2 | xmlns="http://www.w3.org/1999/xhtml" | 2 | xmlns="http://www.w3.org/1999/xhtml" |
| 19 | 3 | xmlns:tal="http://xml.zope.org/namespaces/tal" | 3 | xmlns:tal="http://xml.zope.org/namespaces/tal" |
| 20 | 4 | xmlns:metal="http://xml.zope.org/namespaces/metal" | 4 | xmlns:metal="http://xml.zope.org/namespaces/metal" |
| 21 | 5 | xmlns:i18n="http://xml.zope.org/namespaces/i18n" | 5 | xmlns:i18n="http://xml.zope.org/namespaces/i18n" |
| 22 | 6 | xml:lang="en" | 6 | metal:use-macro="view/macro:page/main_only"> |
| 11 | 7 | lang="en" | ||
| 12 | 8 | dir="ltr" | ||
| 13 | 9 | metal:use-macro="view/macro:page/onecolumn" | ||
| 14 | 10 | > | ||
| 15 | 11 | <body> | ||
| 16 | 12 | <div metal:fill-slot="main"> | ||
| 23 | 13 | 7 | ||
| 26 | 14 | <div id="maincontent"> | 8 | <div metal:fill-slot="heading"> |
| 25 | 15 | <p tal:content="structure view/current_bugtask/fmt:link" /> | ||
| 27 | 16 | <h1>Activity log for bug #<span tal:replace="context/bug/id" /></h1> | 9 | <h1>Activity log for bug #<span tal:replace="context/bug/id" /></h1> |
| 28 | 10 | </div> | ||
| 29 | 17 | 11 | ||
| 30 | 12 | <div metal:fill-slot="main"> | ||
| 31 | 13 | <div class="top-portlet"> | ||
| 32 | 14 | <p tal:content="structure context/fmt:link" /> | ||
| 33 | 15 | </div> | ||
| 34 | 16 | <div class="yui-g"> | ||
| 35 | 18 | <table class="listing"> | 17 | <table class="listing"> |
| 36 | 19 | <thead> | 18 | <thead> |
| 37 | 20 | <tr> | 19 | <tr> |
| 38 | @@ -39,9 +38,6 @@ | |||
| 39 | 39 | </tr> | 38 | </tr> |
| 40 | 40 | </tbody> | 39 | </tbody> |
| 41 | 41 | </table> | 40 | </table> |
| 42 | 41 | </div> | ||
| 43 | 42 | </div> | 42 | </div> |
| 49 | 43 | 43 | </bug-activity> | |
| 45 | 44 | </div> | ||
| 46 | 45 | |||
| 47 | 46 | </body> | ||
| 48 | 47 | </html> | ||
This branch converts bug-activity.pt to the new 3.0 base template according to the instructions at <https:/
I have before and after screenshots (also linked from the bug report):
http://
http://
My comments on the changes so far, taken from <https:/
> The changes so far have been almost exclusively mechanical. I've
> chosen the main_only base template, and tried to apply the CSS
> rules. I can see the following issues with the new page:
>
> * "Mozilla Firefox" is repeated in the breadcrumbs and in the
> location-portlet.
>
> * The bug activity log has always been shown in bugtask
> context. The new application tabs block is much more prominent
> than the old and looks a bit daft in the activity log which is
> bug-related, not bugtask related.
>
>
> * Repetition of "bug #1" close together.
>
> * The table border butts against the divider at the bottom of the
> page, and creates the appearance of a double thickness border. A
> margin-bottom on that table would fix it, but that smells a
> but. Is there already a way to handle this "nicely"?
Test: ./bin/test -vvt xx-bug-activity
Lint free.