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

Ensure decent contrast in color-related utilities #34045

Closed
wants to merge 2 commits into from

Conversation

ffoodd
Copy link
Member

@ffoodd ffoodd commented May 20, 2021

Fixes #34016

Drafting this because it's valuable for Bootstrap's overall accessibility, just a I did with :focus-visible polyfill. Would regret not trying :)

Inspired by Boosted, I still think this is highly valuable by getting decent contrasts for free using our utilities — and no more requiring eg. .text-dark alongside .bg-info.

Things to consider:

  • Might be considered a BC, even if nothing will break: colors and background will change and while it's certainly for the best, it'll obviously get back to us as issues and discussions (RTL is recent, isn't it?).
  • Always ignoring !important to ease overrides, in order to still use .text-* and .bg-* together.
  • Could lead to side-effect when eg. a component part uses a color and a .bg-*: since utilities are added last in our build, .bg-* according color would override the component part's one if its selector is a class (or equivalent specificity).

I didn't met such cases while working with Boosted for years so I'm pretty confident, however Bootstrap is a bit more popular and used :D

To do

If this PR gets positive feedback, before merging we'll need to:

  • drop a few .text-* or .bg-* throughout our docs (and examples)
  • updates utilities documentation
  • mention this in the migration guide, somehow: it'll ease the migration from badge badge-info from v4 to badge bg-info instead of badge bg-info text-dark—and we'll probably find other cases.

@ffoodd
Copy link
Member Author

ffoodd commented May 21, 2021

Friendly ping @mdo, if this sounds good to you I'll tackle the to-do list :)

@mdo
Copy link
Member

mdo commented May 25, 2021

  • Could lead to side-effect when eg. a component part uses a color and a .bg-*: since utilities are added last in our build, .bg-* according color would override the component part's one if its selector is a class (or equivalent specificity).

I think that's the biggest issue here. I don't think we can introduce variability into our utilities like this. It solves the badge problem, but introduces new complexities across other components IMO. If we want badges back to their v4 behavior, we should move them back to the .badge .badge-* behavior.

Maybe we need to test it more across our components to see if we have any unexpected results?

@ffoodd
Copy link
Member Author

ffoodd commented May 26, 2021

Got a few side-effects:

There might be more (in examples, or part of the docs) and to be safe, we'd need to check components with a parent background and / or text utility.

I seriously think this is highly valuable for Bootstrap, but I have to admit that's it's not trivial at all. Even if it might not be a BC IMHO, it'd require many small adjustments here and there in our core.

Since the current need is only for badges, I won't invest more time on this if there's not an agreement on this, so @mdo feel free to close this if it still seems inaccurate to you :)

At least PR exists and might be useful for someone.

Sidenote: this change should concern colored links and outline buttons too, if we're going this way.

In such case, I also miss Percy action I had on Boosted to check visual diff 🤖

@ffoodd ffoodd closed this Feb 28, 2022
@XhmikosR XhmikosR deleted the main-fod-utilities-contrast branch March 27, 2023 09:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Apply .text-dark for .badge with some .bg-* classes
2 participants