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

Reboot's th updates: Inherit font-weight: bold that comes from user agent stylesheets. #30781

Merged
merged 4 commits into from Jun 17, 2020

Conversation

zalog
Copy link
Contributor

@zalog zalog commented May 12, 2020

Inherits font-weight: bold that comes from user agent stylesheets.

@zalog zalog requested a review from a team as a code owner May 12, 2020 04:56
@MartijnCuppens
Copy link
Member

I like the default bold font weight which makes a visual difference between the table cells. We could nullify this variable though

@zalog
Copy link
Contributor Author

zalog commented May 12, 2020

Hmm, yep but I think the default needs to be consistent through all browsers.
Done adding a var, rebased/squash :)

@ffoodd
Copy link
Member

ffoodd commented May 12, 2020

Agreed, every browser applies font-weight: bold; (search for th) so there is a kind of consensus here.

I'm not sure about the interest of allowing this through a variable, TBH: it's pretty easy to either apply a utility or to override those styles. We'd be adding code for something that hasn't been requested.

@zalog
Copy link
Contributor Author

zalog commented May 12, 2020

Yep.

I think if bs users needs to style th, they should do this on the html element, and not class. Seems "natural" I think :)

@ffoodd
Copy link
Member

ffoodd commented May 12, 2020

Using a Sass variables is not easier than editing _reboot.scss—and as outlined previously, there has never be any request for this, as far as I can search in issues.

Furthermore, if one uses this to unset boldness on th, how are they supposed to make them distinguishable from other cells?Because they should.

@zalog
Copy link
Contributor Author

zalog commented May 12, 2020

"Using a Sass variables is not easier than editing _reboot.scss" actually it is and the thing is, I think we don't want to force our users to edit any of our bootstrap core code.

"how are they supposed to make them distinguishable", yep. I think the possibilities are endless here: a simple border under thead > * > *, or a background can do that. There are a lot of use cases here. Right now, I'm working on a table that has on thead font size bigger and no other differences between tbody's :)

Imho, we should give our user this option, of not leaving browsers manage this and not trough .table class :)

@ffoodd
Copy link
Member

ffoodd commented May 12, 2020

It's a bit clearer, thanks for your patience :)

I'm still not sure about this, since it could too easily give undistinguishable th. Not strictly against but not really for either :')

@MartijnCuppens feel free to approve or discuss!

@mdo mdo changed the base branch from master to main June 16, 2020 19:57
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.

I'm in favor of this since the font-weight stays bold. We could easily backport this to v4 as well I'd think. Thoughts @MartijnCuppens?

@XhmikosR XhmikosR added this to Inbox in v5.0.0-alpha2 via automation Jun 17, 2020
@XhmikosR XhmikosR added this to Inbox in v4.5.1 via automation Jun 17, 2020
@XhmikosR
Copy link
Member

I marked this for backport, but you guys decide if it's considered a breaking changes. The next v4 release is supposed to be a patch release.

@MartijnCuppens
Copy link
Member

It's not a BC, this PR just adds a null variable

@XhmikosR
Copy link
Member

I will backport it if it applies clean then.

@XhmikosR XhmikosR merged commit 9f9e4d0 into twbs:main Jun 17, 2020
v5.0.0-alpha2 automation moved this from Inbox to Shipped Jun 17, 2020
@XhmikosR XhmikosR moved this from Inbox to Needs manual backport in v4.5.1 Jun 17, 2020
@XhmikosR
Copy link
Member

This doesn't apply clean to v4-dev, feel free to backport it manually @MartijnCuppens

@zalog zalog deleted the zalog-reboot-table branch June 17, 2020 19:07
@mdo mdo added this to In progress in v4.5.3 via automation Aug 4, 2020
@mdo mdo removed this from Needs manual backport in v4.5.1 Aug 4, 2020
@mdo mdo moved this from In progress to Needs manual backport in v4.5.3 Aug 4, 2020
mdo added a commit that referenced this pull request Sep 29, 2020
Manually backports #30781 to v4.
@mdo mdo mentioned this pull request Sep 29, 2020
@mdo mdo moved this from Needs manual backport to Cherry-picked/Manually backported in v4.5.3 Sep 29, 2020
XhmikosR pushed a commit that referenced this pull request Sep 30, 2020
Manually backports #30781 to v4.
olsza pushed a commit to olsza/bootstrap that referenced this pull request Oct 3, 2020
Inherit `font-weight: bold` that comes from user agent stylesheets.
mdo added a commit that referenced this pull request Oct 4, 2020
Manually backports #30781 to v4.
XhmikosR pushed a commit that referenced this pull request Oct 5, 2020
Manually backports #30781 to v4.
@XhmikosR XhmikosR moved this from Cherry-picked/Manually backported to Shipped in v4.5.3 Oct 5, 2020
@XhmikosR XhmikosR changed the title reboot: table th Reboot's th updates Oct 13, 2020
@XhmikosR XhmikosR changed the title Reboot's th updates Reboot's th updates: Inherit font-weight: bold that comes from user agent stylesheets. Oct 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
v4.5.3
Shipped
v5.0.0-alpha2
  
Shipped
Development

Successfully merging this pull request may close these issues.

None yet

5 participants