From 1c4d4a8b90e24a277187c7505538dbf461b95d11 Mon Sep 17 00:00:00 2001 From: Andy Joslin Date: Tue, 11 Feb 2014 09:44:52 -0500 Subject: [PATCH] fix(browser): on first hash-set, dont set scrollTop --- js/ext/angular/src/ionicAngular.js | 3 ++- .../src/service/decorators/location.js | 27 +++++++++++++++++++ .../service/delegates/ionicScrollDelegate.js | 18 +++++++------ js/ext/angular/test/anchorScroll.html | 3 +-- .../test/service/decorators/location.unit.js | 25 +++++++++++++++++ .../delegates/ionicScrollDelegate.unit.js | 7 ++++- 6 files changed, 71 insertions(+), 12 deletions(-) create mode 100644 js/ext/angular/src/service/decorators/location.js create mode 100644 js/ext/angular/test/service/decorators/location.unit.js diff --git a/js/ext/angular/src/ionicAngular.js b/js/ext/angular/src/ionicAngular.js index 5270b64aae..2bb04721a4 100644 --- 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 new file mode 100644 index 0000000000..4d1bde139e --- /dev/null +++ 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 index d43555217b..d4bb6aa579 100644 --- 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 index 1f9191311e..6c47a6fd28 100644 --- 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); } } diff --git a/js/ext/angular/test/service/decorators/location.unit.js b/js/ext/angular/test/service/decorators/location.unit.js new file mode 100644 index 0000000000..1534b64593 --- /dev/null +++ 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 index 397264a236..17f7562f26 100644 --- 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('')(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();