Skip to content

Commit

Permalink
fix(gridUtil): rtlScrollType using wrong method
Browse files Browse the repository at this point in the history 
rtlScrollType was not using angular.element().remove() to remove a
temporary element from the DOM. This was working in Chrome and perhaps
other browsers but failing in IE, and would only show up on RTL grids.

Fixes #3637
  • Loading branch information
@c0bra
c0bra committed Jun 2, 2015
1 parent 58bd0eb commit 15ee480
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 21 deletions.
32 changes: 16 additions & 16 deletions src/js/core/services/ui-grid-util.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ function augmentWidthOrHeight( elem, name, extra, isBorderBox, styles ) {
val = 0;

var sides = ['Top', 'Right', 'Bottom', 'Left'];

for ( ; i < 4; i += 2 ) {
var side = sides[i];
// dump('side', side);
Expand Down Expand Up @@ -166,7 +166,7 @@ var uidPrefix = 'uiGrid-';
/**
* @ngdoc service
* @name ui.grid.service:GridUtil
*
*
* @description Grid utility functions
*/
module.service('gridUtil', ['$log', '$window', '$document', '$http', '$templateCache', '$timeout', '$interval', '$injector', '$q', '$interpolate', 'uiGridConstants',
Expand Down Expand Up @@ -292,7 +292,7 @@ module.service('gridUtil', ['$log', '$window', '$document', '$http', '$templateC
if (angular.isUndefined(excludeProperties)) { excludeProperties = []; }

var item = data[0];

angular.forEach(item,function (prop, propName) {
if ( excludeProperties.indexOf(propName) === -1){
columnDefs.push({
Expand Down Expand Up @@ -383,7 +383,7 @@ module.service('gridUtil', ['$log', '$window', '$document', '$http', '$templateC
.then(s.postProcessTemplate);
},

//
//
postProcessTemplate: function (template) {
var startSym = $interpolate.startSymbol(),
endSym = $interpolate.endSymbol();
Expand Down Expand Up @@ -440,7 +440,7 @@ module.service('gridUtil', ['$log', '$window', '$document', '$http', '$templateC
* @returns {number} Element width in pixels, accounting for any borders, etc.
*/
elementWidth: function (elem) {

},

/**
Expand All @@ -454,7 +454,7 @@ module.service('gridUtil', ['$log', '$window', '$document', '$http', '$templateC
* @returns {number} Element height in pixels, accounting for any borders, etc.
*/
elementHeight: function (elem) {

},

// Thanks to http://stackoverflow.com/a/13382873/888165
Expand All @@ -473,7 +473,7 @@ module.service('gridUtil', ['$log', '$window', '$document', '$http', '$templateC
// add innerdiv
var inner = document.createElement("div");
inner.style.width = "100%";
outer.appendChild(inner);
outer.appendChild(inner);

var widthWithScroll = inner.offsetWidth;

Expand Down Expand Up @@ -541,7 +541,7 @@ module.service('gridUtil', ['$log', '$window', '$document', '$http', '$templateC
// var toFix = ['wheel', 'mousewheel', 'DOMMouseScroll', 'MozMousePixelScroll'];
// var toBind = 'onwheel' in document || document.documentMode >= 9 ? ['wheel'] : ['mousewheel', 'DomMouseScroll', 'MozMousePixelScroll'];
var lowestDelta, lowestDeltaXY;

var orgEvent = event || window.event,
args = [].slice.call(arguments, 1),
delta = 0,
Expand Down Expand Up @@ -727,7 +727,7 @@ module.service('gridUtil', ['$log', '$window', '$document', '$http', '$templateC
resetUids: function () {
uid = ['0', '0', '0'];
},

/**
* @ngdoc method
* @methodOf ui.grid.service:GridUtil
Expand All @@ -736,7 +736,7 @@ module.service('gridUtil', ['$log', '$window', '$document', '$http', '$templateC
* treatment within ui-grid if we so desired. At present we only log
* error messages if uiGridConstants.LOG_ERROR_MESSAGES is set to true
* @param {string} logMessage message to be logged to the console
*
*
*/
logError: function( logMessage ){
if ( uiGridConstants.LOG_ERROR_MESSAGES ){
Expand All @@ -752,7 +752,7 @@ module.service('gridUtil', ['$log', '$window', '$document', '$http', '$templateC
* treatment within ui-grid if we so desired. At present we only log
* warning messages if uiGridConstants.LOG_WARN_MESSAGES is set to true
* @param {string} logMessage message to be logged to the console
*
*
*/
logWarn: function( logMessage ){
if ( uiGridConstants.LOG_WARN_MESSAGES ){
Expand All @@ -767,7 +767,7 @@ module.service('gridUtil', ['$log', '$window', '$document', '$http', '$templateC
* @description wraps the $log method, allowing us to choose different
* treatment within ui-grid if we so desired. At present we only log
* debug messages if uiGridConstants.LOG_DEBUG_MESSAGES is set to true
*
*
*/
logDebug: function() {
if ( uiGridConstants.LOG_DEBUG_MESSAGES ){
Expand Down Expand Up @@ -903,7 +903,7 @@ module.service('gridUtil', ['$log', '$window', '$document', '$http', '$templateC
}
}

definer.remove();
angular.element(definer).remove();
rtlScrollType.type = type;

return type;
Expand Down Expand Up @@ -1080,11 +1080,11 @@ module.service('gridUtil', ['$log', '$window', '$document', '$http', '$templateC
* trailing (bool) - whether to trigger after throttle time ends if called multiple times
* Updated to use $interval rather than $timeout, as protractor (e2e tests) is able to work with $interval,
* but not with $timeout
*
*
* Note that when using throttle, you need to use throttle to create a new function upfront, then use the function
* return from that call each time you need to call throttle. If you call throttle itself repeatedly, the lastCall
* variable will get overwritten and the throttling won't work
*
*
* @example
* <pre>
* var throttledFunc = gridUtil.throttle(function(){console.log('throttled');}, 500, {trailing: true});
Expand Down Expand Up @@ -1258,7 +1258,7 @@ module.service('gridUtil', ['$log', '$window', '$document', '$http', '$templateC
// offsetX = event.clientX - boundingRect.left;
// offsetY = event.clientY - boundingRect.top;
// }

// event.deltaX = deltaX;
// event.deltaY = deltaY;
// event.deltaFactor = lowestDelta;
Expand Down
19 changes: 14 additions & 5 deletions test/unit/core/services/ui-grid-util.spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ describe('ui.grid.utilService', function() {
translationExpects.forEach( function (set) {
var strIn = set[0];
var strOut = set[1];

expect(gridUtil.readableColumnName(strIn)).toEqual(strOut);
});
});
Expand Down Expand Up @@ -139,7 +139,7 @@ describe('ui.grid.utilService', function() {
]);
});
});

describe('getColumnsFromData', function() {
it('should create column defs from a data array omitting $$hashKey', function() {
var data = [
Expand All @@ -149,7 +149,7 @@ describe('ui.grid.utilService', function() {
$$hashKey: '00A'
}
];

var excludeProperties = ['$$hashKey'];

var columns = gridUtil.getColumnsFromData(data, excludeProperties);
Expand Down Expand Up @@ -368,7 +368,7 @@ describe('ui.grid.utilService', function() {
describe('resetUids()', function () {
it('should reset the UID index back to 000', function () {
gridUtil.resetUids();

for (var i = 0; i < 50; i++) {
gridUtil.nextUid();
}
Expand Down Expand Up @@ -513,4 +513,13 @@ describe('ui.grid.utilService', function() {
}));
});
});
});

describe('rtlScrollType', function () {
it('should not throw an exception', function () {
// This was throwing an exception in IE because IE doesn't have a native <element>.remove() method.
expect(function () {
gridUtil.rtlScrollType();
}).not.toThrow();
});
});
});

0 comments on commit 15ee480

Please sign in to comment.