Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

ngClass[Odd/Even] overhaul #15228

Closed
wants to merge 15 commits into from
Closed

ngClass[Odd/Even] overhaul #15228

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
8822006
refactor(ngClass): remove unnecessary dependency on `$animate`
gkalpak Oct 7, 2016
0409589
refactor(ngClass): remove redundant `$observe`r
gkalpak Oct 7, 2016
2705085
test(ngClass): add some tests about one-time bindings and objects ins…
gkalpak Oct 7, 2016
0ba4be6
fix(ngClassOdd/Even): keep track of classes even if odd/even do not m…
gkalpak Oct 7, 2016
89268af
perf(ngClass): only access the element's `data` once
gkalpak Oct 7, 2016
2871a11
refactor(ngClass): simplify conditions
gkalpak Oct 7, 2016
a7e69f7
refactor(ngClass): move helper functions outside the closure
gkalpak Oct 7, 2016
37cec7a
refactor(ngClass): exit `arrayDifference()` early if an input is empty
gkalpak Oct 7, 2016
5708424
perf(ngClass): avoid deep-watching (if possible) and unnecessary copies
gkalpak Oct 7, 2016
739f723
fixup! refactor(ngClass): remove unnecessary dependency on `$animate`
gkalpak Oct 8, 2016
181c51b
fixup! refactor(ngClass): remove redundant `$observe`r
gkalpak Oct 8, 2016
16e666b
fixup! test(ngClass): add some tests about one-time bindings and obje…
gkalpak Oct 8, 2016
dfe4307
squash! fix(ngClassOdd/Even): keep track of classes even if odd/even …
gkalpak Oct 8, 2016
f766698
fixup! perf(ngClass): avoid deep-watching (if possible) and unnecessa…
gkalpak Oct 8, 2016
a9742f9
fixup! refactor(ngClass): simplify conditions
gkalpak Oct 10, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
215 changes: 132 additions & 83 deletions src/ng/directive/ngClass.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,129 +8,178 @@

function classDirective(name, selector) {
name = 'ngClass' + name;
return ['$animate', function($animate) {
var indexWatchExpression;

return ['$parse', function($parse) {
return {
restrict: 'AC',
link: function(scope, element, attr) {
var oldVal;
var expression = attr[name].trim();
var isOneTime = (expression.charAt(0) === ':') && (expression.charAt(1) === ':');

scope.$watch(attr[name], ngClassWatchAction, true);
var watchInterceptor = isOneTime ? toFlatValue : toClassString;
var watchExpression = $parse(expression, watchInterceptor);
var watchAction = isOneTime ? ngClassOneTimeWatchAction : ngClassWatchAction;

attr.$observe('class', function(value) {
ngClassWatchAction(scope.$eval(attr[name]));
});
var classCounts = element.data('$classCounts');
var oldModulo = true;
var oldClassString;

if (!classCounts) {
// Use createMap() to prevent class assumptions involving property
// names in Object.prototype
classCounts = createMap();
element.data('$classCounts', classCounts);
}

if (name !== 'ngClass') {
scope.$watch('$index', function($index, old$index) {
/* eslint-disable no-bitwise */
var mod = $index & 1;
if (mod !== (old$index & 1)) {
var classes = arrayClasses(scope.$eval(attr[name]));
if (mod === selector) {
addClasses(classes);
} else {
removeClasses(classes);
}
}
/* eslint-enable */
});
if (!indexWatchExpression) {
indexWatchExpression = $parse('$index', function moduloTwo($index) {
// eslint-disable-next-line no-bitwise
return $index & 1;
});
}

scope.$watch(indexWatchExpression, ngClassIndexWatchAction);
}

function addClasses(classes) {
var newClasses = digestClassCounts(classes, 1);
attr.$addClass(newClasses);
scope.$watch(watchExpression, watchAction, isOneTime);

function addClasses(classString) {
classString = digestClassCounts(split(classString), 1);
attr.$addClass(classString);
}

function removeClasses(classes) {
var newClasses = digestClassCounts(classes, -1);
attr.$removeClass(newClasses);
function removeClasses(classString) {
classString = digestClassCounts(split(classString), -1);
attr.$removeClass(classString);
}

function digestClassCounts(classes, count) {
// Use createMap() to prevent class assumptions involving property
// names in Object.prototype
var classCounts = element.data('$classCounts') || createMap();
function updateClasses(oldClassString, newClassString) {
var oldClassArray = split(oldClassString);
var newClassArray = split(newClassString);

var toRemoveArray = arrayDifference(oldClassArray, newClassArray);
var toAddArray = arrayDifference(newClassArray, oldClassArray);

var toRemoveString = digestClassCounts(toRemoveArray, -1);
var toAddString = digestClassCounts(toAddArray, 1);

attr.$addClass(toAddString);
attr.$removeClass(toRemoveString);
}

function digestClassCounts(classArray, count) {
var classesToUpdate = [];
forEach(classes, function(className) {

forEach(classArray, function(className) {
if (count > 0 || classCounts[className]) {
classCounts[className] = (classCounts[className] || 0) + count;
if (classCounts[className] === +(count > 0)) {
classesToUpdate.push(className);
}
}
});
element.data('$classCounts', classCounts);

return classesToUpdate.join(' ');
}

function updateClasses(oldClasses, newClasses) {
var toAdd = arrayDifference(newClasses, oldClasses);
var toRemove = arrayDifference(oldClasses, newClasses);
toAdd = digestClassCounts(toAdd, 1);
toRemove = digestClassCounts(toRemove, -1);
if (toAdd && toAdd.length) {
$animate.addClass(element, toAdd);
}
if (toRemove && toRemove.length) {
$animate.removeClass(element, toRemove);
function ngClassIndexWatchAction(newModulo) {
// This watch-action should run before the `ngClass[OneTime]WatchAction()`, thus it
// adds/removes `oldClassString`. If the `ngClass` expression has changed as well, the
// `ngClass[OneTime]WatchAction()` will update the classes.
if (newModulo === selector) {
addClasses(oldClassString);
} else {
removeClasses(oldClassString);
}

oldModulo = newModulo;
}

function ngClassWatchAction(newVal) {
// eslint-disable-next-line no-bitwise
if (selector === true || (scope.$index & 1) === selector) {
var newClasses = arrayClasses(newVal || []);
if (!oldVal) {
addClasses(newClasses);
} else if (!equals(newVal,oldVal)) {
var oldClasses = arrayClasses(oldVal);
updateClasses(oldClasses, newClasses);
}
function ngClassOneTimeWatchAction(newClassValue) {
var newClassString = toClassString(newClassValue);

if (newClassString !== oldClassString) {
ngClassWatchAction(newClassString);
}
if (isArray(newVal)) {
oldVal = newVal.map(function(v) { return shallowCopy(v); });
} else {
oldVal = shallowCopy(newVal);
}

function ngClassWatchAction(newClassString) {
if (oldModulo === selector) {
updateClasses(oldClassString, newClassString);
}

oldClassString = newClassString;
}
}
};
}];

function arrayDifference(tokens1, tokens2) {
var values = [];
// Helpers
function arrayDifference(tokens1, tokens2) {
Copy link

@w-sz w-sz Oct 9, 2016 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe this will be better ?

function arrayDifference(tokens1, tokens2) {
      var tokens1Length = tokens1.length, tokens2Length = tokens2.length;

      if (!tokens1 || !tokens1Length) return [];
      if (!tokens2 || !tokens2Length) return tokens1;

      var values = [];

      outer:
        for (var i = 0; i < tokens1Length; i++) {
          var token = tokens1[i];
          for (var j = 0; j < tokens2Length; j++) {
            if (token === tokens2[j]) continue outer;
          }
          values.push(token);
        }

      return values;
    }

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought about that too. The "problem" is that tokens1/tokens2 can be undefined. So, "caching" their length gets more complicated (to the point it shouldn't make any difference).

I also thought about just caching it for the for loop (e.g. for (var i = 0, ii = tokens1.length; i < ii; i++)), but I think the VM will detect that there is nothing mutating .length inside the loop and make that optimisation itself (this is just an assumption - maybe I am wrong).

if (!tokens1 || !tokens1.length) return [];
if (!tokens2 || !tokens2.length) return tokens1;

outer:
for (var i = 0; i < tokens1.length; i++) {
var token = tokens1[i];
for (var j = 0; j < tokens2.length; j++) {
if (token === tokens2[j]) continue outer;
}
values.push(token);
var values = [];

outer:
for (var i = 0; i < tokens1.length; i++) {
var token = tokens1[i];
for (var j = 0; j < tokens2.length; j++) {
if (token === tokens2[j]) continue outer;
}
return values;
values.push(token);
}

function arrayClasses(classVal) {
var classes = [];
if (isArray(classVal)) {
forEach(classVal, function(v) {
classes = classes.concat(arrayClasses(v));
});
return classes;
} else if (isString(classVal)) {
return classVal.split(' ');
} else if (isObject(classVal)) {
forEach(classVal, function(v, k) {
if (v) {
classes = classes.concat(k.split(' '));
}
});
return classes;
return values;
}

function split(classString) {
return classString && classString.split(' ');
}

function toClassString(classValue) {
var classString = classValue;

if (isArray(classValue)) {
classString = classValue.map(toClassString).join(' ');
} else if (isObject(classValue)) {
classString = Object.keys(classValue).
filter(function(key) { return classValue[key]; }).
join(' ');
}

return classString;
}

function toFlatValue(classValue) {
var flatValue = classValue;

if (isArray(classValue)) {
flatValue = classValue.map(toFlatValue);
} else if (isObject(classValue)) {
var hasUndefined = false;

flatValue = Object.keys(classValue).filter(function(key) {
var value = classValue[key];

if (!hasUndefined && isUndefined(value)) {
hasUndefined = true;
}

return value;
});

if (hasUndefined) {
// Prevent the `oneTimeLiteralWatchInterceptor` from unregistering
// the watcher, by including at least one `undefined` value.
flatValue.push(undefined);
}
return classVal;
}
}];

return flatValue;
}
}

/**
Expand Down
Loading