Vertically align cell contents in DataTable
#3843
Conversation
🦋 Changeset detectedLatest commit: 1fae2c7 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
size-limit report 📦
|
thyeggman
temporarily deployed
to
github-pages
GitHub Actions
Inactive
thyeggman
temporarily deployed
to
github-pages
GitHub Actions
Inactive
|
Here's a screenshot from the storybook showing the new alignment: 
|
There was a problem hiding this comment.
Really like this change. The table cell items look a lot better. I ran the storybooks and compared them with the original and couldn't really find any regressions so far. I will approve but please don't merge till @joshblack or @mperrotti (original component authors) approve
There was a problem hiding this comment.
LGTM! Will wait to see what @mperrotti says before merging 👀
| @@ -66,7 +66,10 @@ const StyledTable = styled.table<React.ComponentPropsWithoutRef<'table'>>` | |||
| .TableHeader, | |||
| .TableCell { | |||
| text-align: start; | |||
| display: flex; | |||
| align-items: center; | |||
There was a problem hiding this comment.
I'm a little worried about how this might affect the readability of rows where some cells have 1 line of text, and other cells wrap to 2+ lines.
Compare these two tables:
There was a problem hiding this comment.
Yeah that's a good point. I'm not sure if the best solution is something that involves proper padding if there's a row action available, or increasing the base height of a row when there is no action. I don't think anyone but us is using DataTable at the moment, and our current plan is to truncate row content when it overflows a cell.
There was a problem hiding this comment.
Late on this but I think align-items: baseline would solve for that specific issue, though not sure if it would introduce problems with other elements like IconButtons or Labels.
joshblack
temporarily deployed
to
github-pages
GitHub Actions
Inactive
Changelog
This PR vertically aligns cell content for the
DataTablecomponent. When a row has actions, the typical pattern is to use anIconButton. However, the button is taller than the row when the row contents are text. Currently, by default, cell contents are top-aligned, so this creates a disparity between the top and bottom margins for cell content. This is best demonstrated by this storybook:New
Added styles to
TableCellandTableHeaderChanged
NA
Removed
NA
Rollout strategy
Testing & Reviewing
Merge checklist
Take a look at the What we look for in reviews section of the contributing guidelines for more information on how we review PRs.