Skip to content

Prevent nested tables style leaks#30466

Merged
MartijnCuppens merged 4 commits intomasterfrom
master-mc-nested-tables
Apr 24, 2020
Merged

Prevent nested tables style leaks#30466
MartijnCuppens merged 4 commits intomasterfrom
master-mc-nested-tables

Conversation

@MartijnCuppens
Copy link
Member

@MartijnCuppens MartijnCuppens commented Mar 26, 2020

Prevent nested table style leaks by using children combinators and CSS custom properties:

  • Change in reboot: inherit border color and set border style to solid to make it easier to change borders. Border width is still 0, so no borders are visible by default.
  • The only way to get the colors right is by only colorizing the td and th backgrounds, therefore we need to work with CSS custom properties.
  • Stole the background image hack from @mdo in De-dupe .table-dark, add new border color utilities, and update table docs #30342
  • All table variants are now stored in $table-variants. This way we have more control over colorizing tables. The dark table is also a variant now.
    • $table-border-factor will determine the contrast between the background and the borders
    • $table-accent-bg-factor will determine the contrast between the background and striped rows
    • $table-hover-bg-factor will determine the contrast between the background and hovered rows
    • The color of the text will be determined by contrast-color()
  • Simplified the docs a little
    • Stored the table content in a partial, which is easier to reuse
    • Only kept the whole html for the first example, stripped some html in the other examples (inspired by the modal docs change)

TODO:

Preview: https://deploy-preview-30466--twbs-bootstrap.netlify.com/docs/4.3/content/tables/


@mdo
Copy link
Member

mdo commented Apr 2, 2020

This is pretty damned awesome, pretty powerful. Nicely done!

Only feedback on this right now is that I'm really partial to the modified table styles I implemented months back with the darker, single pixel table header border :). In addition, I think we'll need more detailed "how it works" and "how to customize" sections to explain the collision of CSS variables and Sass variables.

Screen Shot 2020-04-02 at 9 41 08 AM

@MartijnCuppens MartijnCuppens force-pushed the master-mc-nested-tables branch from 02da595 to 7ead756 Compare April 4, 2020 10:58
@MartijnCuppens
Copy link
Member Author

Restored the table thead styles (also added the border between multiple tbodys or a tfoot)

Would be handy if I could use #30502 to document things.

@MartijnCuppens MartijnCuppens force-pushed the master-mc-nested-tables branch 8 times, most recently from 9a461b9 to 1b4c71a Compare April 10, 2020 18:52
@MartijnCuppens
Copy link
Member Author

@twbs/css-review, ready for review. @mdo, could you doublecheck the doc edits?

@tmorehouse
Copy link
Contributor

tmorehouse commented Apr 10, 2020

Should the custom CSS property vars be prefixed ( i.e. --bs-table-bg)? Just in case users have set up their own vars for other purposes (i.e. non bootstrap table styles)?

@MartijnCuppens
Copy link
Member Author

@tmorehouse, I've opened #30558 to discuss this, we'll need to change our custom properties in :root too, which is unralated to this PR.

@MartijnCuppens
Copy link
Member Author

bs prefix added ✅

@MartijnCuppens
Copy link
Member Author

@twbs/css-review, this one is ready to review.

@MartijnCuppens MartijnCuppens force-pushed the master-mc-nested-tables branch from efa3ad6 to 626e5b6 Compare April 21, 2020 08:20
@ffoodd
Copy link
Member

ffoodd commented Apr 21, 2020

LGTM

Copy link
Member

@mdo mdo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just some docs copy changes, otherwise looks great!

@MartijnCuppens MartijnCuppens force-pushed the master-mc-nested-tables branch from 24da4d0 to 1e143bd Compare April 24, 2020 07:29
@MartijnCuppens MartijnCuppens merged commit cff2a08 into master Apr 24, 2020
@MartijnCuppens MartijnCuppens deleted the master-mc-nested-tables branch April 24, 2020 08:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

5 participants