Bootstrap: Ability to remove form-validation-state values before bootstrap variables is loaded

[mstrelan]

I'm working on a project whereby I can't include anything between _variables.scss file and _forms.scss and therefore am unable to use map-remove() to remove the "valid" form-validation-state before it's too late. What I can do is set "valid" to an empty map before the variables are loaded, but then there are errors when _forms.scss calls the form-validation-state() mixin. I'm wondering if there is any sense in adding a check to see if "color" and "icon" exist before calling the mixin, like so:

@each $state, $data in $form-validation-states {
  @if map_has_key($data, color) and map_has_key($data, icon) {
    @include form-validation-state($state, map-get($data, color), map-get($data, icon));
  }
}

[ysds]

At first, I had a negative impression of this proposal, but after thinking about this for a while I became interested.

I look not only at validation but also the whole. Currently, the way to remove items from a map, such as theme colors, requires a lot of work (import each our sass files), but addition and modification are easy.

https://getbootstrap.com/docs/4.3/getting-started/theming/#remove-from-map

But if we can do like the following controls, customization will be easier and consistency:

$theme-colors: (
  "primary": #0074d9, // Modify
  "custom-color": #900 // Add
  "info": null // Remove!!
);

// Bootstrap and its default variables
@import "../node_modules/bootstrap/scss/bootstrap";

Necessary changes in Bootstrap may be:

@each $color, $value in $theme-colors {
  @if ($value != null) {
    .alert-#{$color} {
      @include alert-variant(theme-color-level($color, $alert-bg-level), theme-color-level($color, $alert-border-level), theme-color-level($color, $alert-color-level));
    }
  }
}

[mstrelan]

Yes I agree it should be broader than validation. The checking for null is a bit of a nuisance though. It would be handy if the map-merge function removed any keys with null values. I guess another approach would be to have another variable, e.g. $theme-colors-exclude: () !default; and immediately after the map-merge we loop through the exclusions and remove them from the map. That could possibly work better.

[kylefox]

I think a cleaner solution might be to remove the map-merge of $form-validation-states in _variables.scss completely.

Sure, making form-validation-state ignore blank states _works_, but it feels like a clunky solution. Ideally, it would be easy to prevent the "valid" state from being added to $form-validation-states in the first place.

This currently isn't possible because the "valid" and "invalid" states are forcefully merged into whatever else $form-validation-states contains:

// bootstrap/scss/_variables.scss

$form-validation-states: () !default;

$form-validation-states: map-merge(
  (
    "valid": (
      "color": $form-feedback-valid-color,
      "icon": $form-feedback-icon-valid
    ),
    "invalid": (
      "color": $form-feedback-invalid-color,
      "icon": $form-feedback-icon-invalid
    ),
  ),
  $form-validation-states
);

Whereas if $form-validation-states was merely given a default value, it could easily be provided as a configuration variable, like all other bootstrap configuration:

// bootstrap/scss/_variables.scss

$form-validation-states: (
  "valid": (
    "color": $form-feedback-valid-color,
    "icon": $form-feedback-icon-valid
  ),
  "invalid": (
    "color": $form-feedback-invalid-color,
    "icon": $form-feedback-icon-invalid
  ),
) !default;

// my-app.scss

// ...other Bootstrap configuration here...

$form-validation-states: (
  "invalid": (
    "color": $form-feedback-invalid-color,
    "icon": $form-feedback-icon-invalid
  )
);

@import "bootstrap/scss/bootstrap";

Of course, this means you'd need to provide the color values. But chances are, if your Bootstrap configuration is sophisticated enough that you're customizing form states, you've likely already defined your own color variables.

[Leenie83]

Agree

[mdo]

If the suggestion by @kylefox solves this, that'll be addressed when we ditch all our map merges (see #28508). Thoughts @twbs/css-review?

[MartijnCuppens]

I think ditching the map-merge()s is the way to go. It way easier to understand what's going on and easier to get control over what people (don't) want to generate.

[mdo]

Closing per last comment—the referenced PR has been merged into master for our upcoming v4 release.