From cc693d06e6a55e05cb47a21275c3990f1f60d82c Mon Sep 17 00:00:00 2001 From: Kristiyan Kostadinov Date: Sun, 10 Nov 2024 09:12:08 +0100 Subject: [PATCH] fix(material/core): incorrect validation in get-theme-color Fixes that the `get-theme-color` function was incorrectly validating its number of arguments. We were always checking if the arguments are between 2 and 4, whereas it actually accepts either 2 or 3 for M3 themes and between 2 and 4 for M2 themes. Fixes #29819. --- src/material/core/theming/_inspection.scss | 45 ++++++++++++------- src/material/core/theming/_m2-inspection.scss | 8 ++-- .../tests/theming-inspection-api.spec.ts | 2 +- 3 files changed, 33 insertions(+), 22 deletions(-) diff --git a/src/material/core/theming/_inspection.scss b/src/material/core/theming/_inspection.scss index 7747097aa126..0f0e11fcb4b4 100644 --- a/src/material/core/theming/_inspection.scss +++ b/src/material/core/theming/_inspection.scss @@ -62,10 +62,17 @@ $_typography-properties: (font, font-family, line-height, font-size, letter-spac } } -/// Gets a color from a theme object. This function can take 2 or 3 arguments. If 2 arguments are -/// passed, the second argument is treated as the name of a color role. If 3 arguments are passed, -/// the second argument is treated as the name of a color palette (primary, secondary, etc.) and the -/// third is treated as the palette hue (10, 50, etc.) +/// Gets a color from a theme object. This function take a different amount of arguments depending +/// on if it's working with an M2 or M3 theme: +/// - With an M3 theme it accepts either 2 or 3 arguments. If 2 arguments are passed, the second +/// argument is treated as the name of a color role. If 3 arguments are passed, the second argument +/// is treated as the name of a color palette (primary, secondary, etc.) and the third is treated +/// as the palette hue (10, 50, etc.). +/// - With an M2 theme theme it accepts between 2 and 4 arguments, or the equivalent of calling +/// the `m2-get-theme-color` function. The first argument is the theme, the second one is the +/// palette from which to extract the color, the third one is the hue within the palette and the +/// fourth is the opacity of the returned color. +/// the second one is the /// @param {Map} $theme The theme /// @param {String} $color-role-or-palette-name The name of the color role to get, or the name of a /// color palette. @@ -74,25 +81,29 @@ $_typography-properties: (font, font-family, line-height, font-size, letter-spac /// @return {Color} The requested theme color. @function get-theme-color($theme, $args...) { $version: get-theme-version($theme); - $args-count: list.length($args); - @if $args-count != 1 and $args-count != 2 and $args-count != 3 { - @error #{'Expected between 2 and 4 arguments. Got:'} $args-count + 1; - } + $args-count: list.length($args) + 1; + // M2 theme @if $version == 0 { + @if $args-count < 2 or $args-count > 4 { + @error 'Expected between 2 and 4 arguments when working with an M2 theme. ' + + 'Got: #{$args-count}'; + } @return m2-inspection.get-theme-color($theme, $args...); } - @else if $version == 1 { - @if $args-count == 1 { - @return _get-theme-role-color($theme, $args...); - } - @else if $args-count == 2 { - @return _get-theme-palette-color($theme, $args...); + + // M3 theme + @if $version == 1 { + @if $args-count < 2 or $args-count > 3 { + @error 'Expected either 2 or 3 arguments when working with an M3 theme. Got: #{$args-count}'; } + @return if($args-count == 2, + _get-theme-role-color($theme, $args...), + _get-theme-palette-color($theme, $args...) + ); } - @else { - @error #{'Unrecognized theme version:'} $version; - } + + @error 'Unrecognized theme version: #{$version}'; } /// Gets a role color from a theme object. diff --git a/src/material/core/theming/_m2-inspection.scss b/src/material/core/theming/_m2-inspection.scss index 6463b26d3ebd..f096009282ff 100644 --- a/src/material/core/theming/_m2-inspection.scss +++ b/src/material/core/theming/_m2-inspection.scss @@ -102,14 +102,14 @@ $_typography-properties: (font, font-family, line-height, font-size, letter-spac @return if(map.get($colors, is-dark), dark, light); } -/// Gets a color from a theme object. This function can take 2 or 3 arguments. If 2 arguments are -/// passed, the second argument is treated as the name of a color role. If 3 arguments are passed, -/// the second argument is treated as the name of a color palette (primary, secondary, etc.) and the -/// third is treated as the palette hue (10, 50, etc.) +/// Gets a color from a theme object. This function can take between 2 and 4 arguments. The first +/// argument is the theme, the second one is the palette from which to extract the color, the third +/// one is the hue within the palette and the fourth is the opacity of the returned color. /// @param {Map} $theme The theme /// @param {String} $palette-name The name of a color palette. /// @param {Number} $hue The palette hue to get (passing this argument means the second argument is /// interpreted as a palette name). +/// @param {Number} $opacity The alpha channel value for the color. /// @return {Color} The requested theme color. @function get-theme-color($theme, $palette-name, $args...) { $theme: _get-m2-config($theme); diff --git a/src/material/core/theming/tests/theming-inspection-api.spec.ts b/src/material/core/theming/tests/theming-inspection-api.spec.ts index 6d3f9b74f5aa..b0d3eb11d44f 100644 --- a/src/material/core/theming/tests/theming-inspection-api.spec.ts +++ b/src/material/core/theming/tests/theming-inspection-api.spec.ts @@ -315,7 +315,7 @@ describe('theming inspection api', () => { color: mat.get-theme-color($theme); } `), - ).toThrowError(/Expected between 2 and 4 arguments\. Got: 1/); + ).toThrowError(/Expected either 2 or 3 arguments\. Got: 1/); }); it('should get typography properties from theme', () => {