Merge lp:~rbanffy/launchpad/highlight_listing_tr into lp:launchpad

Proposed by Ricardo Bánffy on 2015-05-11
Status: Needs review
Proposed branch: lp:~rbanffy/launchpad/highlight_listing_tr
Merge into: lp:launchpad
Diff against target: 14 lines (+4/-0)
1 file modified
lib/canonical/launchpad/icing/css/base.css (+4/-0)
To merge this branch: bzr merge lp:~rbanffy/launchpad/highlight_listing_tr
Reviewer Review Type Date Requested Status
Ricardo Bánffy (community) Resubmit on 2016-03-18
Colin Watson 2015-05-11 Needs Fixing on 2015-05-12
Review via email: mp+258766@code.launchpad.net

Description of the Change

Makes slight changes to the Launchpad web interface styles to make the site slightly easier to use and navigate. Some parte look strange. The initial motivation was to make listings (such as https://code.launchpad.net/maas/+activereviews) easier to read with alternating backgrounds and line hovers.

To post a comment you must log in.
Colin Watson (cjwatson) wrote :

Please set a commit message for your merge proposal; the landing machinery requires this. Also, it's a good idea with UI branches to give some example pages that highlight your change. In the absence of one I looked at https://code.launchpad.dev/gnome-terminal, but you may have a better example.

This doesn't seem to quite do what you want. The way that descendant selectors work mean that what you're doing here is selecting a tr element inside another tr. This works better:

  table.listing tbody tr:not(.thead):hover

But not(.thead) is weird inside tbody; and in any case tables aren't obliged to have an explicit tbody element. Perhaps what you meant is more like this?

  table.listing tr:not(.thead):hover

The contrast between white and lightgrey feels a bit too sharp and jarring to me, and I wonder if there's a shade in between those two that we could pick, preferably one already in use in Launchpad. For selected rows in the official tags form, we use #eee, and that feels much nicer to me here.

There are some views where this looks distinctly weird. For example, in a development instance with sampledata, try https://launchpad.dev/ubuntu/+source/evolution and hover over the various rows in the table at the bottom. Can we find a way not to highlight the blank rows?

review: Needs Fixing
Ricardo Bánffy (rbanffy) wrote :

Used a CSS selector identical to launchpad_tweaks. If affects pages in the "Code" section.

review: Resubmit

Unmerged revisions

17486. By Ricardo Bánffy on 2016-03-18

Apply cjwatson's suggestion and use the CSS selector that's used in launchpad_tweaks

17485. By Ricardo Bánffy on 2016-03-18

merge trunk

17484. By Ricardo Bánffy on 2015-05-07

Narrow down CSS selector

17483. By Ricardo Bánffy on 2015-05-07

Highlight the listing row where the pointer is so it's easier to navigate long listings

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/canonical/launchpad/icing/css/base.css'
2--- lib/canonical/launchpad/icing/css/base.css 2015-03-24 11:30:04 +0000
3+++ lib/canonical/launchpad/icing/css/base.css 2016-03-18 20:02:02 +0000
4@@ -402,6 +402,10 @@
5 border: none;
6 }
7
8+table.listing tbody tr:hover {
9+ background: #eee;
10+ }
11+
12 table.listing tfoot td {
13 border: 1px solid #d2d2d2;
14 }