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

Support simpler table structure #34269

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from
Draft

Conversation

ffoodd
Copy link
Member

@ffoodd ffoodd commented Jun 16, 2021

Fixes #34184

Not quite sure about this, but requiring a tbody doesn't feel right either…

@ffoodd ffoodd requested a review from a team as a code owner June 16, 2021 15:42
@ffoodd ffoodd closed this Jun 16, 2021
@ffoodd ffoodd reopened this Jun 16, 2021
@ffoodd ffoodd marked this pull request as draft June 16, 2021 15:44
@ffoodd ffoodd changed the title fix(tables): support simpler table structure Support simpler table structure Jun 16, 2021
@mdo
Copy link
Member

mdo commented Jun 16, 2021

Starting to feel like it might be easier to just target the td and th, then provide some sort of undo for nested tables like .table-reset.

td,
th {
   padding: $table-cell-padding-y $table-cell-padding-x;
   background-color: var(--#{$variable-prefix}table-bg);
   border-bottom-width: $table-border-width;
   box-shadow: inset 0 0 0 9999px var(--#{$variable-prefix}table-accent-bg);
 }
.table-reset {
  td,
  th {
    padding: unset;
    background-color: unset;
    border-bottom-width: unset;
    box-shadow: unset;
  }
}

Also makes me think we can update some of these values to be more CSS variables, and then reset them that way. So instead of the above reset, something like this:

.table-reset {
  --bs-table-bg: unset;
  --bs-table-border: unset;
  // etc
}

@ffoodd
Copy link
Member Author

ffoodd commented Jun 16, 2021

Indeed. However I'm still concerned this is a valid issue or not.

@ffoodd
Copy link
Member Author

ffoodd commented Jun 16, 2021

From the discussion in the linked issue, we definetely need to adress this.

Using a reset based on a class would ne some kind of regression since styles could leak again to nested tables.

We coule do the same but target .table .table instead, being closer to existing feature. Any opinion ?

@mdo
Copy link
Member

mdo commented Apr 12, 2022

At this point, I'm almost wondering if we go the route of our old selectors and explicitly include td and th to drop the universal selectors?

@mdo mdo force-pushed the main-fod-simpler-table-structure branch from ef53363 to 87239ef Compare April 12, 2022 22:29
@ffoodd
Copy link
Member Author

ffoodd commented Apr 14, 2022

It'd be much simpler indeed, I guess.

Regarding nested tables and leaking styles, the important parts are direct child combinators.

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 this pull request may close these issues.

Buttons do not show up properly in a table without a tbody
3 participants