style(docs): improve table readability #2129
Conversation
| margin-bottom: 10.5em; | ||
| box-shadow: 0 1px 3px rgba(0,0,0,0.1); | ||
| } | ||
| table.docutils td, table.docutils th { |
There was a problem hiding this comment.
Could you fix the indentation on these rules?
| table.docutils { | ||
| border-collapse: collapse; | ||
| margin-bottom: 10.5em; | ||
| box-shadow: 0 1px 3px rgba(0,0,0,0.1); |
There was a problem hiding this comment.
Could you make the spacing with the rgba call consistent with the rest of the file?
| table.docutils tr:hover { | ||
| background-color: var(--selection); | ||
| } | ||
|
|
There was a problem hiding this comment.
Could you remove the extra blank line here? It looks like sections in this file are separated by a single blank line, not two.
There was a problem hiding this comment.
Hi and thanks for working on this. I think it's an improvement in general, but there are a few things I'm not so sure about:
- The hover color is a bit too colorful I think, and it makes text selection invisible inside the table. Also the contrast with the text color in dark mode is not great.
- The alternating background colors are a nice touch in light mode, but on my screen I barely see a difference in dark mode.
Other than these stylistic questions which might come down to personal taste (I'm open to hear what others think about it), there are a few issues with the code itself which I've marked.
Thanks again! 🎸
| border-bottom: 1px solid var(--hairline-color); | ||
| } | ||
| table.docutils { | ||
| border-collapse: collapse; |
There was a problem hiding this comment.
border-collapse: collapse is already the default for this element, I don't think we need to repeat it.
| } | ||
| table.docutils { | ||
| border-collapse: collapse; | ||
| margin-bottom: 10.5em; |
There was a problem hiding this comment.
That margin is way too big, did you mean 1.5em?
| table.docutils { | ||
| border-collapse: collapse; | ||
| margin-bottom: 10.5em; | ||
| box-shadow: 0 1px 3px rgba(0,0,0,0.1); |
There was a problem hiding this comment.
What's the rationale behind adding a box-shadow here? Unless I'm mistaken there are no other elements in the documentation that use a box shadow, right?
| table.docutils td, table.docutils th { | ||
| border-bottom: 1px solid #ccc; | ||
| padding: 8px 12px; | ||
| line-height: 1.4; |
There was a problem hiding this comment.
What's the purpose of this rule? Disabling it doesn't seem to have much visual effect (it seems to reduce the height of the cell by ~1px). Is it related to the padding you added? If so, what's the relationship between the two values?
| margin-bottom: 10.5em; | ||
| box-shadow: 0 1px 3px rgba(0,0,0,0.1); | ||
| } | ||
| table.docutils td, table.docutils th { |
There was a problem hiding this comment.
Can you use nesting to avoid having to repeat table.docutils?
table.docutils {
/* rules for <table> */
td, th {
/* rules for cells */
}
...
}
| box-shadow: 0 1px 3px rgba(0,0,0,0.1); | ||
| } | ||
| table.docutils td, table.docutils th { | ||
| border-bottom: 1px solid #ccc; |
There was a problem hiding this comment.
Can we use a theme color (see _utils.scss in the same folder) instead of hardcoding #ccc?
| border-bottom: 1px solid #ccc; | |
| border-bottom: 1px solid $gray-medium; |
| } | ||
| table.docutils thead th { | ||
| background-color: var(--code-bg); | ||
| border-bottom: 2px solid #999; |
There was a problem hiding this comment.
Can we use a theme color here too, maybe $gray-dark?
|
@bmispelon Thank you for the review. I will make the necessary changes. |
Summary
This PR improves the readability and visual appeal of tables in Django documentation. Specifically, it addresses the following issue( #2128):
20250716-0802-23.5769416.mp4
20250716-0831-05.0014901.mp4