fix(browser): on first hash-set, dont set scrollTop

ionic-team · Feb 11, 2014 · 1c4d4a8 · 1c4d4a8
1 parent 59944db
commit 1c4d4a8
Show file tree

Hide file tree

Showing 6 changed files with 71 additions and 12 deletions.
diff --git a/js/ext/angular/src/ionicAngular.js b/js/ext/angular/src/ionicAngular.js
@@ -10,7 +10,8 @@ angular.module('ionic.service', [
   'ionic.service.modal',
   'ionic.service.popup',
   'ionic.service.templateLoad',
-  'ionic.service.view'
+  'ionic.service.view',
+  'ionic.decorator.location'
 ]);
 
 // UI specific services and delegates

diff --git a/js/ext/angular/src/service/decorators/location.js b/js/ext/angular/src/service/decorators/location.js
@@ -0,0 +1,27 @@
+angular.module('ionic.decorator.location', [])
+
+.config(['$provide', function($provide) {
+  $provide.decorator('$location', ['$delegate', '$timeout', $LocationDecorator]);
+}]);
+
+function $LocationDecorator($location, $timeout) {
+
+  var firstHashSet = false;
+  $location.__hash = $location.hash;
+  //Fix: first time window.location.hash is set, the scrollable area
+  //found nearest to body's scrollTop is set to scroll to an element
+  //with that ID.
+  $location.hash = function(value) {
+    if (!firstHashSet && angular.isDefined(value)) {
+      $timeout(function() {
+        var scroll = document.querySelector('.scroll-content');
+        if (scroll)
+          scroll.scrollTop = 0;
+      }, 0, false);
+      firstHashSet = true;
+    }
+    return $location.__hash(value);
+  };
+
+  return $location;
+}
diff --git a/js/ext/angular/src/service/delegates/ionicScrollDelegate.js b/js/ext/angular/src/service/delegates/ionicScrollDelegate.js
@@ -81,14 +81,16 @@ angular.module('ionic.ui.service.scrollDelegate', [])
       });
 
       $scope.$parent.$on('scroll.anchorScroll', function(e, animate) {
-        var hash = $location.hash();
-        var elm;
-        if (hash && (elm = document.getElementById(hash)) ) {
-          var scroll = ionic.DomUtil.getPositionInParent(elm, scrollEl);
-          scrollView.scrollTo(scroll.left, scroll.top, !!animate);
-        } else {
-          scrollView.scrollTo(0,0, !!animate);
-        }
+        scrollViewResize().then(function() {
+          var hash = $location.hash();
+          var elm;
+          if (hash && (elm = document.getElementById(hash)) ) {
+            var scroll = ionic.DomUtil.getPositionInParent(elm, scrollEl);
+            scrollView.scrollTo(scroll.left, scroll.top, !!animate);
+          } else {
+            scrollView.scrollTo(0,0, !!animate);
+          }
+        });
       });
 
       /**

diff --git a/js/ext/angular/test/anchorScroll.html b/js/ext/angular/test/anchorScroll.html
@@ -58,8 +58,7 @@
 function MyCtrl($scope, $location, $ionicScrollDelegate) {
   $scope.scrollTo = function(id) {
     $location.hash(id);
-    console.log($location.hash());
-    $ionicScrollDelegate.anchorScroll();
+    $ionicScrollDelegate.anchorScroll(true);
   }
 }
     </script>

diff --git a/js/ext/angular/test/service/decorators/location.unit.js b/js/ext/angular/test/service/decorators/location.unit.js
@@ -0,0 +1,25 @@
+describe('$location decorator', function() {
+
+  beforeEach(module('ionic.decorator.location'));
+
+  describe('.hash()', function() {
+
+    it('should find .scroll-content and set scrollTop=0', inject(function($location, $timeout, $rootScope) {
+      var scroll = { scrollTop: 5 };
+      spyOn(document, 'querySelector').andCallFake(function() {
+        return scroll;
+      });
+
+      $location.hash('123');
+      $timeout.flush();
+      expect(scroll.scrollTop).toBe(0);
+
+      //Second time? shouldnt try to set things
+      scroll.scrollTop = 4;
+      $location.hash('456');
+      $timeout.verifyNoPendingTasks();
+      expect(scroll.scrollTop).toBe(4);
+    }));
+
+  });
+});
diff --git a/js/ext/angular/test/service/delegates/ionicScrollDelegate.unit.js b/js/ext/angular/test/service/delegates/ionicScrollDelegate.unit.js
@@ -97,13 +97,14 @@ describe('anchorScroll', function() {
 
   function testWithAnimate(animate) {
     describe('with animate=' + animate, function() {
-      var contentEl, scope, del;
+      var contentEl, scope, del, timeout;
       beforeEach(inject(function($rootScope, $compile, $timeout, $document, $ionicScrollDelegate) {
         scope = $rootScope.$new();
         contentEl = $compile('<content></content>')(scope);
 
         document.body.appendChild(contentEl[0]);
         del = $ionicScrollDelegate;
+        timeout = $timeout;
       }));
 
       it('should anchorScroll to an element with id', function() {
@@ -118,6 +119,7 @@ describe('anchorScroll', function() {
         contentEl.append(anchorMe);
 
         del.anchorScroll(animate);
+        timeout.flush();
         expect(sv.scrollTo).toHaveBeenCalledWith(2, 1, animate);
       });
 
@@ -126,6 +128,8 @@ describe('anchorScroll', function() {
         spyOn(sv, 'scrollTo');
         spyOn(ionic.DomUtil, 'getPositionInParent');
         del.anchorScroll(animate);
+        timeout.flush();
+
         expect(sv.scrollTo).toHaveBeenCalledWith(0, 0, animate);
         expect(ionic.DomUtil.getPositionInParent).not.toHaveBeenCalled();
       });
@@ -137,6 +141,7 @@ describe('anchorScroll', function() {
 
         setLocationHash('doesnotexist');
         del.anchorScroll(animate);
+        timeout.flush();
 
         expect(sv.scrollTo).toHaveBeenCalledWith(0, 0, animate);
         expect(ionic.DomUtil.getPositionInParent).not.toHaveBeenCalled();