Extended List group variants capabilities

[ydmitry]

Refs to #31538 split some part from #31574

[ydmitry]

Can add docs and $list-group-custom-properties variable similar to #31753

[ffoodd]

Thanks for the PR!

The way you did this feels very weird though, compared to our existing components. Why not simply use your values as default values for mixin agument?

Eg:

@mixin list-group-item-variant(
  $state,
  $bg-color: color-level($value, $list-group-item-bg-level),
  $color: color-level($value, $list-group-item-color-level),
  $hover-bg-color: darken($bg-color, 5%),
  $hover-color: $color,
  $active-bg-color: $color,
  $active-color: color-contrast($color)
) {

Am I missing something?

PS: I also used our color-contrast() function to ensure contrast for active state.

[ydmitry]

@ffoodd if we just use it as a default value in mixin we won't be able on application side by just changing a $list-group-item-variants map to set different values for $hover-bg-color, $hover-color, etc.

If we do it only on mixin default variable to change them I would need to:

1. Set everything in $list-group-item-variants to null
2. Create new map $app-list-group-item-variants and write own @each:

@each $color, $value in $app-list-group-item-variants {
  @if type-of($value) == "map" {
    @include list-group-item-variant(
      $color,
      map-get($value, bg-color),
      map-get($value, color),
      map-get($value, hover-bg-color),
      map-get($value, hover-color),
      map-get($value, active-bg-color),
      map-get($value, active-color)
    );
  }
}

[ffoodd]

That feels very weird to me, we have a mixin with arguments so no need to add another inerface between variables and mixin: if one need to customize it from Sass, it would be better to use the mixin out of a loop, wouldn't it?

Let's wait for other opinions on this, I'm not against but questionning :)

[ydmitry]

@ffoodd we can use spread operator, checked if it will work in this scss gist: https://www.sassmeister.com/gist/aa12c7e6678298c1492919d2c016ace4?token=043f800ce522585e46e6df8f9922d34da00023e9&scope=gist,read:user

So we will be able to pass not full list of variables and then default values will be used. And if we need customization we just will add key to map

[ydmitry]

@ffoodd added a commit with fixing usage of contrast-color from your comment and used default values in mixins by passing maps via spread operator, so if you need to customize, need just to add e.g.:

// This is in applications SCSS:
$list-group-item-variants: (
   custom: (
     bg-color: color-level($purple, $list-group-item-bg-level),
     color: color-level($purple, $list-group-item-color-level)
     // Other properties of mixin are optional now
  ),
  primary: (
     bg-color: color-level($primary, $list-group-item-bg-level),
     color: color-level($primary, $list-group-item-color-level)
     hover-bg-color: lighten($primary, 10%) // now it's lighten, not darken
   )
);

If this will be better, I can update #31757 and #31753

[mdo]

Closing per my comment at #31538 (comment).

Let's please revisit this one-by-one, component-by-component, so we don't waste your time and effort.