Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

Commit 05eca64

Browse files
squash
1 parent f5595df commit 05eca64

File tree

2 files changed

+69
-32
lines changed

2 files changed

+69
-32
lines changed

src/ng/anchorScroll.js

Lines changed: 16 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -212,20 +212,21 @@ function $AnchorScrollProvider() {
212212
var offset = getYOffset();
213213

214214
if (offset) {
215-
// `offset` is the number of pixels we should scroll up in order to align `elem` properly.
215+
// `offset` is the number of pixels we should scroll UP in order to align `elem` properly.
216216
// This is true ONLY if the call to `elem.scrollIntoView()` initially aligns `elem` at the
217-
// top of the viewport. IF the number of pixels from the top of `elem` to the end of the
218-
// page's content is less than the height of the viewport, then `elem.scrollIntoView()`
219-
// will NOT align the top of `elem` at the top of the viewport (but further down). This is
220-
// often the case for elements near the bottom of the page.
221-
// In such cases we do not need to scroll the whole `offset` up, just the fraction of the
222-
// offset that is necessary to align the top of `elem` at the desired position.
223-
// ---
224-
// Note: getBoundingClientRect()'s top is relative to the top of the viewport
225-
// (not the page), which is exactly what interests us.
226-
var necessaryOffset = offset - elem.getBoundingClientRect().top;
227-
228-
$window.scrollBy(0, -1 * necessaryOffset);
217+
// top of the viewport.
218+
//
219+
// IF the number of pixels from the top of `elem` to the end of the page's content is less
220+
// than the height of the viewport, then `elem.scrollIntoView()` will align the `elem` some
221+
// way down the page.
222+
//
223+
// This is often the case for elements near the bottom of the page.
224+
//
225+
// In such cases we do not need to scroll the whole `offset` up, just the difference between
226+
// the top of the element and the offset, which is enough to align the top of `elem` at the
227+
// desired position.
228+
var elemTop = elem.getBoundingClientRect().top;
229+
$window.scrollBy(0, elemTop - offset);
229230
}
230231
} else {
231232
$window.scrollTo(0, 0);
@@ -237,12 +238,12 @@ function $AnchorScrollProvider() {
237238
$rootScope.$evalAsync(scroll);
238239
} else if (!scrollScheduled) {
239240
scrollScheduled = true;
240-
angular.element($window).on('load', function unbindAndScroll() {
241+
jqLite($window).on('load', function unbindAndScroll() {
241242
// When navigating to a page with a URL including a hash,
242243
// Firefox overwrites our `yOffset` if `$apply()` is used instead.
243244
$rootScope.$evalAsync(function() {
244245
scrollScheduled = false;
245-
angular.element($window).off('load', unbindAndScroll);
246+
jqLite($window).off('load', unbindAndScroll);
246247
scroll();
247248
});
248249
});

test/ng/anchorScrollSpec.js

Lines changed: 53 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -235,6 +235,47 @@ describe('$anchorScroll', function() {
235235
};
236236
}
237237

238+
239+
function spyOnJQLiteOnOff() {
240+
return function() {
241+
spyOn(jqLite.prototype,'on').andCallThrough();
242+
spyOn(jqLite.prototype,'off').andCallThrough();
243+
};
244+
}
245+
246+
function unspyOnJQLiteOnOff() {
247+
return function() {
248+
jqLite.prototype.on = jqLite.prototype.on.originalValue;
249+
jqLite.prototype.off = jqLite.prototype.off.originalValue;
250+
};
251+
}
252+
253+
function expectJQLiteOnOffCallsToEqual(callCount) {
254+
return function() {
255+
var onCalls = 0, offCalls = 0;
256+
257+
forEach(jqLite.prototype.on.calls, function(call) {
258+
if ( call.args[0] === 'load' ) {
259+
onCalls += 1;
260+
}
261+
});
262+
263+
forEach(jqLite.prototype.off.calls, function(call) {
264+
if ( call.args[0] === 'load' ) {
265+
offCalls += 1;
266+
}
267+
});
268+
};
269+
}
270+
271+
function expectJQLiteOnOffCallsToHaveSameHandler() {
272+
return function() {
273+
var registeredListener = jqLite.prototype.on.mostRecentCall.args[1];
274+
var unregisteredListener = jqLite.prototype.off.mostRecentCall.args[1];
275+
expect(unregisteredListener).toBe(registeredListener);
276+
};
277+
}
278+
238279
beforeEach(createMockWindow('interactive'));
239280

240281

@@ -259,26 +300,21 @@ describe('$anchorScroll', function() {
259300
expectScrollingTo('id=some2')));
260301

261302

262-
it('should properly register and unregister listeners for the `load` event', inject(
263-
addElements('id=some1', 'id=some2'),
264-
265-
changeHashTo('some1'),
266-
changeHashTo('some2'),
267-
triggerLoadEvent(),
303+
it('should properly register and unregister listeners for the `load` event', function() {
304+
module(spyOnJQLiteOnOff());
305+
inject(
306+
addElements('id=some1', 'id=some2'),
268307

269-
function() {
270-
expect(windowSpies.addEventListener.callCount).toBe(1);
271-
expect(windowSpies.addEventListener).
272-
toHaveBeenCalledWith('load', jasmine.any(Function), false);
308+
changeHashTo('some1'),
309+
changeHashTo('some2'),
273310

274-
expect(windowSpies.removeEventListener.callCount).toBe(1);
275-
expect(windowSpies.removeEventListener).
276-
toHaveBeenCalledWith('load', jasmine.any(Function), false);
311+
triggerLoadEvent(),
277312

278-
var registeredListener = windowSpies.addEventListener.calls[0].args[1];
279-
var unregisteredListener = windowSpies.removeEventListener.calls[0].args[1];
280-
expect(unregisteredListener).toBe(registeredListener);
281-
}));
313+
expectJQLiteOnOffCallsToEqual(1),
314+
expectJQLiteOnOffCallsToHaveSameHandler(),
315+
unspyOnJQLiteOnOff()
316+
);
317+
});
282318

283319

284320
it('should scroll immediately if already `readyState === "complete"`', inject(

0 commit comments

Comments
 (0)