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

refactor(datepicker): use ng-if to increase performance #1915

Closed
wants to merge 1 commit into from
Closed

refactor(datepicker): use ng-if to increase performance #1915

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
3 changes: 3 additions & 0 deletions misc/test-lib/helpers.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@ beforeEach(function() {
var element = angular.element(this.actual);
return element.hasClass('ng-hide') ||
element.css('display') == 'none';
},
toBeGone: function() {
return this.actual.length === 0;
}
});
});
20 changes: 11 additions & 9 deletions src/datepicker/datepicker.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -451,19 +451,21 @@ function ($compile, $parse, $document, $position, dateFilter, datepickerPopupCon

scope.showButtonBar = angular.isDefined(attrs.showButtonBar) ? scope.$parent.$eval(attrs.showButtonBar) : datepickerPopupConfig.showButtonBar;

scope.popup = {};

scope.getText = function( key ) {
return scope[key + 'Text'] || datepickerPopupConfig[key + 'Text'];
};

attrs.$observe('datepickerPopup', function(value) {
dateFormat = value || datepickerPopupConfig.datepickerPopup;
ngModel.$render();
dateFormat = value || datepickerPopupConfig.datepickerPopup;
ngModel.$render();
});

// popup element used to display calendar
var popupEl = angular.element('<div datepicker-popup-wrap><div datepicker></div></div>');
popupEl.attr({
'ng-model': 'date',
'ng-model': 'popup.date',
'ng-change': 'dateSelection()'
});

Expand Down Expand Up @@ -518,9 +520,9 @@ function ($compile, $parse, $document, $position, dateFilter, datepickerPopupCon
// Inner change
scope.dateSelection = function(dt) {
if (angular.isDefined(dt)) {
scope.date = dt;
scope.popup.date = dt;
}
ngModel.$setViewValue(scope.date);
ngModel.$setViewValue(scope.popup.date);
ngModel.$render();

if ( closeOnDateSelection ) {
Expand All @@ -531,15 +533,15 @@ function ($compile, $parse, $document, $position, dateFilter, datepickerPopupCon

element.bind('input change keyup', function() {
scope.$apply(function() {
scope.date = ngModel.$modelValue;
scope.popup.date = ngModel.$modelValue;
});
});

// Outter change
ngModel.$render = function() {
var date = ngModel.$viewValue ? dateFilter(ngModel.$viewValue, dateFormat) : '';
element.val(date);
scope.date = parseDate( ngModel.$modelValue );
scope.popup.date = parseDate( ngModel.$modelValue );
};

var documentClickBind = function(event) {
Expand Down Expand Up @@ -603,7 +605,7 @@ function ($compile, $parse, $document, $position, dateFilter, datepickerPopupCon
}

scope.$on('$destroy', function() {
$popup.remove();
$popup.next().remove();
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure about this change, will need to test to see if this is safe.

element.unbind('keydown', keydown);
$document.unbind('click', documentClickBind);
});
Expand All @@ -624,4 +626,4 @@ function ($compile, $parse, $document, $position, dateFilter, datepickerPopupCon
});
}
};
});
});
95 changes: 58 additions & 37 deletions src/datepicker/test/datepicker.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1102,7 +1102,7 @@ describe('datepicker directive', function () {
}));

it('does not to display datepicker initially', function() {
expect(dropdownEl).toBeHidden();
expect(dropdownEl).toBeGone();
});

it('to display the correct value in input', function() {
Expand All @@ -1111,18 +1111,20 @@ describe('datepicker directive', function () {
});

describe('initially opened', function () {
var wrapElement;

beforeEach(inject(function(_$document_, _$sniffer_) {
$document = _$document_;
$sniffer = _$sniffer_;
$rootScope.isopen = true;
$rootScope.date = new Date('September 30, 2010 15:30:00');
var wrapElement = $compile('<div><input ng-model="date" datepicker-popup is-open="isopen"><div>')($rootScope);
wrapElement = $compile('<div><input ng-model="date" datepicker-popup is-open="isopen"><div>')($rootScope);
$rootScope.$digest();
assignElements(wrapElement);
}));

it('datepicker is displayed', function() {
expect(dropdownEl).not.toBeHidden();
expect(dropdownEl).not.toBeGone();
});

it('renders the calendar correctly', function() {
Expand Down Expand Up @@ -1156,10 +1158,12 @@ describe('datepicker directive', function () {
});

it('closes the dropdown when a day is clicked', function() {
expect(dropdownEl.css('display')).not.toBe('none');
expect(dropdownEl).not.toBeGone();

clickOption(17);
expect(dropdownEl.css('display')).toBe('none');

assignElements(wrapElement);
expect(dropdownEl).toBeGone();
});

it('updates the model & calendar when input value changes', function() {
Expand All @@ -1181,10 +1185,12 @@ describe('datepicker directive', function () {
});

it('closes when click outside of calendar', function() {
expect(dropdownEl).not.toBeHidden();
expect(dropdownEl).not.toBeGone();

$document.find('body').click();
expect(dropdownEl.css('display')).toBe('none');

assignElements(wrapElement);
expect(dropdownEl).toBeGone();
});

it('sets `ng-invalid` for invalid input', function() {
Expand Down Expand Up @@ -1219,25 +1225,27 @@ describe('datepicker directive', function () {
});

it('returns to the input when ESC key is pressed in the popup and closes', function() {
expect(dropdownEl).not.toBeHidden();
expect(dropdownEl).not.toBeGone();

dropdownEl.find('button').eq(0).focus();
expect(document.activeElement.tagName).toBe('BUTTON');

triggerKeyDown(dropdownEl, 'esc');
expect(dropdownEl).toBeHidden();
assignElements(wrapElement);
expect(dropdownEl).toBeGone();
expect(document.activeElement.tagName).toBe('INPUT');
});

it('returns to the input when ESC key is pressed in the input and closes', function() {
expect(dropdownEl).not.toBeHidden();
expect(dropdownEl).not.toBeGone();

dropdownEl.find('button').eq(0).focus();
expect(document.activeElement.tagName).toBe('BUTTON');

triggerKeyDown(inputEl, 'esc');
$rootScope.$digest();
expect(dropdownEl).toBeHidden();
assignElements(wrapElement);
expect(dropdownEl).toBeGone();
expect(document.activeElement.tagName).toBe('INPUT');
});
});
Expand Down Expand Up @@ -1265,32 +1273,37 @@ describe('datepicker directive', function () {
});

describe('toggles programatically by `open` attribute', function () {
var wrapElement;

beforeEach(inject(function() {
$rootScope.open = true;
var wrapElement = $compile('<div><input ng-model="date" datepicker-popup is-open="open"><div>')($rootScope);
wrapElement = $compile('<div><input ng-model="date" datepicker-popup is-open="open"><div>')($rootScope);
$rootScope.$digest();
assignElements(wrapElement);
}));

it('to display initially', function() {
expect(dropdownEl.css('display')).not.toBe('none');
expect(dropdownEl).not.toBeGone();
});

it('to close / open from scope variable', function() {
expect(dropdownEl.css('display')).not.toBe('none');
expect(dropdownEl).not.toBeGone();
$rootScope.open = false;
$rootScope.$digest();
expect(dropdownEl.css('display')).toBe('none');
assignElements(wrapElement);
expect(dropdownEl).toBeGone();

$rootScope.open = true;
$rootScope.$digest();
expect(dropdownEl.css('display')).not.toBe('none');
assignElements(wrapElement);
expect(dropdownEl).not.toBeGone();
});
});

describe('custom format', function () {
beforeEach(inject(function() {
var wrapElement = $compile('<div><input ng-model="date" datepicker-popup="dd-MMMM-yyyy"><div>')($rootScope);
$rootScope.isopen = true;
var wrapElement = $compile('<div><input ng-model="date" is-open="isopen" datepicker-popup="dd-MMMM-yyyy"><div>')($rootScope);
$rootScope.$digest();
assignElements(wrapElement);
}));
Expand All @@ -1315,7 +1328,8 @@ describe('datepicker directive', function () {
describe('dynamic custom format', function () {
beforeEach(inject(function() {
$rootScope.format = 'dd-MMMM-yyyy';
var wrapElement = $compile('<div><input ng-model="date" datepicker-popup="{{format}}"><div>')($rootScope);
$rootScope.isopen = true;
var wrapElement = $compile('<div><input ng-model="date" is-open="isopen" datepicker-popup="{{format}}"><div>')($rootScope);
$rootScope.$digest();
assignElements(wrapElement);
}));
Expand Down Expand Up @@ -1366,16 +1380,18 @@ describe('datepicker directive', function () {
}

describe('', function () {
var wrapElement;

beforeEach(inject(function() {
$rootScope.isopen = true;
var wrapElement = $compile('<div><input ng-model="date" datepicker-popup is-open="isopen"><div>')($rootScope);
wrapElement = $compile('<div><input ng-model="date" datepicker-popup is-open="isopen"><div>')($rootScope);
$rootScope.$digest();
assignElements(wrapElement);
assignButtonBar();
}));

it('should exist', function() {
expect(dropdownEl).not.toBeHidden();
expect(dropdownEl).not.toBeGone();
expect(dropdownEl.find('li').length).toBe(2);
});

Expand Down Expand Up @@ -1421,15 +1437,17 @@ describe('datepicker directive', function () {

it('should have a button to close calendar', function() {
buttons.eq(2).click();
expect(dropdownEl).toBeHidden();
assignElements(wrapElement);
expect(dropdownEl).toBeGone();
});
});

describe('customization', function() {
it('should change text from attributes', function() {
$rootScope.clearText = 'Null it!';
$rootScope.close = 'Close';
var wrapElement = $compile('<div><input ng-model="date" datepicker-popup current-text="Now" clear-text="{{clearText}}" close-text="{{close}}ME"><div>')($rootScope);
$rootScope.isopen = true;
var wrapElement = $compile('<div><input ng-model="date" is-open="isopen" datepicker-popup current-text="Now" clear-text="{{clearText}}" close-text="{{close}}ME"><div>')($rootScope);
$rootScope.$digest();
assignElements(wrapElement);
assignButtonBar();
Expand All @@ -1441,7 +1459,8 @@ describe('datepicker directive', function () {

it('should remove bar', function() {
$rootScope.showBar = false;
var wrapElement = $compile('<div><input ng-model="date" datepicker-popup show-button-bar="showBar"><div>')($rootScope);
$rootScope.isopen = true;
var wrapElement = $compile('<div><input ng-model="date" is-open="isopen" datepicker-popup show-button-bar="showBar"><div>')($rootScope);
$rootScope.$digest();
assignElements(wrapElement);
expect(dropdownEl.find('li').length).toBe(1);
Expand All @@ -1451,7 +1470,8 @@ describe('datepicker directive', function () {
describe('`ng-change`', function() {
beforeEach(inject(function() {
$rootScope.changeHandler = jasmine.createSpy('changeHandler');
var wrapElement = $compile('<div><input ng-model="date" datepicker-popup ng-change="changeHandler()"><div>')($rootScope);
$rootScope.isopen = true;
var wrapElement = $compile('<div><input ng-model="date" is-open="isopen" datepicker-popup ng-change="changeHandler()"><div>')($rootScope);
$rootScope.$digest();
assignElements(wrapElement);
assignButtonBar();
Expand Down Expand Up @@ -1496,7 +1516,8 @@ describe('datepicker directive', function () {
beforeEach(inject(function() {
$rootScope.changeHandler = jasmine.createSpy('changeHandler');
$rootScope.date = new Date();
var wrapElement = $compile('<div><input ng-model="date" datepicker-popup ng-required="true" ng-change="changeHandler()"><div>')($rootScope);
$rootScope.isopen = true;
var wrapElement = $compile('<div><input ng-model="date" is-open="isopen" datepicker-popup ng-required="true" ng-change="changeHandler()"><div>')($rootScope);
$rootScope.$digest();
assignElements(wrapElement);
}));
Expand Down Expand Up @@ -1524,10 +1545,10 @@ describe('datepicker directive', function () {

it('should append to the body', function() {
var $body = $document.find('body'),
bodyLength = $body.children().length,
elm = angular.element(
'<div><input datepicker-popup ng-model="date" datepicker-append-to-body="true"></input></div>'
);
bodyLength = $body.children().length,
elm = angular.element(
'<div><input datepicker-popup is-open="true" ng-model="date" datepicker-append-to-body="true" /></div>'
);
$compile(elm)($rootScope);
$rootScope.$digest();

Expand All @@ -1536,11 +1557,11 @@ describe('datepicker directive', function () {
});
it('should be removed on scope destroy', function() {
var $body = $document.find('body'),
bodyLength = $body.children().length,
isolatedScope = $rootScope.$new(),
elm = angular.element(
'<input datepicker-popup ng-model="date" datepicker-append-to-body="true"></input>'
);
bodyLength = $body.children().length,
isolatedScope = $rootScope.$new(),
elm = angular.element(
'<input datepicker-popup is-open="true" ng-model="date" datepicker-append-to-body="true" />'
);
$compile(elm)(isolatedScope);
isolatedScope.$digest();
expect($body.children().length).toEqual(bodyLength + 1);
Expand All @@ -1554,8 +1575,8 @@ describe('datepicker directive', function () {
beforeEach(inject(function(datepickerConfig) {
angular.extend(originalConfig, datepickerConfig);
datepickerConfig.showWeeks = false;

var wrapElement = $compile('<div><input ng-model="date" datepicker-popup><div>')($rootScope);
$rootScope.isopen = true;
var wrapElement = $compile('<div><input ng-model="date" is-open="isopen" datepicker-popup><div>')($rootScope);
$rootScope.$digest();
assignElements(wrapElement);
}));
Expand Down Expand Up @@ -1667,4 +1688,4 @@ describe('datepicker directive', function () {
expect(getTitle()).toBe('2013');
});
});
});
});
2 changes: 1 addition & 1 deletion template/datepicker/popup.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<ul class="dropdown-menu" ng-style="{display: (isOpen && 'block') || 'none', top: position.top+'px', left: position.left+'px'}" ng-keydown="keydown($event)">
<ul class="dropdown-menu" ng-if="isOpen" ng-style="{display: (isOpen && 'block') || 'none', top: position.top+'px', left: position.left+'px'}" ng-keydown="keydown($event)">
<li ng-transclude></li>
<li ng-if="showButtonBar" style="padding:10px 9px 2px">
<span class="btn-group">
Expand Down