From 479175bf5d5d0540c1455fc63fccad94c55d51fa Mon Sep 17 00:00:00 2001 From: Andy Joslin Date: Tue, 4 Feb 2014 08:46:02 -0500 Subject: [PATCH] refactor(view): use nextUid from angular to generate unique ids Math.random() is unreliable, produces duplicates, and numbers can overflow if you use them for long enough. --- .../angular/src/directive/ionicViewState.js | 1 - js/ext/angular/src/service/ionicView.js | 38 +++++++++---------- js/utils/utils.js | 33 ++++++++++++++++ 3 files changed, 52 insertions(+), 20 deletions(-) diff --git a/js/ext/angular/src/directive/ionicViewState.js b/js/ext/angular/src/directive/ionicViewState.js index f2ea4d92d8..8666126e05 100644 --- a/js/ext/angular/src/directive/ionicViewState.js +++ b/js/ext/angular/src/directive/ionicViewState.js @@ -353,5 +353,4 @@ angular.module('ionic.ui.viewState', ['ionic.service.view', 'ionic.service.gestu return directive; }]); - })(); diff --git a/js/ext/angular/src/service/ionicView.js b/js/ext/angular/src/service/ionicView.js index 95fc551aa8..f6f9b5b2bd 100644 --- a/js/ext/angular/src/service/ionicView.js +++ b/js/ext/angular/src/service/ionicView.js @@ -1,7 +1,7 @@ angular.module('ionic.service.view', ['ui.router', 'ionic.service.platform']) -.run( ['$rootScope', '$state', '$location', '$document', '$animate', '$ionicPlatform', +.run( ['$rootScope', '$state', '$location', '$document', '$animate', '$ionicPlatform', function( $rootScope, $state, $location, $document, $animate, $ionicPlatform) { // init the variables that keep track of the view history @@ -29,7 +29,7 @@ angular.module('ionic.service.view', ['ui.router', 'ionic.service.platform']) if(!data.url && data.uiSref) { data.url = $state.href(data.uiSref); } - + if(data.url) { // don't let it start with a #, messes with $location.url() if(data.url.indexOf('#') === 0) { @@ -66,7 +66,7 @@ angular.module('ionic.service.view', ['ui.router', 'ionic.service.platform']) }]) -.factory('$ionicViewService', ['$rootScope', '$state', '$location', '$window', '$injector', +.factory('$ionicViewService', ['$rootScope', '$state', '$location', '$window', '$injector', function( $rootScope, $state, $location, $window, $injector) { var $animate = $injector.has('$animate') ? $injector.get('$animate') : false; @@ -106,7 +106,7 @@ angular.module('ionic.service.view', ['ui.router', 'ionic.service.platform']) }; function createViewId(stateId) { - return ('_' + stateId + '_' + Math.round(Math.random() * 99999999)).replace(/\./g, '_'); + return ionic.Utils.nextUid(); } return { @@ -135,7 +135,7 @@ angular.module('ionic.service.view', ['ui.router', 'ionic.service.platform']) return rsp; } - if(currentView && + if(currentView && currentView.stateId === currentStateId && currentView.historyId === hist.historyId) { // do nothing if its the same stateId in the same history @@ -167,7 +167,7 @@ angular.module('ionic.service.view', ['ui.router', 'ionic.service.platform']) rsp.historyId = forwardView.historyId; } - } else if(currentView && currentView.historyId !== hist.historyId && + } else if(currentView && currentView.historyId !== hist.historyId && hist.cursor > -1 && hist.stack.length > 0 && hist.cursor < hist.stack.length && hist.stack[hist.cursor].stateId === currentStateId) { // they just changed to a different history and the history already has views in it @@ -210,7 +210,7 @@ angular.module('ionic.service.view', ['ui.router', 'ionic.service.platform']) } // add the new view to the stack - viewHistory.histories[rsp.viewId] = this.createView({ + viewHistory.histories[rsp.viewId] = this.createView({ viewId: rsp.viewId, index: hist.stack.length, historyId: hist.historyId, @@ -219,7 +219,7 @@ angular.module('ionic.service.view', ['ui.router', 'ionic.service.platform']) stateId: currentStateId, stateName: this.getCurrentStateName(), stateParams: this.getCurrentStateParams(), - url: $location.url() + url: $location.url(), }); // add the new view to this history's stack @@ -246,7 +246,7 @@ angular.module('ionic.service.view', ['ui.router', 'ionic.service.platform']) }, registerHistory: function(scope) { - scope.$historyId = 'h' + Math.round(Math.random() * 99999999999); + scope.$historyId = ionic.Utils.nextUid(); }, createView: function(data) { @@ -275,9 +275,9 @@ angular.module('ionic.service.view', ['ui.router', 'ionic.service.platform']) }, isCurrentStateNavView: function(navView) { - return ($state && - $state.current && - $state.current.views && + return ($state && + $state.current && + $state.current.views && $state.current.views[navView] ? true : false); }, @@ -308,7 +308,7 @@ angular.module('ionic.service.view', ['ui.router', 'ionic.service.platform']) return id; } // if something goes wrong make sure its got a unique stateId - return 'r' + Math.round(Math.random() * 9999999); + return ionic.Utils.nextUid(); }, _getView: function(viewId) { @@ -329,8 +329,8 @@ angular.module('ionic.service.view', ['ui.router', 'ionic.service.platform']) if( !$rootScope.$viewHistory.histories[ histObj.historyId ] ) { // this history object exists in parent scope, but doesn't // exist in the history data yet - $rootScope.$viewHistory.histories[ histObj.historyId ] = { - historyId: histObj.historyId, + $rootScope.$viewHistory.histories[ histObj.historyId ] = { + historyId: histObj.historyId, parentHistoryId: this._getParentHistoryObj(histObj.scope.$parent).historyId, stack: [], cursor: -1 @@ -385,7 +385,7 @@ angular.module('ionic.service.view', ['ui.router', 'ionic.service.platform']) return function(shouldAnimate) { return { - + enter: function(element) { if(doAnimation && shouldAnimate) { @@ -397,7 +397,7 @@ angular.module('ionic.service.view', ['ui.router', 'ionic.service.platform']) $animate.enter(element, navViewElement, null, function() { document.body.classList.remove('disable-pointer-events'); - }); + }); return; } @@ -412,8 +412,8 @@ angular.module('ionic.service.view', ['ui.router', 'ionic.service.platform']) // leave with an animation setAnimationClass(); - $animate.leave(element, function() { - element.remove(); + $animate.leave(element, function() { + element.remove(); }); return; } diff --git a/js/utils/utils.js b/js/utils/utils.js index 3247236531..18867dcb79 100644 --- a/js/utils/utils.js +++ b/js/utils/utils.js @@ -1,4 +1,7 @@ (function(ionic) { + + /* for nextUid() function below */ + var uid = ['0','0','0']; /** * Various utilities used throughout Ionic @@ -139,6 +142,36 @@ } } return obj; + }, + + /** + * A consistent way of creating unique IDs in angular. The ID is a sequence of alpha numeric + * characters such as '012ABC'. The reason why we are not using simply a number counter is that + * the number string gets longer over time, and it can also overflow, where as the nextId + * will grow much slower, it is a string, and it will never overflow. + * + * @returns an unique alpha-numeric string + */ + nextUid: function() { + var index = uid.length; + var digit; + + while(index) { + index--; + digit = uid[index].charCodeAt(0); + if (digit == 57 /*'9'*/) { + uid[index] = 'A'; + return uid.join(''); + } + if (digit == 90 /*'Z'*/) { + uid[index] = '0'; + } else { + uid[index] = String.fromCharCode(digit + 1); + return uid.join(''); + } + } + uid.unshift('0'); + return uid.join(''); } };