Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Table child elements (thead, tbody, tfoot, tr, td, th) should inherit parent background color #29244

Closed
tmorehouse opened this issue Aug 13, 2019 · 14 comments · Fixed by #30466
Closed
Labels

Comments

@tmorehouse
Copy link
Contributor

tmorehouse commented Aug 13, 2019

When working on "sticky header" and "sticky column" features for BootstrapVue (using Bootstrap v4.3.1 CSS), I had to generate a huge amount of SCSS (and conditionals in JS) to ensure that the cells had a background color (they were transparent by default, as table cells do not inherit their parent background color).

It would be nice if Bootstrap set up the table child elements to inherit their parent element's background color, so if a color is applied to a cell, row, or table they will have a solid background.

We also came up with a simple way to generate zebra striping and hover state with much less lines of CSS by cheating with a transparent solid gradient background image (which gets overlayed on the background color).

Something like this would be great in Bootstrap (v5), and it probably would decrease the amount of CSS rules generated (specifically for the them color variants):

.table {
  // Ensure table has a background color by default, plus handle reset
  // of color for nested tables
  background-color: if($table-bg, $table-bg, $body-bg);

  // Ensure child elements inherit their parent element's background color
  // As table child elements do not inherit bg-color by default.
  // Must be defined before any table theme colors are defined
  thead,
  tbody,
  tfoot,
  tr,
  th,
  td {
    background-color: inherit;
  }

  // ...
  // Define table theme background and border color stuff here
  // But no hover/striping stuff, as it is handled below
  // ...

  // Zebra striping (regardless of background color)
  // Also works when cells have their on theme color variant applied
  // table cell backgrounds remain `solid` so if sticky headers/columns is
  // used, the header cells won't show the scrolled table contents through them
  &.table-striped {
    > tbody {
      > tr:nth-of-type(#{$table-striped-order}) {
        // Could do this to reduce the number of rules generated
        // > :nth-child(1n) {
        > td,
        > th {
          // Simple way to "darken" a row, regardless of it's `.table-<variant>` color
          // using transparent black, which is overlayed on top of bg color
          // No need to generate special striping states for each theme color
          background-image: linear-gradient($table-accent-bg, $table-accent-bg);
          background-repeat: no-repeat;
        }
      }
    }

    // Dark mode Zebra striping
    &.table-dark {
      > tbody {
        > tr:nth-of-type(#{$table-striped-order}) {
          // Could do this to reduce the number of rules generated
          // > :nth-child(1n) {
          > td,
          > th {
            // Simple way to "lighten" a row, regardless of it's `.bg-<variant>` color,
            // using transparent white, which is overlayed on top of bg color
            // No need to generate special striping states for each theme color
            background-image: linear-gradient($table-dark-accent-bg, $table-dark-accent-bg);
            background-repeat: no-repeat;
          }
        }
      }
    }
  }

  // Table hover rows  (regardless of background color)
  // Also Works when cells have their on theme color applied
  &.table-hover {
    > tbody {
      > tr:hover {
        // Could do this to reduce the number of rules generated
        // > :nth-child(1n) {
        > td,
        > th {
          // Simple way to "darken" a row, regardless of it's `.table-<variant>` color
          // using transparent black, which is overlayed on top of bg color
          // No need to generate special hover states for each theme color
          background-image: linear-gradient($table-hover-bg, $table-hover-bg);
          background-repeat: no-repeat;
        }
      }
    }

    // Dark mode table hover
    &.table-dark {
      > tbody {
        > tr:hover {
          // Could do this to reduce the number of rules generated
          // > :nth-child(1n) {
          > td,
          > th {
            // Simple way to "lighten" a row, regardless of it's `.bg-<variant>` color
            // using transparent white, which is overlayed on top of bg color
            // No need to generate special hover states for each theme color
            background-image: linear-gradient($table-dark-hover-bg, $table-dark-hover-bg);
            background-repeat: no-repeat;
          }
        }
      }
    }
  }
}

If you want to take a peek at what it looks like, check out https://bootstrap-vue.js.org/docs/components/table/#sticky-headers and https://bootstrap-vue.js.org/docs/components/table/#sticky-columns

The only issue we have is that the table borders scroll with the body since tables are set to border-collapse: collapsed; (which has the effect of transferring the borders to the table element instead of the cells). Setting that to border-collapse: separate; border-spacing: 0; fixes the border scrolling issue, but doubles the borders in some instances... but that could be fixed by only applying border to the left, bottom, and right where needed.

Related to #28480, #26678

Here is what a scrollable table with sticky headers looks like without the above changes (cells in behind of the sticky headers show through):

image

And with the above changes (for background-color inheritance):

image

/cc @mdo

@mdo
Copy link
Member

mdo commented Mar 6, 2020

Rereading this after @ysds's summarized things in #28480, a couple comments so far:

  • We're adding sticky header support for tables in Added a class for a new feature to fix table header on top #29456. For background-color there, I think relying on a .bg-white or equivalent is reasonable vs diving back into an inheriting discussion.

  • The table striping and hover hack with gradients is awesome, I'll look at that!

@tmorehouse
Copy link
Contributor Author

tmorehouse commented Mar 6, 2020

The inheritance is of BG color is needed when applying a color to the <tr> and using sticky headers (specifically sticky columns)

@mdo
Copy link
Member

mdo commented Mar 6, 2020

The inheritance could present other issues though as shown here: https://codepen.io/emdeoh/pen/OJVxqeG.

@tmorehouse
Copy link
Contributor Author

Ah, true, if the background color is translucent. Are any of the v5 background colors for table's translucent?

@mdo
Copy link
Member

mdo commented Mar 6, 2020

We have some translucency coming, but it's more about anticipating what others will try to do with this thing haha.

@tmorehouse
Copy link
Contributor Author

Yeah, true 😆

@mdo
Copy link
Member

mdo commented Mar 6, 2020

Check out #30342 for where my head is at on a bunch of the table changes. Overhauling the docs, changing some features, and putting some of your suggestions to work :D.

@tmorehouse
Copy link
Contributor Author

tmorehouse commented Mar 6, 2020

One issue with the linear gradient only on the <tr> element is that cells with a .table-{variant} class do not get the shading (for both striping and hover):

image

But if you target the inner cells (td and th), then the shading applies to the cell variants too:

image

It adds a few extra lines of CSS selectors, but it provides a more robust visual appeal (and will stop people from asking why the hover/striping are not applying to cells that have a table variant 😜), which is still a great trade off compared to all the variant rules in v4

One other thing is using the child selector > also makes hover rows work with nested tables. Where the parent table has hover rows, and the nested table all rows unfortunately get all hover styled without the child > selectors. Although it is not too bad:

image

Above row "C" is hovered (which also triggers hover on the parent table <tr>). The deeper you nest tables, the darker the hover shading becomes on the inner most table. Also if you hover over the nested table's header it gets hover styling due to the lack of specificity on the selectors.

@tmorehouse
Copy link
Contributor Author

But other-wise looks great!

@mdo
Copy link
Member

mdo commented Mar 6, 2020

We don't explicitly have examples that show this is possible, and this currently doesn't work in v4. I don't know that this is something we need to explicitly worry about.

I hear you on the nesting though, and with nesting hovers. Will keep looking into that.

@tmorehouse
Copy link
Contributor Author

tmorehouse commented Mar 6, 2020

The docs (v4) do mention that the table variants can be placed on cells though, and BootstrapVue's user base does take advantage of individual cell variants (it was a popular ask, so we had to add the custom SCSS to handle it for our Vue table components)

@mdo
Copy link
Member

mdo commented Mar 6, 2020

Oh yeah, for sure, just pointing out that it doesn't actually work with those other table modifiers :). I also don't think it's worth the trouble to add additional styles to account for that either, at least for striping—if you want a cell to be a particular color, it should be that color.

Actually I guess we could put the background-image on the cells for hover/stripe, and then the .table- modifiers would stack whether they were on the row or the cell?

@tmorehouse
Copy link
Contributor Author

tmorehouse commented Mar 6, 2020

Actually I guess we could put the background-image on the cells for hover/stripe, and then the .table- modifiers would stack whether they were on the row or the cell?

Yeah, that would work well regardless of where the color is applied (table, tbody, tr, td, or th). If a cell doesn't have a background color, the row's background color will show through, and if the row doesn't have a background color then the background of the row group will show, and so-on and so-on and so-on, and so-on... https://www.youtube.com/watch?v=mcskckuosxQ)

@tmorehouse
Copy link
Contributor Author

tmorehouse commented Mar 6, 2020

e.g.:

.table-striped {
  > tbody > tr:nth-of-type(#{$table-striped-order}) {
    > td,
    > th {
      background-image: linear-gradient($table-accent-bg, $table-accent-bg);
    }
  }
}

.table-hover {
  > tbody > tr:hover {
    > td,
    > th {
      color: $table-hover-color;
      background-image: linear-gradient($table-hover-bg, $table-hover-bg);
    }
  }
}

These shading rules wont leak into nested tables

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants