-
Notifications
You must be signed in to change notification settings - Fork 98
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Performance is really bad at ng-repeat. #9
Comments
Hi martingg88 It could be a bit later... But you can try my improvement if you still need it. function buildEllipsis() {
if (typeof(scope.ngBind) !== 'undefined') {
var bindArray = scope.ngBind.split(" "),
ellipsisSymbol = (typeof(attributes.ellipsisSymbol) !== 'undefined') ? attributes.ellipsisSymbol : '…',
appendString = (typeof(scope.ellipsisAppend) !== 'undefined' && scope.ellipsisAppend !== '') ? ellipsisSymbol + '<span>' + scope.ellipsisAppend + '</span>' : ellipsisSymbol;
attributes.isTruncated = false;
element.html(scope.ngBind);
// refine the algorithm to improve the performance significantly
// [email protected] 20150228
var desiredHeight = element[0].clientHeight;
var actualHeight = element[0].scrollHeight;
if (actualHeight > desiredHeight) {
// PERFORMANCE IMPROVEMENT: calc the proper size by heights
var size = Math.floor(bindArray.length *
desiredHeight / actualHeight);
var text = bindArray.slice(0, size).join(' ');
element.html(text + appendString);
while (isOverflowed(element) && size > 0) {
--size;
// PERFORMANCE IMPROVEMENT: use String.substr rather than Array.join
text = text.substr(0, text.length - bindArray[size].length - 1);
element.html(text + appendString);
}
}
}
} |
Tested in production environment. Huge improvement. Thank you! |
@miraclemarc, please try this updated version. It places the ellipsis more accurate.
|
Thank you! I will test your latest additions Update: |
is this improvement going to get committed ? |
@miraclemarc yep, the first version is better in performance, but it could place the ellipsis incorrectly.
or even
|
Just curious instead of substracting words from the end, which could seriously hurt performance when the text is long, how about we use an idea similar to "binary search"? The key here is to find the "boundary position" where the text just overflows. |
Here is a faster version based on the first version by @HuangJian , it uses binary search to find the "boundary position", along with other performance enhancement such as using var domElement = element[0];
function getEmptySpaceLocations(text) {
var spaceIndices = [];
var re = /\s+/gm;
var match;
while ((match = re.exec(text)) != null) {
spaceIndices.push(match.index);
}
return spaceIndices;
}
function getSubText(fullText, spaceIndices, index) {
return fullText.substr(0, spaceIndices[index]);
}
function isSubTextOverflow(fullText, spaceIndices, index, appendString) {
var text = getSubText(fullText, spaceIndices, index);
domElement.innerHTML = text + appendString;
return isOverflown(element);
}
function buildEllipsis() {
if (typeof (scope.ngBind) !== 'undefined') {
var text = scope.ngBind;
var spaceIndices = getEmptySpaceLocations(text);
attributes.isTruncated = false;
element.html(text);
var desiredHeight = element[0].clientHeight;
var actualHeight = element[0].scrollHeight;
if (actualHeight > desiredHeight) {
var totalSpaceLocations = spaceIndices.length;
var begin = 0;
var end = totalSpaceLocations - 1;
var lastOverflown = true;
var currentIndex;
var currentOverflown = true;
var notFound = true;
var seekedTimes = 0;
var ellipsisSymbol = (typeof (attributes.ellipsisSymbol) !== 'undefined') ? attributes.ellipsisSymbol : '…';
var appendString = (typeof (scope.ellipsisAppend) !== 'undefined' && scope.ellipsisAppend !== '') ? ellipsisSymbol + '<span>' + scope.ellipsisAppend + '</span>' : ellipsisSymbol;
while (notFound) {
currentIndex = begin + ((end - begin) >> 1);
currentOverflown = isSubTextOverflow(text, spaceIndices, currentIndex, appendString);
seekedTimes++;
if ((currentOverflown != lastOverflown) && (end - begin) == 1) {
notFound = false;
}
else {
if (currentOverflown) {
end = currentIndex;
}
else {
begin = currentIndex;
}
}
}
var truncatedText = getSubText(text, spaceIndices, currentIndex) + appendString;
element.html(truncatedText);
attributes.isTruncated = true;
console.log('Seeked: ' + seekedTimes + ' Spaces: ' + totalSpaceLocations + ' Length: ' + text.length);
}
}
}
function isOverflown(thisElement) {
return thisElement[0].scrollHeight > thisElement[0].clientHeight;
} |
@ MauMaGau Bump, could you please integrate this fix like they did in the educarlabs fork (which is less maintained). This fix is a huge performance upgrade when the text that has to be ellipsed is very long. |
Ok so I have put my money where my mouth is a few weeks after I asked for this here and integrated it myself. However I still haven't opened a PR, something that slipped... because life. Do know I have a present for you all 🎁 and please do nag me about it if I still haven't done this a while after this comment ^_^ |
nag 💅 |
nag 💅 |
There you go! #79 |
There is the worst performance if I implement it in ng-repeat..
Any help would be more appreciated.
The text was updated successfully, but these errors were encountered: