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

Use box-shadow CSS variables shadow utilities #38816

Merged
merged 2 commits into from Jul 27, 2023
Merged

Use box-shadow CSS variables shadow utilities #38816

merged 2 commits into from Jul 27, 2023

Conversation

craigiswayne
Copy link
Contributor

Description

Currently the shadow utility css classes have the following code:

.shadow {
  box-shadow: 0 .5rem 1rem rgba(0,0,0,.15)!important
}

.shadow-sm {
  box-shadow: 0 .125rem .25rem rgba(0,0,0,.075)!important
}

.shadow-lg {
  box-shadow: 0 1rem 3rem rgba(0,0,0,.175)!important
}

This PR would result in the following change:
Making use of css box-shadow variables in the shadow utility classes

.shadow {
  box-shadow: var(--bs-box-shadow) !important;
}

.shadow-sm {
  box-shadow: var(--bs-box-shadow-sm) !important;
}

.shadow-lg {
  box-shadow: var(--bs-box-shadow-lg) !important;
}

The values of these css variables are unchanged and are as follows

--bs-box-shadow: 0 0.5rem 1rem rgba(0, 0, 0, 0.15);
--bs-box-shadow-sm: 0 0.125rem 0.25rem rgba(0, 0, 0, 0.075);
--bs-box-shadow-lg: 0 1rem 3rem rgba(0, 0, 0, 0.175);

As you can see they are the same value as the sass variables

these box-shadow css variables only seemed to be currently in use by the following:

  • $toast-box-shadow
  • $thumbnail-box-shadow
$toast-box-shadow:                  var(--#{$prefix}box-shadow) !default;
$thumbnail-box-shadow:              var(--#{$prefix}box-shadow-sm) !default;

The resulting values of these box shadow's don't change

Motivation & Context

To allow overriding the shadow utility classes without the use of !important or sass variables

Type of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Refactoring (non-breaking change)
  • Breaking change (fix or feature that would change existing functionality)

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
  • I have added tests to cover my changes
  • All new and existing tests passed

Live previews

Not sure if i need to do this

Related issues

None. There are a few issues related to shadows, but none specifically asking for css variables in the current shadow utility classes

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.

LGTM, Thanks for the contribution!

I wonder if we should change all the occurences of $box-shadow* in our Scss. It can maybe be done afterwards (_variables.scss, _navbar.scss, sidebar.scss).

@mdo mdo changed the title Making use of css box-shadow variables in the shadow utility classes Use box-shadow CSS variables shadow utilities Jul 6, 2023
@mdo
Copy link
Member

mdo commented Jul 6, 2023

@julien-deramond @louismaximepiton Thoughts on making this stuff part of a v5.3.1 or v5.3.2? Wondering if this counts as anything more meaningful than just a general quality of life improvement and consistency change.

@louismaximepiton
Copy link
Member

I think you're right it's only general improvement for quality of life. I guess that it could be merged whenever but I think the sooner the better. However, I don't have a strong opinion whether it should be in v5.3.1 or v5.3.2.

@julien-deramond julien-deramond merged commit c81a694 into twbs:main Jul 27, 2023
15 checks passed
romankupchak93 pushed a commit to romankupchak93/bootstrap that referenced this pull request Jan 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

4 participants