Skip to content
This repository has been archived by the owner on Sep 5, 2024. It is now read-only.

Commit

Permalink
fix(theming): BREAKING: no longer apply md-default-theme class to unn…
Browse files Browse the repository at this point in the history
…ested themable elements

closes #4846

Themable elements no longer have a `.md-default-theme` class applied to
them if they are not inside of a directive explicitly setting
`md-theme="default"` this helps reduce specificity making it easier to
override angular material's default style. At the same time, the rule is
applied if `md-theme="default"` so that the (presumably nested) theme
can have higher specificity than other themes in the DOM.

Fix required: Stylesheets should not target elements using the
`md-default-theme` selector. Overrides should happen by targeting non
angular-material provided classes. As a quick fix, a
`md-theme="default"` at the `ng-app` level will result in the
`md-default`theme` class being applied as it was before this fix.
Theming related styles now target `md-default-theme` making it harder to
override when the class is applied.
  • Loading branch information
rschmukler committed Oct 12, 2015
1 parent 7393782 commit 5eb94a5
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 22 deletions.
17 changes: 8 additions & 9 deletions src/components/card/card.spec.js
Original file line number Diff line number Diff line change
@@ -1,16 +1,15 @@
describe('mdCard directive', function() {

beforeEach(module('material.components.card'));
var $mdThemingMock = function() { $mdThemingMock.called = true; };

it('should have the default theme class when the md-theme attribute is not defined', inject(function($compile, $rootScope) {
var card = $compile('<md-card></md-card>')($rootScope.$new());
$rootScope.$apply();
expect(card.hasClass('md-default-theme')).toBe(true);
beforeEach(module(function($provide) {
$provide.value('$mdTheming', $mdThemingMock);
}));

it('should have the correct theme class when the md-theme attribute is defined', inject(function($compile, $rootScope) {
var card = $compile('<md-card md-theme="green"></md-card>')($rootScope.$new());
$rootScope.$apply();
expect(card.hasClass('md-green-theme')).toBe(true);
beforeEach(module('material.components.card'));

it('should be themable', inject(function($compile, $rootScope) {
$compile('<md-card></md-card>')($rootScope.$new());
expect($mdThemingMock.called).toBe(true);
}));
});
7 changes: 4 additions & 3 deletions src/core/services/theming/theming.js
Original file line number Diff line number Diff line change
Expand Up @@ -322,15 +322,16 @@ function ThemingProvider($mdColorPalette) {
var attrThemeValue = el.attr('md-theme-watch');
if ( (alwaysWatchTheme || angular.isDefined(attrThemeValue)) && attrThemeValue != 'false') {
var deregisterWatch = $rootScope.$watch(function() {
return ctrl && ctrl.$mdTheme || defaultTheme;
return ctrl && ctrl.$mdTheme || (defaultTheme == 'default' ? '' : defaultTheme);
}, changeTheme);
el.on('$destroy', deregisterWatch);
} else {
var theme = ctrl && ctrl.$mdTheme || defaultTheme;
var theme = ctrl && ctrl.$mdTheme || (defaultTheme == 'default' ? '' : defaultTheme);
changeTheme(theme);
}

function changeTheme(theme) {
if (!theme) return;
if (!registered(theme)) {
$log.warn('Attempted to use unregistered theme \'' + theme + '\'. ' +
'Register it with $mdThemingProvider.theme().');
Expand Down Expand Up @@ -438,7 +439,7 @@ function parseRules(theme, colorType, rules) {
// Don't apply a selector rule to the default theme, making it easier to override
// styles of the base-component
if (theme.name == 'default') {
newRule = newRule.replace(/\.md-default-theme/g, '');
newRule = newRule.replace(/((\w|-)+)\.md-default-theme((\.|\w|-|:|\(|\)|\[|\]|"|'|=)*)/g, '$&, $1$3');
}
generatedRules.push(newRule);
});
Expand Down
26 changes: 16 additions & 10 deletions src/core/services/theming/theming.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -280,7 +280,7 @@ describe('$mdThemingProvider', function() {
var hue1 = themingProvider._rgba(themingProvider._PALETTES.testPalette['300'].value, '0.3');
var hue2 = themingProvider._rgba(themingProvider._PALETTES.testPalette['800'].value, '0.3');
var hue3 = themingProvider._rgba(themingProvider._PALETTES.testPalette.A100.value, '0.3');
result = parse('.md-THEME_NAME-theme.md-primary { color: "{{primary-color-0.3}}"; }');
var result = parse('.md-THEME_NAME-theme.md-primary { color: "{{primary-color-0.3}}"; }');
expect(result[0]).toEqual({content: 'color: ' + primary + ';', hue: null, type: 'primary'});
expect(result[1]).toEqual({content: 'color: ' + hue1 + ';', hue: 'hue-1', type: 'primary'});
expect(result[2]).toEqual({content: 'color: ' + hue2 + ';', hue: 'hue-2', type: 'primary'});
Expand Down Expand Up @@ -331,11 +331,6 @@ describe('$mdTheming service', function() {
});


it('applies a default theme if no theme can be found', inject(function($mdTheming) {
$mdTheming(el);
expect(el.hasClass('md-default-theme')).toBe(true);
}));

it('supports setting a different default theme', function() {
$mdThemingProvider.setDefaultTheme('other');
inject(function($rootScope, $compile, $mdTheming) {
Expand All @@ -359,8 +354,9 @@ describe('$mdTheming service', function() {
}));

it('provides the md-themable directive', function() {
$mdThemingProvider.setDefaultTheme('some');
el = compileAndLink('<h1 md-themable></h1>');
expect(el.hasClass('md-default-theme')).toBe(true);
expect(el.hasClass('md-some-theme')).toBe(true);
});

it('can inherit from explicit parents', inject(function($rootScope, $mdTheming) {
Expand Down Expand Up @@ -395,7 +391,7 @@ describe('md-theme directive', function() {
it('warns when an unregistered theme is used', function() {
inject(function($log, $compile, $rootScope) {
spyOn($log, 'warn');
var el = $compile('<div md-theme="unregistered"></div>')($rootScope);
$compile('<div md-theme="unregistered"></div>')($rootScope);
$rootScope.$apply();
expect($log.warn).toHaveBeenCalled();
});
Expand All @@ -404,7 +400,7 @@ describe('md-theme directive', function() {
it('does not warn when a registered theme is used', function() {
inject(function($log, $compile, $rootScope) {
spyOn($log, 'warn');
var el = $compile('<div md-theme="default"></div>')($rootScope);
$compile('<div md-theme="default"></div>')($rootScope);
$rootScope.$apply();
expect($log.warn.calls.count()).toBe(0);
});
Expand Down Expand Up @@ -447,7 +443,7 @@ describe('md-themable directive', function() {

it('should support watching parent theme by default', function() {
$mdThemingProvider.alwaysWatchTheme(true);
inject(function($rootScope, $compile, $mdTheming) {
inject(function($rootScope, $compile) {
$rootScope.themey = 'red';
var el = $compile('<div md-theme="{{themey}}"><span md-themable></span></div>')($rootScope);
$rootScope.$apply();
Expand All @@ -457,4 +453,14 @@ describe('md-themable directive', function() {
expect(el.children().hasClass('md-red-theme')).toBe(true);
});
});

it('should not apply a class for an unnested default theme', inject(function($rootScope, $compile) {
var el = $compile('<div md-themable></div>')($rootScope);
expect(el.hasClass('md-default-theme')).toBe(false);
}));

it('should apply a class for a nested default theme', inject(function($rootScope, $compile) {
var el = $compile('<div md-theme="default" md-themable></div>')($rootScope);
expect(el.hasClass('md-default-theme')).toBe(true);
}));
});

0 comments on commit 5eb94a5

Please sign in to comment.