-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Infra: Mute table zebra stripes to de-emphasise different rows #2740
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
I don't disagree, though I don't think it looks that much more muted than |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Implementation details look good, but personally I find #111111
almost invisible on non-IPS monitors at non-optimal viewing angles, which also raises a11y concerns, whereas #222222
is much easier to spot while not being as dark.
Now that I'm at home on my personal IPS monitors |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks @hugovk
@gvanrossum n co, any objections here? |
To me, any zebra striping, no matter how muted, looks wrong. This may be a US/Europe thing? I remember looong ago coming across zebra stripes in some US document and being utterly confused by it. But I seem to be in a minority of one, so I'll stop resisting. Just don't ask me to decide between |
Not a minority of one, but I don't have much of an argument to give, other than that zebra striping can be extremely confusing when there's any possibility that the table will have three rows in it. |
Hmm, I dunno then. In general maybe tables are not the appropriate construct when you only have three rows? But we certainly don't want to be ambiguous or confusing, especially to our international audience. Maybe add back the horizontal rules for contrast and just use the lightest colors such that they don't look content-meaningful? @gvanrossum does even |
I prefer zebra stripes, and I'm European. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Based on what people have said, what about this?
- Add back the horizontal rules separating the rows
- Mute both the light and dark colors (
color-background-accent-light
) all the way to#dddddd
and#111111
respectively - Add a
:hover
on the rows ofcolor-background-accent-strong
- Also, add
color-background-accent-strong
to more clearly delineate the heading (as my eye has a hard time picking out the bold now)
This gives just enough static zebra contrast to guide the eye while barely being consciously visible, adds the higher-contrast horizontal rules to clearly delineate the rows themselves, and highlights the currently hovered row with a higher-contrast color, plus makes the headers easier to see.
Here's how that looks:
![]() | ![]() |
@@ -33,7 +33,8 @@ | |||
/* Set master colours */ | |||
:root { | |||
--colour-background: var(--light, white) var(--dark, #011); | |||
--colour-background-accent: var(--light, #ccc) var(--dark, #333); | |||
--colour-background-accent-strong: var(--light, #ccc) var(--dark, #333); | |||
--colour-background-accent-light: var(--light, #ddd) var(--dark, #222); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
--colour-background-accent-light: var(--light, #ddd) var(--dark, #222); | |
--colour-background-accent-light: var(--light, #eee) var(--dark, #111); |
Mute the colors as much as practical (since we're adding back the lines to clearly delineate the rows)
border-top: 1px solid var(--colour-background-accent-light); | ||
border-bottom: 1px solid var(--colour-background-accent-light); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
border-top: 1px solid var(--colour-background-accent-light); | |
border-bottom: 1px solid var(--colour-background-accent-light); | |
border-top: 1px solid var(--colour-background-accent-strong); | |
border-bottom: 1px solid var(--colour-background-accent-strong); |
To be consistent with the current and the lines
border-top: 1px solid var(--colour-background-accent); | ||
border-bottom: 1px solid var(--colour-background-accent); | ||
border-top: 1px solid var(--colour-background-accent-light); | ||
border-bottom: 1px solid var(--colour-background-accent-light); | ||
} | ||
table caption { | ||
margin: 1rem 0 .75rem; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
} | |
} | |
table tr { | |
border-bottom: 1px solid var(--colour-rule-strong); | |
} | |
table thead tr { | |
background-color: var(--colour-background-accent-strong); | |
} |
Add horizontal rules and clearly delineate header row
} | ||
table caption { | ||
margin: 1rem 0 .75rem; | ||
} | ||
table tbody tr:nth-of-type(odd) { | ||
background-color: var(--colour-background-accent); | ||
background-color: var(--colour-background-accent-light); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
} | |
} | |
table tbody tr:hover { | |
background-color: var(--colour-background-accent-strong); | |
} |
Add "highlight" on hover
border-top: 1px solid var(--colour-background-accent); | ||
border-bottom: 1px solid var(--colour-background-accent); | ||
border-top: 1px solid var(--colour-background-accent-light); | ||
border-bottom: 1px solid var(--colour-background-accent-light); | ||
} | ||
table caption { | ||
margin: 1rem 0 .75rem; | ||
} | ||
table tbody tr:nth-of-type(odd) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
table tbody tr:nth-of-type(odd) { | |
table tbody tr:nth-of-type(even) { |
Switch this to even, since it looks better with the revised colors
Since I keep getting CC'ed on zebra stripe issues, let me reiterate once more that I think they are distracting and confusing, and moreover unnecessary. IIRC the research quoted that "proved" that zebra stripes have a usability benefit was using tables that were much wider, with much less (no) vertical spacing. The text design we are using for PEPs doesn't have those problems -- in the example given above (Tier 3 status) there is absolutely no concern about misreading the table even when you're pressed for time. And the hover feature is just obnoxious. Let's reserve background highlighting for table headings and subheadings. Look at this table in the Library Reference and tell me whether it needs zebra strips for usability: https://docs.python.org/3/library/stdtypes.html#numeric-types-int-float-complex |
While I agree that zebras are a suboptimal choice, I don't think anyone has stated that this change is not an improvement over status quo as-is. Let's land this, instead of blocking this on concensus over zebra, adding additional borders etc. We iterate on the other sylistic stuff in a follow up. |
To merge this, the latest review needs to be addressed by the PR author (suggestions accepted or rejected). |
Yeah, AFAIK I don't think anyone disagrees the status quo is not good and this is at least less bad than that. Muted zebra stripes are the status quo (#2691 introduced a regression with darker zebra stripe colors, which this essentially just reverts) and the status quo wins in a statement, so IMO we should at least merge this now and then iterate from there. |
I already approved the PR; my review comments are just stylistic suggestions to go in different directions based on people's feedback, which is what @pradyunsg is saying we should defer for now (and I agree). |
The last review is not marked as an approval, and review with many comments makes the status confused. Anyway, this is just a minor aesthetic issue, merge the change as there seems to be a consensus that it’s a little improvement. |
Merged, thanks all! |
What effect is this supposed to have? On e.g. https://peps.python.org/#index-by-category the zebra stripes are still far from muted. |
I checked, and can confirm they are now FWIW, I would still favor that approach (minus the hover that you didn't like) as a workable compromise that mutes the zebras much further (as you want), still retains contrast (for a11y) and refines the overall appearance over the status quo. |
You seem to contradict yourself. Is eeeeee enough for a11y or isn’t it? |
I thought I replied here, but not seeing it, so replying again. |
I don't know what #eeeeee looks like, but on peps.python.org, PEP 0 still has way too much contrast in its zebra stripes -- it looks more like the "Before" example than the "After" example from earlier in this issue. |
See Hugo's comment above for screenshots of how |
Alternative to and closes #2734.
Mute the table "zebra stripes", so some rows don't look more important than others.
Re: #2734 (comment) - let's use a different CSS colour for this, so we don't affect the colour already tuned for admonitions.
Before
After
PS @CAM-Gerlach: you suggested
#111111
in #2734 (comment), I went with#222222
in this PR because I think#111111
looks a bit too muted:What do you think?