-
-
Notifications
You must be signed in to change notification settings - Fork 78.8k
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
[v5] Warnings and invalid CSS properties with enabled shadows but set to none #32412
Comments
@filkris Based on the |
That's right, and this is because we use this mixin with multiple arguments (eg. in Moreover if you want to globally disable shadows, you should set Does this explanation answer or help? |
We ran a few examples isolating the mixin and found out how it works. However, when you see something like this in variables: $btn-box-shadow: inset 0 1px 0 rgba($white, .15), 0 1px 1px rgba($black, .075) !default; there is a great chance to change it intuitively to: $btn-box-shadow: none; not to Maybe you can consider replacing the @warn on L10 with @error rule to prevent generation of invalid CSS code or bypassing it somehow. It is not just in terms of intuition, please take a look at this slippery example in _form-control.scss: @if $enable-shadows {
@include box-shadow($input-box-shadow, $input-focus-box-shadow);
} @else {
// Avoid using mixin so we can pass custom focus shadow properly
box-shadow: $input-focus-box-shadow;
} While making a UI, if you set The next problem is located in _buttons.scss on L37, L57 and L107 lines: @if $enable-shadows {
@include box-shadow($btn-active-box-shadow, 0 0 0 $btn-focus-width rgba(mix($color, $border, 15%), .5));
} @else {
// Avoid using mixin so we can pass custom focus shadow properly
box-shadow: 0 0 0 $btn-focus-width rgba(mix($color, $border, 15%), .5);
} Again, if your From our point of view, a framework is a tool to make your life easier. Dealing with shadows, when and what should be set to Documentation do not describe anything about this. As we noticed, it says only At the end, all of this just sets the simple |
Both of your examples are for cases where a transparent box-shadow is used as a focus ring on an element, which is integral to element interaction styling. As such, they remain present even when Please reference this PR that involves your issue and has the first of at least two refences in the migration guide documentation involving this mixin. Also reference the customization guide that explains |
We are not talking about "migration guide documentation". As we said, its really weird to disable box shadows if |
I can see what you're saying about not knowing what value to set them to when attempting to disable them. I think the core issue here is that - as far as I can tell - the variables you mentioned all relate to form element interaction and were never designed to be fully disabled. Doing so would cause interacting with form elements to be confusing due to a lack of feedback when interacting with them since the framework has no built-in "fallback" feedback in place. I think the best way to alleviate this would be some comments near the variable definitions to warn to not disable them, or at least if they are disabled that a custom replacement is required in order to not negatively affect form element interactions. |
As @voltaek said, And that's why there is "such complicated API around one simple CSS property": that's not about the property itself, but its usage. If we ever wanted to simplify the behaviour you're questionning and asking, we'd need to force a fallback (using In other words: if we consider this, what kind of focus indicator would you expect to see when you completely disable shadows? |
The idea behind adding the shadows as a focus indicators is absolutely fine. However, we have two issues here:
The point of this discussion is to make the Bootstrap more flexible and intuitive/friendly not a "feature request" to drop shadows support. Bootstrap as the framework provides shadows to you and that is perfectly fine but if you want, you can disable them and change At the end, we expected from v5 to be more utility oriented, but it looks like it is not. If the creators are satisfied with current implementation of possibly dangerous behavior of this API, there is no need to change anything. |
Would I take some time to discuss this, if that was the case? You're pointing valid issues, that's why I try to understand and answer with questions and suggestions. Clarifying which shadows get removed when And ensuring we output valid CSS too, since it shouldn't be that hard to check. Even though I don't think that'd change the |
There is a change in behaviour though between I understand that we should be using Maybe introduce variables |
Operating System: Windows 10
Browser: Version 87.0.4280.88 (Official Build) (64-bit)
Compiling the Bootstrap v5.0.0beta1 with enabled shadows, but setting some of variables to
none
produces warnings followed by setting invalid CSS properties (Example:box-shadow: inset 0 1px 2px rgba(0, 0, 0, 0.075), none;
).Compiler warnings:
Then we have tried to detect any of calls to the mixin by doing the following search:
From the results, we can see the following variables can not be set to
none
:Please, can you confirm the same behavior?
The text was updated successfully, but these errors were encountered: