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

Doc: use .text-bg-{color} for all badges #39214

Merged
merged 1 commit into from
Oct 23, 2023

Conversation

julien-deramond
Copy link
Member

@julien-deramond julien-deramond commented Sep 20, 2023

Description

This PR suggests using .text-bg-{color} all the time when badges are used.

Note that https://deploy-preview-39214--twbs-bootstrap.netlify.app/docs/5.3/utilities/position/#examples still uses .bg-danger as there's no text in this rounded badge.

Motivation & Context

The rendering is the same in Bootstrap documentation because the same color palette is used whether we are in light or dark mode (see our $theme-colors).

However, when a project needs to have specific colors for the dark mode, it needs to create a $theme-colors-dark. It will imply that the background and text colors will be different for .text-bg-{color}.
Without this change, our badges would have the right background color in dark mode, but the text color wouldn't be the right one.

Type of changes

  • Enhancement (non-breaking change)

Checklist

  • I have read the contributing guidelines
  • My code follows the code style of the project (using npm run lint)
  • My change introduces changes to the documentation
  • I have updated the documentation accordingly
  • (N/A) I have added tests to cover my changes
  • All new and existing tests passed

Live previews

Copy link
Member

@louismaximepiton louismaximepiton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we don't have a different color palette for dark mode, it looks harmful for basic users.

However, for advanced users that might want to implement a different color palette using dark mode, they might have some issues because :

The calculated color value from .text-bg-* helper comes from color-contrast() applied on theme-colors so it won't support CSS variables. -> Should we introduce something to handle this use case ?

Copy link
Member

@louismaximepiton louismaximepiton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rethinking about this, I might overthinked a bit on this since we don't have CSS variables right now. Let's go for it !

@julien-deramond
Copy link
Member Author

@mdo friendly ping for a double-check :) Adding you as a reviewer. If not available, don't hesitate to tell me, and then we'll merge the PR.

@julien-deramond julien-deramond requested a review from mdo October 13, 2023 08:57
@mdo mdo merged commit f7426c0 into main Oct 23, 2023
12 checks passed
@mdo mdo deleted the main-jd-use-text-bg-colors-for-all-badges branch October 23, 2023 19:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants