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 > . I am expecting to > 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://launchpadlibrarian.net/30223879/before-mechanical-changes.png > > http://launchpadlibrarian.net/30223898/after-mechanical-changes.png 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 > : > > > 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 have just one piece of information on that page (the activity log). > > Test: ./bin/test -vvt xx-bug-activity > > Lint free. So, other general thoughts: * The date formatting (which beuno mentioned) * 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)? * 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? OK, sorry I can't be more thorough, but I literally need to run! Look forward to seeing your thoughts. allenap: I like the way you're doing the whole process through the bug... allowing anyone to comment as you go :) noodles775: Release early :) Is there a reason why you put the para with the formatted link in a separate portlet? ie. why not just have one portlet with the formatted link followed by the table? * matsubara-lunch has quit (Read error: 110 (Connection timed out)) noodles775: I got the impression from the wiki page that I should do something like that. The link isn't directly related to the table; it doesn't describe the table, or alter it, so it sort-of seemed like a distinct piece of content. allenap: actually, I don't think you need the yui-g div either, as you're not using any grid here (it's only needed if you've got child-divs that you want in a column layout afaik). Hmm. Yep, I can see why you'd say that, but that should also then mean that the table should be in a portlet div (but that woudl put a horizontal line between them right?) * deryck is now known as deryck[lunch] noodles775: Would it? Ah, so portlet divs contain yui-u and yui-g divs? Other way around, if you are using a grid layout then your portlet div will be a *child* of the yui-u div. (That was a recent change aiui, so that portlets could be re-usable on different pages without inheriting the yui layout) noodles775: Right, so something like: div.yui-g > div.top-portlet > (link and table) allenap: yes, although I've just realised what that link actually is... it really should be in the breadcrumbs right? salgado: beuno: I approved the projectgroup page. There are some minor things that can be fixed, but I think we should land it today and file bugs about issues that can be fixed after the distribution layout is updated. noodles775: I guess so. Something like: Mozilla Firefox > Bug #1234 > Activity Log -- even though the Mozilla Firefox bit is a bit smelly. Bug #1234: Gina is an unmaintainable mess of command line options, environment variables and shell scripts sinzui, woooooooooooooooooooo allenap: Yeah, exactly. Why do you think that including the product as the base of the breadcrumbs is smelly? noodles775: When there's more than one bug task it implies (to me) that the activity is relevant only to that product, but it's actually bug activity for all bug tasks. allenap, the date is also formatted in a way that it isn't elsewhere beuno: Ah, good spot. I'll use a standard formatter. allenap: Sure, but doesn't that mean that the same activity log is traversable via a different url? noodles775: It is :) So I think that's fine then for the breadcrumb - it's representing the path to how the user got here, but given what you said, as a user I would then also expect to see other products affected here (if there are any?) (but that wouldn't be part of a mechanical change :) ). well, mechanical changes are a loose term :) lol OK, I'll summarise in the MP my other thoughts so I can see both yours and beuno's feedback (as I have to go soon).