feat(elevation): preserve the alpha value of the provided color

[benelliott]

Ensure that if the elevation shadow color is overridden with a color that has an alpha value, that
value is propagated to the opacity of the shadow itself. This is useful for users who want to
"tone down" the default shadows without changing their color tone.

[jelbourn]

This seems a bit odd to me. I don't really follow the reasoning behind taking 20% of the given opacity. Could you elaborate on this?

Additionally, this would be a breaking change for anyone depending on the existing behavior, so it would either have to be a new opt-in API or just keep the existing behavior.

[benelliott]

@jelbourn If you look at a lot of Google Material Design sites (such as Docs and Search), you will see that the shadows are often less dark than those provided by @angular/material. When making sites with the library I often have requests from stakeholders to "tone the shadows down".

The mat-elevation mixin does allow users to override the default shadow colour (black) but it knocks out the alpha value of the colour you pass in. You can see this behaviour here. The only way to "lighten" elevation shadows in the current implementation is to use a paler colour (i.e. grey), but this can give an undesirable appearance against a colourful background.

This pull request allows users to pass in rgba colours to mat-elevation and for the alpha value of those colours to proportionately affect the opacity of the shadow. Thus you can "weaken" shadows like so: @include mat-elevation($zValue, rgba(0, 0, 0, 0.5)).

[jelbourn]

Accepting the opacity provided is good, but doing * .2 seems suprising. I would do it as:

@function _get-umbra-map($color) {
  $opacity: opacity($color);
  $opacity: if($opacity == null or $opacity == 1 or $opacity == 0, 0.2, $opacity);
  $shadow-color: rgba($color, $opacity);

  @return (
    0: '0px 0px 0px 0px #{$shadow-color}',
    1: '0px 2px 1px -1px #{$shadow-color}',
    2: '0px 3px 1px -2px #{$shadow-color}',
    3: '0px 3px 3px -2px #{$shadow-color}',
    4: '0px 2px 4px -1px #{$shadow-color}',
    5: '0px 3px 5px -1px #{$shadow-color}',
    6: '0px 3px 5px -1px #{$shadow-color}',
    7: '0px 4px 5px -2px #{$shadow-color}',
    8: '0px 5px 5px -3px #{$shadow-color}',
    9: '0px 5px 6px -3px #{$shadow-color}',
    10: '0px 6px 6px -3px #{$shadow-color}',
    11: '0px 6px 7px -4px #{$shadow-color}',
    12: '0px 7px 8px -4px #{$shadow-color}',
    13: '0px 7px 8px -4px #{$shadow-color}',
    14: '0px 7px 9px -4px #{$shadow-color}',
    15: '0px 8px 9px -5px #{$shadow-color}',
    16: '0px 8px 10px -5px #{$shadow-color}',
    17: '0px 8px 11px -5px #{$shadow-color}',
    18: '0px 9px 11px -5px #{$shadow-color}',
    19: '0px 9px 12px -6px #{$shadow-color}',
    20: '0px 10px 13px -6px #{$shadow-color}',
    21: '0px 10px 13px -6px #{$shadow-color}',
    22: '0px 10px 14px -6px #{$shadow-color}',
    23: '0px 11px 14px -7px #{$shadow-color}',
    24: '0px 11px 15px -7px #{$shadow-color}'
  );
}

[benelliott]

@jelbourn This wouldn't have the effect I am trying to create, which is that if you pass rgba(0,0,0,0.6) into mat-elevation then you would have the same box shadow "style" as before (ie. when using rgb(0,0,0), but with the shadows at 60% of their original strength).

The reason I multiply by 0.2 is that 0.2 is the scale that had been originally determined to be correct for the umbra map. If you instead go by your snippet, this will mean that each component of the box shadow (umbra, penumbra, ambient) will have the same opacity, overriding their designed values and degrading the visuals.

Here is a codepen to illustrate what I mean: https://codepen.io/benelliott/pen/KRggmz

I do however agree it would be cleaner to do this multiplication once at the top of each function and then to reuse the result. Happy to amend the PR.

[jelbourn]

I think it's more difficult to explain that the color provided is the "base" color that is then modified to get a more shadowy opacity rather than just saying "The shadow will use the opacity provided". An alternative would be to add an extra optional opacity argument to the mixins and use that if it is present.

[benelliott]

@jelbourn Maybe we see the API differently. I think of it as a function which takes a color and returns a Material-Design-style shadow for that color, including the necessary modifications to its opacity in the different components of the shadow. I also think of the default shadow as being fully opaque black, rgba(0,0,0,1). So for me the expected behaviour is that when I instead provide the function with rgba(0,0,0,0.6), the shadow would look the same as it did with black, but only 60% of the strength. As a user of the API I don't care what transformation it has to do in order to give me a Material shadow; hence it can transform the color's opacity if it wants to.

Would you prefer a PR that looked more like:

@mixin mat-elevation($zValue, $color: black, $opacity: 1) {
  @if type-of($zValue) != number or not unitless($zValue) {
    @error '$zValue must be a unitless number';
  }
  @if $zValue < 0 or $zValue > 24 {
    @error '$zValue must be between 0 and 24';
  }

  box-shadow: #{map-get(_get-umbra-map($color), $zValue, $opacity)},
              #{map-get(_get-penumbra-map($color), $zValue, $opacity)},
              #{map-get(_get-ambient-map($color), $zValue, $opacity)};
}

@function _get-umbra-map($color, $opacity) {
  @return (
    0: '0px 0px 0px 0px #{rgba($color, $opacity * 0.2)}',
    ...
   );
}

[jelbourn]

After sleeping on it over the weekend, I'm on board with using the existing opacity as a coefficient now, since otherwise it's too hard to customize while still keeping the umbra/penumbra/ambient blend.

I do think that the opacity has to be a separate argument, per your last comment, since otherwise it would be a breaking change (even though I suspect not many people are using this mixin directory). Would you be able to make that change and then also update elevation.md to document that API?

[benelliott]

@jelbourn I've addressed your comment and squashed the changes into a single commit. Let me know what you think!

Having played around with elevation a bit I feel like it makes sense for Material components' @include mat-elevation(..) rules to be pulled out into the themes, as this would allow the user to customize the looks of the shadows in the Material components by overriding $mat-elevation-color and $mat-elevation-opacity as well as their own. If you agree I can work on this in a separate PR.

[jelbourn]

LGTM, thanks!

I think moving the elevations out of the core styles would be a breaking change, so we probably wouldn't be able to swing that.

[angular-automatic-lock-bot]

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}