Skip to content
This repository has been archived by the owner on Aug 29, 2023. It is now read-only.

Commit

Permalink
fix(mdMedia): avoid unnecessary digest and make changes apply quicker
Browse files Browse the repository at this point in the history
Previously, the `updateAll()` method relied on `$timeout` for scheduling a
digest. This made the possible changes apply after the next rendering and
could possibly lead to extra digests.

Replaces the call to `$timeout` with a call to `$evalAsync()`,
which avoids an extra digest if one is already in progress and also applies
the changes before the next rendering.

Closes #978.
  • Loading branch information
gkalpak authored and ThomasBurleson committed Dec 24, 2014
1 parent 8736c7c commit 98247bc
Show file tree
Hide file tree
Showing 3 changed files with 78 additions and 18 deletions.
17 changes: 10 additions & 7 deletions src/core/util/media.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ angular.module('material.core')
* @example $mdMedia('(min-width: 1200px)') == true if device-width >= 1200px
* @example $mdMedia('max-width: 300px') == true if device-width <= 300px (sanitizes input, adding parens)
*/
function mdMediaFactory($window, $mdUtil, $timeout, $mdConstant) {
function mdMediaFactory($mdConstant, $mdUtil, $rootScope, $window) {
var queriesCache = $mdUtil.cacheFactory('$mdMedia:queries', {capacity: 15});
var resultsCache = $mdUtil.cacheFactory('$mdMedia:results', {capacity: 15});

Expand Down Expand Up @@ -42,13 +42,16 @@ function mdMediaFactory($window, $mdUtil, $timeout, $mdConstant) {
}

function updateAll() {
var keys = cache.keys();
if (keys.length) {
for (var i = 0, ii = keys.length; i < ii; i++) {
cache.put(keys[i], !!$window.matchMedia(keys[i]).matches);
var keys = resultsCache.keys();
var len = keys.length;

if (len) {
for (var i = 0; i < len; i++) {
add(keys[i]);
}
// trigger a $digest()
$timeout(angular.noop);

// Trigger a $digest() if not already in progress
$rootScope.$evalAsync();
}
}
}
72 changes: 64 additions & 8 deletions src/core/util/media.spec.js
Original file line number Diff line number Diff line change
@@ -1,25 +1,81 @@
describe('$mdMedia', function() {
var matchMediaResult;
var queriesCache;
var resultsCache;


beforeEach(module('material.core'));

var matchMediaResult = false;
beforeEach(inject(function($window) {
beforeEach(inject(function($cacheFactory, $mdMedia, $window) {
matchMediaResult = false;

queriesCache = $cacheFactory.get('$mdMedia:queries');
resultsCache = $cacheFactory.get('$mdMedia:results');

spyOn($window, 'matchMedia').andCallFake(function() {
return { matches: matchMediaResult };
return {matches: matchMediaResult};
});
}));

it('should validate input', inject(function($window, $mdMedia) {
afterEach(function() {
queriesCache.removeAll();
resultsCache.removeAll();
});


it('should look up queries in `$mdConstant.MEDIA`', inject(
function($mdConstant, $mdMedia, $window) {
$mdConstant.MEDIA.somePreset = 'someQuery';

$mdMedia('somePreset');
expect($window.matchMedia).toHaveBeenCalledWith('someQuery');

delete($mdConstant.MEDIA.somePreset);
}
));

it('should look up validated queries in `queriesCache`', inject(function($mdMedia, $window) {
queriesCache.put('originalQuery', 'validatedQuery');

$mdMedia('originalQuery');
expect($window.matchMedia).toHaveBeenCalledWith('validatedQuery');
}));

it('should validate queries', inject(function($mdMedia, $window) {
$mdMedia('something');
expect($window.matchMedia).toHaveBeenCalledWith('(something)');
}));

it('should return result of matchMedia and recalculate on resize', inject(function($window, $mdMedia) {
it('should cache validated queries in `queriesCache`', inject(function($mdMedia) {
$mdMedia('query');
expect(queriesCache.get('query')).toBe('(query)');
}));

it('should return cached results if available', inject(function($mdMedia) {
resultsCache.put('(query)', 'result');
expect($mdMedia('(query)')).toBe('result');
}));

it('should cache results in `resultsCache`', inject(function($mdMedia) {
$mdMedia('(query)');
expect(resultsCache.get('(query)')).toBe(false);
}));

it('should recalculate on resize', inject(function($mdMedia, $window) {
matchMediaResult = true;
expect($mdMedia('foo')).toBe(true);
expect($mdMedia('query')).toBe(true);
expect($window.matchMedia.callCount).toBe(1);

expect($mdMedia('query')).toBe(true);
expect($window.matchMedia.callCount).toBe(1);

matchMediaResult = false;
expect($mdMedia('foo')).toBe(true);
expect($mdMedia('query')).toBe(true);
expect($window.matchMedia.callCount).toBe(1);

angular.element($window).triggerHandler('resize');
expect($mdMedia('foo')).toBe(false);

expect($mdMedia('query')).toBe(false);
expect($window.matchMedia.callCount).toBe(2);
}));
});
7 changes: 4 additions & 3 deletions src/core/util/util.js
Original file line number Diff line number Diff line change
Expand Up @@ -261,8 +261,9 @@ angular.module('material.core')
}

/*
* Angular's $cacheFactory doesn't have a keys() method,
* so we add one ourself.
* Inject a 'keys()' method into Angular's $cacheFactory. Then
* head-hook all other methods
*
*/
function cacheFactory(id, options) {
var cache = $cacheFactory(id, options);
Expand All @@ -288,7 +289,7 @@ angular.module('material.core')

cache._destroy = cache.destroy;
cache.destroy = function() {
keys = null;
keys = {};
return cache._destroy();
};

Expand Down

0 comments on commit 98247bc

Please sign in to comment.