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

v5.3.0: Table rows can no longer be colored with text-bg-* #38779

Open
3 tasks done
LinqToException opened this issue Jun 17, 2023 · 19 comments · May be fixed by #38826
Open
3 tasks done

v5.3.0: Table rows can no longer be colored with text-bg-* #38779

LinqToException opened this issue Jun 17, 2023 · 19 comments · May be fixed by #38826
Assignees
Labels

Comments

@LinqToException
Copy link

LinqToException commented Jun 17, 2023

Prerequisites

Describe the issue

This is (as far as I can tell an undocumented) change from 5.3.0-alpha3 and previous, where this worked, to 5.3.0.

Previously, it was possible to color table rows (and perhaps cells, which I've not used) in a different color using text-bg-*, for example <tr class="text-bg-danger">. I have primarily preferred them over the table-* classes because of the more uniform look with other elements and the stronger, more distinct colors. The documentation does not explicitly state that the text-bg-* or bg-* classes cannot be used on table elements, nor do the table docs mention that you have to use the table-* classes.

When trying to do this now, the row remains default colored, because the rule for .table > :not(caption) > * > * , which sets background-color: var(--bs-table-bg);, takes precedent on the <td>s.

Reduced test cases

<table class="table">
  <thead>
    <tr class="text-bg-primary">
      <td>Column 1</td>
      <td>Column 2</td>
    </tr>
  </thead>
  <tbody>
    <tr>
      <td>Cell 1</td>
      <td>Cell 2</td>
    </tr>
    <tr>
      <td>Cell 3</td>
      <td>Cell 4</td>
    </tr>
</table>

What operating system(s) are you seeing the problem on?

Windows

What browser(s) are you seeing the problem on?

Chrome, Firefox, Microsoft Edge

What version of Bootstrap are you using?

v5.3.0

@louismaximepiton louismaximepiton moved this to Needs review in v5.3.1 Jul 17, 2023
@mdo mdo removed this from v5.3.1 Jul 25, 2023
@mdo mdo added this to v5.3.2 Jul 25, 2023
@louismaximepiton louismaximepiton moved this to Needs review in v5.3.2 Jul 27, 2023
@louismaximepiton louismaximepiton removed the status in v5.3.2 Jul 27, 2023
@louismaximepiton louismaximepiton self-assigned this Jul 27, 2023
@julien-deramond julien-deramond removed this from v5.3.2 Sep 13, 2023
@github-project-automation github-project-automation bot moved this to To do in v5.3.3 Sep 13, 2023
@julien-deramond julien-deramond moved this from To do to Needs review in v5.3.3 Sep 13, 2023
@entraspan
Copy link

I have used a custom thead gradient from bootstrap 3 to 5.3.0-alpha3 without any issues. Thought surely when this broke in 5.3 final it was a simple oversight that would be fixed in the next release.

The description above clearly identifies an issue that was caused by a last minute change with 5.3 final.

Surely there are more than two people waiting for a solution, this should be a higher priority!

Have always been an early adopter of new versions, but just had to revert again to alpha3.

@Pigeo
Copy link

Pigeo commented Nov 15, 2023

I have similar (though different) issues because of this rule selector .table > :not(caption) > * > * : it takes precedence over my simple CSS rule, which is:

.my-table-header {
  color: gray;
}

(this code was working in previous versions of Bootstrap)

Please note: the suggested fix #38826 is not the proper way to fix this issue, because, as you can imagine, it won't fix my issue and a great number of other issues related to other properties like padding, border, box-shadow, etc… Thus, the good way of fixing this issue would be to get rid of .table > :not(caption) > * > * which is overkill and will override a lot of CSS rules that it should not override... (such a rule selector seems to me like code smell)
=> Instead of that, why not use a rule with .table > caption … that would override the default .table rule by reseting some CSS properties like the color, padding, border, etc. to their default value ?

In the meanwhile, I've been adding the following work-around in my SASS styles :

.my-table-header {
  // We can't customize the table header color with Bootstrap SASS variables, as per
  // https://getbootstrap.com/docs/5.2/content/tables/#variables
  // and latest versions of Bootstrap are overriding our color property, 
  // so here is a little fix:
  --bs-table-color: initial;

  color: gray;
}

@louismaximepiton
Copy link
Member

Hey, thanks for raising the issue.

For now I think it's safer to keep the selector .table > :not(caption) > * > * to handle correctly the nesting and don't break anything for people that might need those selector. IMHO, it could be rethought sooner or later to make it less selective since we have CSS variables now. I'll try to think about it, and if possible as a non-breaking change for folks so that could be embedded asap otherwise, it might happen for v6.

However, I think that this subject is unrelated to that issue because as you said it used to work in previous Bootstrap versions.

I don't know your specific use case but I managed to tweak a bit the code for now to be merged as soon as possible in the linked PR. I've updated the linked PR to make this use case work without breaking anything else. Could you please check that the fix work with your use case ?

If not, feel free to open a PR and we'll discuss about your solutions.

@omundy
Copy link

omundy commented Jan 4, 2024

The wildcards in .table > :not(caption) > * > * select everything that is not a caption inside every cell. This means that links suddenly have unexpected styles whether you use a caption or not (I have never used a caption in a table myself). Instead of (what I have always thought is) Bootstrap's "add what you need", it's now "add hacks to undo things you didn't need".

    .table > :not(caption) > * > * {
        background-color: red;
    }

image

@LinqToException
Copy link
Author

Thanks for highlighting the issue with a picture. I've been setting up several pages now, still with 5.3.0-alpha3 until this has been sorted out - or eventually move to something else altogether.

and don't break anything for people that might need those selector.

This statement feels wrong to me - this has been a breaking change in behaviour compared to previous versions, yet it is now being kept around in case people rely on it? What about all the developers and cases where the old behaviour, i.e. no behaviour, was intended?

If it were at least possible to turn this behaviour off with a variable or similar, it would be more bearable, but as far as I know, that's not really possible?

@bruno-71
Copy link

Same issue here. I can't even override it with a local style

@gvreddy04

This comment was marked as off-topic.

@gvreddy04

This comment was marked as off-topic.

@jsmalme
Copy link

jsmalme commented May 7, 2024

Yeah, I was setting my thead background color to "bg-primary" but that no longer works in 5.3. I tried setting class="table-primary" but the color is far too light for my purposes. I would be like to set the background color more easily without the extra css work-arounds.

@jeffputz
Copy link

I think this broke starting in v5.3.0, and it's definitely still there in v5.3.3. The selector .table > :not(caption) > * > * is just too greedy. The same problem happens if you apply bg-warning to a tr, because the child td will get this rule applied. This breaks something you could do going back to very early BS versions.

@LinqToException
Copy link
Author

LinqToException commented May 16, 2024

Yeah, it's a shame that it's still not fixed. You don't need to go back to "very early" versions however - as described in the issue, 5.3.0-alpha3 hadn't that selector yet.

However, I feel like using an alpha can't/shouldn't be the solution to this breaking change.

@paritoshromy
Copy link

This needs to be fixed soon....its a critical element of the table head design.

@dhodgin
Copy link

dhodgin commented Sep 26, 2024

Same issue with me. Trying to upgrade from 5.1.3 to 5.3.3 and this just completely changed the design of all my tables. Table header background-color is overridden with white from .table>:not(caption)>*>*

My fix is to use !important on my custom class for headers or to set the --bg-table-bg variable

.submitted {
  background-color: #f1f1a7 !important;
  border: 1px solid #dada7f !important;
}
 
OR

.submitted {
  background-color: #f1f1a7;
  border: 1px solid #dada7f;
  --bs-table-bg: #f1f1a7; 
}

It doesn't fully fix the issue though. The background-color in this selector is making alternating rows white in the background and destroyed my UI/UX.

<input> elements also have background colors changed to white which throws off the contrast of my forms site wide.

This is a huge PIA

@vinodsantharam
Copy link

vinodsantharam commented Oct 1, 2024

The selector .table > :not(caption) > * helps us to change color and border-width.
However it can't help us to change the background-color on hover.
Then the solution we are using is to override Bootstrap variable like:

// _table.scss
.table-hover {
  --bs-table-hover-bg: #cee4da;
}

Edit:
Actually we had an issue with active & hover combined. We changed to transparent. Then our custom css is applied and it works like charm ✌️

// _table.scss
.table-hover {
  --bs-table-bg: transparent;
  --bs-table-hover-bg: transparent; 
  --bs-table-active-bg: transparent;
}

@jackie-t
Copy link

jackie-t commented Oct 4, 2024

Guys this is a huge issue, I can't believe this is unresolved since 2023.

I've used Bootstrap 5.3.3 for a new project and I found myself adding more and more hacks because on a dark design, I would get random blotches of white background in a table.

I had to downgrade to Bootstrap 5.2, which immediately fixed this, and allowed me to remove all these hacks. When multiple people cry "this needs to be fixed soon" and weeks pass, and people revert to using alphas (or downgrading), then something's amiss.

@jeffputz
Copy link

jeffputz commented Oct 4, 2024

It's not encouraging that it's marked as "nice to have" for v5.3.4. This breaks tables so badly.

@julien-deramond
Copy link
Member

Guys this is a huge issue, I can't believe this is unresolved since 2023.

It's not encouraging that it's marked as "nice to have" for v5.3.4. This breaks tables so badly.

It's listed as 'nice to have' for v5.3.4 since the focus of this release was elsewhere, and it's already slightly delayed for various reasons. However, that doesn't mean it won't make it into v5.3.5, for example. Just keep in mind, Bootstrap is maintained by volunteers in their spare time. We're doing our best with the limited hours we have. :)

Feel free to help review #38826 by thoroughly testing it to ensure we don't fix one issue only to break something else. That's the challenge we're facing with the table colors: we fix one thing, and while some folks are happy, it ends up causing issues in other unexpected use cases. The use cases are numerous depending on how the markup is used, so it can be a little bit tricky to handle everything.

@jeffputz
Copy link

jeffputz commented Oct 4, 2024

I appreciate the volunteer efforts, I just feel like 8 months is a long time to go between releases. As it stands now, the v5.3.4 only has effectively two "mandatory" items in it, which seems pretty low risk in terms of adding other things.

@jackie-t
Copy link

Likewise, I appreciate the volunteer efforts. You should read my comment as "watch out, this is a huge issue that undermines your efforts of making great software". All the other nice things that may be added aren't useful, if I can't use bootstrap due to how it breaks tables.

This shouldn't downplay your efforts, just alert you that this is something big that overshadows them, and that appears getting missed (due to how much this has been open).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Needs review
Development

Successfully merging a pull request may close this issue.