-
Notifications
You must be signed in to change notification settings - Fork 66
Conversation
} | ||
|
||
th { | ||
padding: em( 12px, 12px ); |
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.
Setting the padding for th
s twice (see line 23) seems weird.
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.
It appears that way, but they aren't. The em values are different since the font size of th
is smaller.
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.
In that case is there really a point in combining the td
and th
rules here since they only actually share 2 properties top border and white space. Maybe just add those 2 properties here to the th
rule and have above just be td
It will make it more clear whats going on to avoid confusion.
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.
Will split these up. Probably not worth the combo.
@MichaelArestad awesome, I especially love what you did with responsive tables. |
any specific reasons why headings are less dark and smaller than rest of table by default? I thought it should be other way round? |
Yep. The other way around is just a browser default leftover. They are just the labels for the content. The content should be the easiest thing to read. If they are all bold, the |
Thought i added this but i don't see it live preview link http://view.css-chassis.com/table/demos/tables.html |
👍 looks great |
509891d
to
14b36cf
Compare
14b36cf
to
0660ad2
Compare
Note: error due to BEM class name. |
Yup looks like we need to modify our settings |
@arschmitz Yep. We should do another PR for fine-tuning the |
td { | ||
border-top: 1px solid #eee; | ||
padding: em( 12px, 16px ); | ||
white-space: nowrap; |
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.
Note for later: Remove nowrap on td
.
|
||
<h2>Tables</h2> | ||
|
||
<p>Here"s a paragraph to show spacing around the table. Aenean lacinia bibendum nulla sed consectetur. Maecenas faucibus mollis interdum..</p> |
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.
Here"s
-> Here's
Fixed a few things and separated overflow styles (fancy gradient) into its own component, |
// ========================================================================== | ||
|
||
// Adds a single pixel underline (so nice) on retina | ||
// Usage: `@include slime-underline;` |
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.
"slime" [sic]
Also, the "reimplementing underlines" technique in general seems (a) overcomplicated/overkill (b) not easy to override
Replaced by #69 |
@jzaefferer replaced by #130... you mean? |
Yes, copy pasta error. |
Super basic v1 of table styling. Looks like this currently and include some fancy overflow styles.
Thoughts?