[v5] Warnings and invalid CSS properties with enabled shadows but set to none

[filkris]

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:

WARNING: The keyword 'none' must be used as a single argument.
    node_modules\bootstrap\scss\mixins\_box-shadow.scss 10:9   box-shadow()
    node_modules\bootstrap\scss\forms\_form-control.scss 40:7  @import
    node_modules\bootstrap\scss\_forms.scss 3:9                @import
    resources\sass\app.scss 23:9                               root stylesheet

WARNING: The keyword 'none' must be used as a single argument.
    node_modules\bootstrap\scss\mixins\_box-shadow.scss 10:9  box-shadow()
    node_modules\bootstrap\scss\_buttons.scss 40:7            @import
    resources\sass\app.scss 24:9                              root stylesheet

WARNING: The keyword 'none' must be used as a single argument.
    node_modules\bootstrap\scss\mixins\_box-shadow.scss 10:9  box-shadow()
    node_modules\bootstrap\scss\mixins\_buttons.scss 37:7     button-variant()
    node_modules\bootstrap\scss\_buttons.scss 60:5            @import
    resources\sass\app.scss 24:9                              root stylesheet

WARNING: The keyword 'none' must be used as a single argument.
    node_modules\bootstrap\scss\mixins\_box-shadow.scss 10:9  box-shadow()
    node_modules\bootstrap\scss\mixins\_buttons.scss 57:9     button-variant()
    node_modules\bootstrap\scss\_buttons.scss 60:5            @import
    resources\sass\app.scss 24:9                              root stylesheet

WARNING: The keyword 'none' must be used as a single argument.
    node_modules\bootstrap\scss\mixins\_box-shadow.scss 10:9  box-shadow()
    node_modules\bootstrap\scss\mixins\_buttons.scss 107:9    button-outline-variant()
    node_modules\bootstrap\scss\_buttons.scss 66:5            @import
    resources\sass\app.scss 24:9                              root stylesheet

WARNING: The keyword 'none' must be used as a single argument.
    node_modules\bootstrap\scss\mixins\_box-shadow.scss 10:9   box-shadow()   
    node_modules\bootstrap\scss\forms\_form-control.scss 40:7  @import        
    node_modules\bootstrap\scss\_forms.scss 3:9                @import        
    resources\sass\app.scss 23:9                               root stylesheet

WARNING: The keyword 'none' must be used as a single argument.
    node_modules\bootstrap\scss\mixins\_box-shadow.scss 10:9  box-shadow()
    node_modules\bootstrap\scss\forms\_form-select.scss 30:7  @import     
    node_modules\bootstrap\scss\_forms.scss 4:9               @import     
    resources\sass\app.scss 23:9                              root stylesheet

Then we have tried to detect any of calls to the mixin by doing the following search:

Searching 79 files for "@include box-shadow("

bootstrapv5\node_modules\bootstrap\scss\_button-group.scss:
  101:   @include box-shadow($btn-active-box-shadow);
  105:     @include box-shadow(none);

bootstrapv5\node_modules\bootstrap\scss\_buttons.scss:
   37:     @include box-shadow($btn-active-box-shadow);
   40:       @include box-shadow($btn-focus-box-shadow, $btn-active-box-shadow);
   49:     @include box-shadow(none);

bootstrapv5\node_modules\bootstrap\scss\_dropdown.scss:
   34:   @include box-shadow($dropdown-box-shadow);
  201:   @include box-shadow($dropdown-dark-box-shadow);

bootstrapv5\node_modules\bootstrap\scss\_images.scss:
   19:   @include box-shadow($thumbnail-box-shadow);

bootstrapv5\node_modules\bootstrap\scss\_modal.scss:
   90:   @include box-shadow($modal-content-box-shadow-xs);
  188:     @include box-shadow($modal-content-box-shadow-sm-up);

bootstrapv5\node_modules\bootstrap\scss\_popover.scss:
   18:   @include box-shadow($popover-box-shadow);

bootstrapv5\node_modules\bootstrap\scss\_progress.scss:
   15:   @include box-shadow($progress-box-shadow);

bootstrapv5\node_modules\bootstrap\scss\forms\_form-control.scss:
   22:   @include box-shadow($input-box-shadow);
   40:       @include box-shadow($input-box-shadow, $input-focus-box-shadow);

bootstrapv5\node_modules\bootstrap\scss\forms\_form-range.scss:
   34:     @include box-shadow($form-range-thumb-box-shadow);
   51:     @include box-shadow($form-range-track-box-shadow);
   60:     @include box-shadow($form-range-thumb-box-shadow);
   77:     @include box-shadow($form-range-track-box-shadow);

bootstrapv5\node_modules\bootstrap\scss\forms\_form-select.scss:
   23:   @include box-shadow($form-select-box-shadow);
   30:       @include box-shadow($form-select-box-shadow, $form-select-focus-box-shadow);

bootstrapv5\node_modules\bootstrap\scss\mixins\_buttons.scss:
   23:   @include box-shadow($btn-box-shadow);
   37:       @include box-shadow($btn-box-shadow, 0 0 0 $btn-focus-width rgba(mix($color, $border, 15%), .5));
   57:         @include box-shadow($btn-active-box-shadow, 0 0 0 $btn-focus-width rgba(mix($color, $border, 15%), .5));
  107:         @include box-shadow($btn-active-box-shadow, 0 0 0 $btn-focus-width rgba($color, .5));

24 matches across 11 files

From the results, we can see the following variables can not be set to none:

$input-box-shadow: none;
$input-focus-box-shadow: none;

$btn-box-shadow: none;
$btn-focus-box-shadow: none;
$btn-active-box-shadow: none;

$form-select-box-shadow: none;
$form-select-focus-box-shadow: none;

Please, can you confirm the same behavior?

[voltaek]

@filkris Based on the box-shadow() mixin code, it looks like the intent is to set the variables to null for them to be ignored.

[ffoodd]

That's right, and this is because we use this mixin with multiple arguments (eg. in _form-control.scss, L40, @include box-shadow($input-box-shadow, $input-focus-box-shadow);). Setting one of the argument with none results in an invalid CSS value.

Moreover if you want to globally disable shadows, you should set $enable-shadows to false. Then using none as a value would probably work — but using null will always be better and recommended since it prevents the property to output.

Does this explanation answer or help?

[filkris]

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 null what is wrong when $enable-shadows is set to true. This will throw just a warning and when you refresh a page, the shadow is gone (because of invalid CSS property value, not because the property is set to none or do not exists). Producing a invalid code inside the framework, where a lot of developers barely takes a look is "weird".

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 $enable-shadows to false (default), however the shadow will be generated, and if you need to disable it, the only way to do it is to set a $input-focus-box-shadow to none (setting it to null will produce box-shadow: null; what is invalid too). Otherwise, if you set $enable-shadows to true and want to disable just some of shadows (for example: $input-focus-box-shadow) you need to change each desired variable from none to null.

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 $enable-shadows is set to false (default) you will get one, but if you set it to true, there is no way to disable it except doing rewrites.

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 none, null or rewritten in v5 is a night mare. Do not forget rewrites like @include box-shadow($form-select-box-shadow, $form-select-focus-box-shadow); as well, what makes us to set both variables to null or maybe none not even sure what is status of $enable-shadows).

Documentation do not describe anything about this. As we noticed, it says only $enable-shadows - "Enables predefined box-shadow styles on various components.", what is not 100% true, not mentioning anything about shadows when the option is set to false or when to use null, when none.

At the end, all of this just sets the simple box-shadow property, nothing more. Is there any reason for such complicated API around one simple CSS property?

[voltaek]

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 $enable-shadows is false (the framework's default), as they do not fall under the same "extra styling" as is added to the styles output by having $enable-shadows enabled and must be kept.

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 null as being commonly used to disable variable usage here.

[filkris]

We are not talking about "migration guide documentation". As we said, its really weird to disable box shadows if $enable-shadows is set to true by setting other variables to null, otherwise setting them to none. Sometimes it is null, sometimes it is none, but if you make a mistake, the invalid CSS property is guaranteed. This can be too much especially for beginners.

[voltaek]

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.

[ffoodd]

As @voltaek said, $enable-shadows stands for decorative shadows. The one you tried to disable (let's call them functionnal shadows) using null should'nt be disabled, as you're basically dropping focus indicators.

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 outline or something?) to ensure you'd still have a sufficient focus indicator.

In other words: if we consider this, what kind of focus indicator would you expect to see when you completely disable shadows?
Or would a clarification in our docs regarding those functionnal shadows be enough to get out of trouble?

[filkris]

The idea behind adding the shadows as a focus indicators is absolutely fine. However, we have two issues here:

1. Documentation

   What does $enable-shadows really mean? It "enables predefined box-shadow styles on various components" by documentation. That should mean, if we set this parameter to false, we will drop the shadows framework support, but it is not true.

   This can be confusing not just to newcomers but everyone.

2. Design of the shadows API

   The API is giving a possibility to developers to instruct the framework to generate an invalid CSS code with no intention. The core of this issue is the non-compliance of box shadow values. Depending on the value of $enable-shadows you have to use none or null to achieve the same thing.

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 border-color on focus for example... In this scenario, it is hard to understand how to disable them and possibly dangerous since you can end up with invalid CSS what will make a scenario where they are not disabled, but they are not appearing because of invalid CSS at same time.

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.

[ffoodd]

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 $enable-shadows is set to false feels accurate, and should be done in several places in our docs I guess.

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 null vs none thing, clarifying what's hapenning should help.