-
-
Notifications
You must be signed in to change notification settings - Fork 79k
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
Iterate on border utilities #36239
Iterate on border utilities #36239
Conversation
Tested it in border utilities and some components. Indeed it seems safe to drop this 👌
|
@julien-deramond Should we do the global value, or use a local one? We use local ones on our |
Yes indeed you could restore the local and remove the global so they all match. If I am remembering well 🤔 there are some edge cases in https://twbs-bootstrap.netlify.app/docs/5.1/utilities/borders/#additive where you can add "border-color": (
// ...
local-vars: (
"border-opacity": 1
),
// ...
), |
--bs-border-opacity: 1
from .border-*
utilitiesa5be314
to
f78c0d9
Compare
Good eyes @julien-deramond. I think the latest commit addresses all concerns. Played around with mixing the utilities and using them on components. |
The latest commit addresses all concerns. Also played around with mixing the utilities and using them on components and didn't see any regressions. I "approve" this PR :) By the way it would be very interesting to have the |
We set `--bs-border-opacity: 1` globally at the `:root` level, so redeclaring it on every `.border-*` utility doesn't make much sense. I think we can drop this.
…ove .border-color utils down the list to fix some specificity issues
Added some examples @julien-deramond: Also removed the callout that was now out of date regarding the setting of CSS variables only for utilities. We backtracked from that on |
f78c0d9
to
c80bbb2
Compare
We set
--bs-border-opacity: 1
globally at the:root
level, so redeclaring it on every.border-*
utility doesn't make much sense. I think we can drop this.Edit: Per conversation below, I've also removed the global
--bs-border-opacity
and restored it on the.border-{color}
utilities. There's no need for border opacity on default.border
utilities, just the color ones, which now properly matches our.text-{color}
and.bg-{color}
utilities. In addition, this rearranged the utilities map to putborder-color
at the end (without which.border-opacity-*
classes wouldn't work properly.