Code review comment for lp:~edwin-grubbs/launchpad/bug-437184-publishing-history-link

Revision history for this message
Curtis Hovey (sinzui) wrote :

Hi Edwin.

I really like the hanging indent presentation. Thanks for choosing to work on the wrapping issue. I think the CSS needs tuning: http://people.canonical.com/~curtis/clipped-icons.png shows the clipped icons. I am not sure if we need to increase the padding of the <li> or if we need to increase the line-height of the <a>.

style.css is deprecated (We should have started removing it this release, but oopses got in the way). Your changes should be in style-3.0.css. We do not write styles for ids because they are harder to reuse. I like this CSS rule and am sure we can use it elsewhere. Change the rule to work with a class or element. For example, if you use `.side a.sprite` you will fix subscribers lists too! (The 'Launchpad code reviewers from Canonical' subscriber to this review will probably be easier to read if it was a hanging indent.)

Is it possible to change DistributionSourcePackageActionMenu.links to a tuple instead of a list?

review: Needs Fixing (code an ui)

« Back to merge proposal