Comment 23 for bug 1635386

Revision history for this message
Dan Wells (dbw2) wrote :

Okay, I have reviewed this branch, and overall, it is very close to ready. I have one small thing, one maybe thing, and one potentially larger thing to mention.

1) (small) There is still a stray 'statusicon' test in one template. I am wondering if we should fix that, or get rid of that test/style entirely (not 100% sure why we need the custom class for that column yet).

2) (maybe) As shown in Kathy's screenshot, we are attempting to "darken" rows when selected, but this gets pretty hairy fast, with no less than 4 different potential shades of red (and 2 shades of orange, for that matter). At that stage, it is hard to argue that the colors remain functional. What do folks feel about using the blue select color globally regardless of original row color? It is easy to visually understand and will work everywhere forever. In my view, if I've selected something, I've already evaluated it, and it is more important from that point forward to see what I've selected than to see a color hint about the row data. Thoughts? We could also consider fancier forms of universal highlight (border-color, glow, shadow, etc.), but we aren't close to there yet.

3) (potentially larger) This branch is going to conflict and interact with bug #1746824 in interesting ways. I *think* we should consider keeping both, as one applies a class across entire rows based on actual row data, while the other applies classes to individual columns based on the general shape of the data class. Do folks agree with this summary? If so, we may want to tweak the two variable names to clarify this complementary relationship.