fix(data-table): fix sticky header issues

[tw15egan]

Closes #6449

Fixes a spacing issue when sticky header is enabled in DataTable

Changelog

Changed

• Set specific padding for normal sticky header td, th
• Ensured other size variants padding is not affected

Testing / Reviewing

Ensure all variations of sticky header sizes look correct

[netlify]

Deploy preview for carbon-components-react ready!

Built with commit 619d8d4

https://deploy-preview-6452--carbon-components-react.netlify.app

[netlify]

Deploy preview for carbon-elements ready!

Built with commit 619d8d4

https://deploy-preview-6452--carbon-elements.netlify.app

[tw15egan]

@joshblack fixed 👍

[guychris]

@tw15egan Hi TJ, I had made a comment in the issue after taking a look at the storybook for this PR, this fixes normal tables but tables with expandable rows will need some work:

#6449 (comment)

[tw15egan]

@guychris should be fixed now, thanks for catching that 👍

[guychris]

Thank you for the quick turnaround, just poking around some more and a few more edge cases I found:

• With expandable rows + stickyHeader, it looks like there's a bit of extra space at the bottom of the table,
  
  https://deploy-preview-6452--carbon-components-react.netlify.app/?path=/story/datatable--with-expansion

• With selection + sticky Header, the checkbox in the header row is not centered:
  
  https://deploy-preview-6452--carbon-components-react.netlify.app/?path=/story/datatable--with-selection

• with overflow menu + sticky header, the overflow menu icon is not centered and it appears the alignment of the header columns is off:
  
  https://deploy-preview-6452--carbon-components-react.netlify.app/?path=/story/datatable--with-overflow-menu

• one combination that I'm not sure how to test with your PR changes would be radio button selection + expandable rows + stickyHeader. Code for that in this sandbox, https://codesandbox.io/s/carbondatatablestickyheader-03096?file=/src/App.js

[tw15egan]

@guychris Thanks for taking the time and making sure I'm getting all the variants! 🙏

Just pushed up some more changes that should take care of the rest of the issues.

In regards to your last one, I checked how it renders with my changes, and it all looks good except the Table Headers are slightly misaligned with the Table Cells. Since there are 2 more cells than headers, you'd just have to set a manual offset on the headers based on the number of controls (in this case 2, so it'd probably be 48 * 2 px. I didn't add this into the styles since I think this is an edge case that would be better handled at the application level.

Let me know if I missed any more!

@joshblack @andreancardona : added a conditional render in TableHeader.js so that we don't render a bx--table-header-label without any text. Instead, it will just render an empty td, which I used to target some very specific nodes: https://github.com/carbon-design-system/carbon/pull/6452/files#diff-977095845d8e1ee345eea1f4b7a52d9dR577

[guychris]

Thanks for the quick turnaround on the fixes @tw15egan , all the scenarios I was going through in the storybook that we might be using look good with stickyHeader now. It does look like changing the row height breaks a few different stylings unfortunately (apologies for opening the stickyHeader can of worms!). We're not changing row height in our case, but playing around in the storybook it's looking like the row height stylings are not being applied consistently to row elements.

few different parts I see that are not styled correctly:

• vertical alignment of header checkbox with tall rows is lower than header text
• compact row height without stickyHeader: 24px, with stickyHeader: 48px.
• short row height without stickyHeader: 32px, with stickyHeader: 51px.
• tall row heights without stickyHeader: 64px, with stickyHeader: 54px.
• compact and short row height, row text and checkboxes are vertically aligned to the top instead of centered
• compact and short row height, overflow menu icon vertical alignment is at the top and the row separator line is out of alignment with the rest of the row.

some screenshots:
compact row heights with stickyHeader and overflow menu:

tall row heights with selection, header checkbox alignment:

[tw15egan]

@guychris just pushed up some more fixes. Keep in mind, that compact / short will break out of the specified size if the screen is too small to accommodate the full width.

[tw15egan]

@laurenmrice not sure what's going on in Safari, might be something with the deploy preview? This is what I see locally

Mouse

Trackpad

[laurenmrice]

For the expanded data table stories, empty extra rows get added to the data table when the sticky header is activated.

[tw15egan]

@laurenmrice should be fixed

[laurenmrice]

The expansion stories looks good for the normal height data table now, but is getting height issues and icon misalignment for the other sizes. After that we should be good

[tw15egan]

@laurenmrice hopefully all set now

[laurenmrice]

Ok, I think this will be the last thing 😁!

For these stories with dynamic content , with expansion , with options :
-The column header text is misaligned with the cell row text below it (for all sizes).
-The stories mentioned with a column header checkbox icon get misaligned with the icons in the cell rows (for all sizes).

[laurenmrice]

with dynamic content and with options stories:
Upon expanding a row the checkbox in that row jumps lower for the short and compact sizes.

[laurenmrice]

Woohoo! looks good to me 🙌🏻🎉