twbs / bootstrap

The most popular HTML, CSS, and JavaScript framework for developing responsive, mobile first projects on the web.
https://getbootstrap.com
MIT License
169.98k stars 78.74k forks source link

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

Open LinqToException opened 1 year ago

LinqToException commented 1 year ago

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

entraspan commented 10 months ago

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 commented 10 months ago

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 commented 10 months ago

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 commented 8 months ago

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 commented 8 months ago

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 commented 6 months ago

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

gvreddy04 commented 6 months ago

https://github.com/vikramlearning/blazorbootstrap/issues/599

gvreddy04 commented 6 months ago

https://github.com/vikramlearning/blazorbootstrap/issues/606

jsmalme commented 4 months ago

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 commented 4 months ago

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 commented 4 months ago

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.