Skip to content

Commit

Permalink
fix(theming): fix broken sass expressions nested in theme classes
Browse files Browse the repository at this point in the history
* Fixes certain theme selectors being broken due to uses of the `&` operator at the end of the selector.
* Adds a custom Stylelint rule to catch future improper uses of the ampersand inside themes.

Relates to angular#3928.
Fixes angular#4077.
  • Loading branch information
crisbeto committed Apr 21, 2017
1 parent e7326ee commit ee0c019
Show file tree
Hide file tree
Showing 7 changed files with 101 additions and 49 deletions.
21 changes: 11 additions & 10 deletions src/lib/input/_input-theme.scss
Original file line number Diff line number Diff line change
Expand Up @@ -22,18 +22,19 @@

.mat-input-placeholder {
color: $input-placeholder-color;
}

// :focus is applied to the input, but we apply mat-focused to the other elements
// that need to listen to it.
.mat-focused & {
color: $input-floating-placeholder-color;
// :focus is applied to the input, but we apply mat-focused to the other elements
// that need to listen to it.
.mat-focused .mat-input-placeholder {
color: $input-floating-placeholder-color;

&.mat-accent {
color: $input-underline-color-accent;
}
&.mat-warn {
color: $input-underline-color-warn;
}
&.mat-accent {
color: $input-underline-color-accent;
}

&.mat-warn {
color: $input-underline-color-warn;
}
}

Expand Down
20 changes: 9 additions & 11 deletions src/lib/radio/_radio-theme.scss
Original file line number Diff line number Diff line change
Expand Up @@ -11,28 +11,26 @@

.mat-radio-outer-circle {
border-color: mat-color($foreground, secondary-text);
}

.mat-radio-checked & {
border-color: mat-color($accent);
}
.mat-radio-checked .mat-radio-outer-circle {
border-color: mat-color($accent);
}

.mat-radio-disabled & {
border-color: mat-color($foreground, disabled);
}
.mat-radio-disabled .mat-radio-outer-circle {
border-color: mat-color($foreground, disabled);
}

.mat-radio-inner-circle {
background-color: mat-color($accent);

.mat-radio-disabled & {
background-color: mat-color($foreground, disabled);
}
}

.mat-radio-ripple .mat-ripple-element {
background-color: mat-color($accent, 0.26);
}

.mat-radio-disabled & {
.mat-radio-disabled {
.mat-radio-ripple .mat-ripple-element, .mat-radio-inner-circle {
background-color: mat-color($foreground, disabled);
}
}
Expand Down
5 changes: 5 additions & 0 deletions src/lib/select/_select-theme.scss
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,11 @@
$accent: map-get($theme, accent);
$warn: map-get($theme, warn);

.mat-select-trigger,
.mat-select-arrow {
color: mat-color($foreground, hint-text);
}

.mat-select-underline {
background-color: mat-color($foreground, divider);
}
Expand Down
45 changes: 19 additions & 26 deletions src/lib/slider/_slider-theme.scss
Original file line number Diff line number Diff line change
@@ -1,6 +1,17 @@
@import '../core/theming/palette';
@import '../core/theming/theming';

@mixin _mat-slider-inner-content-theme($palette) {
.mat-slider-track-fill,
.mat-slider-thumb,
.mat-slider-thumb-label {
background-color: mat-color($palette);
}

.mat-slider-thumb-label-text {
color: mat-color($palette, default-contrast);
}
}

@mixin mat-slider-theme($theme) {
$primary: map-get($theme, primary);
Expand All @@ -19,40 +30,22 @@
background-color: $mat-slider-off-color;
}

.mat-slider-track-fill,
.mat-slider-thumb,
.mat-slider-thumb-label {
.mat-primary & {
background-color: mat-color($primary);
}
.mat-primary {
@include _mat-slider-inner-content-theme($primary);
}

.mat-accent & {
background-color: mat-color($accent);
}
.mat-accent {
@include _mat-slider-inner-content-theme($accent);
}

.mat-warn & {
background-color: mat-color($warn);
}
.mat-warn {
@include _mat-slider-inner-content-theme($warn);
}

.mat-slider-focus-ring {
background-color: $mat-slider-focus-ring-color;
}

.mat-slider-thumb-label-text {
.mat-primary & {
color: mat-color($primary, default-contrast);
}

.mat-accent & {
color: mat-color($accent, default-contrast);
}

.mat-warn & {
color: mat-color($warn, default-contrast);
}
}

.mat-slider:hover,
.cdk-focused {
.mat-slider-track-background {
Expand Down
5 changes: 4 additions & 1 deletion src/lib/tabs/_tabs-theme.scss
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,11 @@
.mat-tab-nav-bar,
.mat-tab-header {
border-bottom: $header-border;
}

.mat-tab-group-inverted-header & {
.mat-tab-group-inverted-header {
.mat-tab-nav-bar,
.mat-tab-header {
border-top: $header-border;
border-bottom: none;
}
Expand Down
8 changes: 7 additions & 1 deletion stylelint-config.json
Original file line number Diff line number Diff line change
@@ -1,9 +1,15 @@
{
"plugins": [
"./tools/stylelint/no-prefixes/no-prefixes.js"
"./tools/stylelint/no-prefixes/no-prefixes.js",
"./tools/stylelint/selector-nested-pattern-scoped/index.js"
],
"rules": {
"material/no-prefixes": [["last 2 versions", "not ie <= 10", "not ie_mob <= 10"]],
"material/selector-nested-pattern-scoped": [".*[^&]$", {
"message": "The & operator is not allowed at the end of theme selectors.",
"filePattern": "-theme\\.scss$"
}],

"color-hex-case": "lower",
"color-no-invalid-hex": true,

Expand Down
46 changes: 46 additions & 0 deletions tools/stylelint/selector-nested-pattern-scoped/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
const stylelint = require('stylelint');
const path = require('path');
const isStandardSyntaxRule = require('stylelint/lib/utils/isStandardSyntaxRule');
const isStandardSyntaxSelector = require('stylelint/lib/utils/isStandardSyntaxSelector');

const ruleName = 'material/selector-nested-pattern-scoped';
const messages = stylelint.utils.ruleMessages(ruleName, {
expected: selector => `Expected nested selector '${selector}' to match specified pattern`,
});

/**
* Re-implementation of the `selector-nested-pattern` Stylelint rule, allowing us
* to scope it to a particular set of files via the custom `filePattern` option. The
* primary use-case is to be able to apply the rule only to theme files.
*
* Reference: https://stylelint.io/user-guide/rules/selector-nested-pattern/
* Source: https://github.com/stylelint/stylelint/blob/master/lib/rules/selector-nested-pattern/
*/
const plugin = stylelint.createPlugin(ruleName, (pattern, options) => {
return (root, result) => {
const selectorPattern = new RegExp(pattern);
const filePattern = new RegExp(options.filePattern);
const fileName = path.basename(root.source.input.file);

if (!filePattern.test(fileName)) return;

root.walkRules(rule => {
if (rule.parent.type === 'rule' &&
isStandardSyntaxRule(rule) &&
isStandardSyntaxSelector(rule.selector) &&
!selectorPattern.test(rule.selector)) {

stylelint.utils.report({
result,
ruleName,
message: messages.expected(rule.selector),
node: rule
});
}
});
};
});

plugin.ruleName = ruleName;
plugin.messages = messages;
module.exports = plugin;

0 comments on commit ee0c019

Please sign in to comment.