New admin table layout #1786
New admin table layout #1786
Conversation
| // If you want to disable this, add the .wrap-text class to the column. | ||
| &:not(.wrap-text) { | ||
| white-space: nowrap; | ||
| } |
There was a problem hiding this comment.
I'm not sure about this. I think our default should be to wrap, and to specify areas where wrapping is undesirable (like actions).
There was a problem hiding this comment.
I think the default should be no wrap. Most of the time we display where little information in cells, mostly values (order numbers, email addresses, etc. You don't want to wrap these.
The only occasion where wrapping was useful (not even needed) was the image caption. It would be a much larger change set to add a no-wrapp class to all cells where we want this to happen.
But this is just my preference. I'm also fine with having the opposite being the default. I would then vote for also making the header cells non wrapping. Nobody wants a
"Created
at"
cell, right?
There was a problem hiding this comment.
Took the opposite approach like suggested.
| margin-bottom: 15px; | ||
| border-collapse: separate; | ||
| // Extend all tables with Bootstrap's default table style | ||
| @extend .table; |
There was a problem hiding this comment.
This isn't worse than what was here before (bootstrap's .table sets pretty much the same CSS as we already had here). It would be ideal if we were to move away from styling all table elements like this and instead required a class. That way we avoid needing to un-style the styles when we want a table to be "normal".
There was a problem hiding this comment.
What would a "normal" table look like? Browser default? I always thought this is our default table style. But I understand your concern. Im also fine with adding a table class to all tables. The change set will be much larger then, though.
There was a problem hiding this comment.
We can deal with this later. It's not a new issue and shouldn't block this PR.
tvdeyen
force-pushed
the
admin-table-layout
branch
from
518e0b0 to
d3ec27b
Compare
|
@jhawthorn I updated the commit and took a more conservative approach. Only table headers, the
|
The content of all table headers, state columns and action cells have text wrapping disabled now. For cases where we want to disble text wrapping in data cells we provide the `.no-wrap` class. This also makes (very sensitive) use of the Bootstrap default table style. The main advantage is that Bootstrap uses rem (a relative unit) for table cell padding. As we plan to raise the body font size this is a huge win for us.
tvdeyen
force-pushed
the
admin-table-layout
branch
from
d3ec27b to
0dcc028
Compare
The content of all table headers, state columns and action cells have text wrapping disabled now. For cases where we want to disble text wrapping in data cells we provide the
.no-wrapclass.This also makes (very sensitive) use of the Bootstrap default table style. The main advantage is that Bootstrap uses rem (a relative unit) for table cell padding. As we plan to raise the body font size this is a huge win for us.
Before
After