From 0be41096888201831c151864c0ab05cf0b5ca734 Mon Sep 17 00:00:00 2001 From: Mike Greiling Date: Thu, 29 May 2014 02:24:41 -0500 Subject: [PATCH 1/4] there is no need for this.offsets and this.targets to be jQuery objects --- js/scrollspy.js | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/js/scrollspy.js b/js/scrollspy.js index cddde45ad..de4f98707 100644 --- a/js/scrollspy.js +++ b/js/scrollspy.js @@ -24,8 +24,8 @@ this.selector = (this.options.target || ((href = $(element).attr('href')) && href.replace(/.*(?=#[^\s]+$)/, '')) //strip for ie7 || '') + ' .nav li > a' - this.offsets = $([]) - this.targets = $([]) + this.offsets = [] + this.targets = [] this.activeTarget = null this.refresh() @@ -41,8 +41,9 @@ ScrollSpy.prototype.refresh = function () { var offsetMethod = this.$element[0] == window ? 'offset' : 'position' - this.offsets = $([]) - this.targets = $([]) + this.offsets = [] + this.targets = [] + var self = this this.$body @@ -75,7 +76,7 @@ var i if (scrollTop >= maxScroll) { - return activeTarget != (i = targets.last()[0]) && this.activate(i) + return activeTarget != (i = targets[targets.length - 1]) && this.activate(i) } if (activeTarget && scrollTop <= offsets[0]) { From 632313d631b51d96557bb6576c0b41e33cbfb009 Mon Sep 17 00:00:00 2001 From: Mike Greiling Date: Thu, 29 May 2014 02:30:18 -0500 Subject: [PATCH 2/4] no need to check for href attribute on the scrollable element --- js/scrollspy.js | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/js/scrollspy.js b/js/scrollspy.js index de4f98707..db493d7ed 100644 --- a/js/scrollspy.js +++ b/js/scrollspy.js @@ -14,16 +14,13 @@ // ========================== function ScrollSpy(element, options) { - var href var process = $.proxy(this.process, this) this.$element = $(element).is('body') ? $(window) : $(element) this.$body = $('body') this.$scrollElement = this.$element.on('scroll.bs.scrollspy', process) this.options = $.extend({}, ScrollSpy.DEFAULTS, options) - this.selector = (this.options.target - || ((href = $(element).attr('href')) && href.replace(/.*(?=#[^\s]+$)/, '')) //strip for ie7 - || '') + ' .nav li > a' + this.selector = (this.options.target || '') + ' .nav li > a' this.offsets = [] this.targets = [] this.activeTarget = null From bc8e6ffb7d702895296050d2fd8847fb5eed2381 Mon Sep 17 00:00:00 2001 From: Mike Greiling Date: Thu, 29 May 2014 02:36:53 -0500 Subject: [PATCH 3/4] removing this.$element as it is a superfluous copy of this.$scrollElement --- js/scrollspy.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/js/scrollspy.js b/js/scrollspy.js index db493d7ed..94171bb1b 100644 --- a/js/scrollspy.js +++ b/js/scrollspy.js @@ -16,15 +16,15 @@ function ScrollSpy(element, options) { var process = $.proxy(this.process, this) - this.$element = $(element).is('body') ? $(window) : $(element) this.$body = $('body') - this.$scrollElement = this.$element.on('scroll.bs.scrollspy', process) + this.$scrollElement = $(element).is('body') ? $(window) : $(element) this.options = $.extend({}, ScrollSpy.DEFAULTS, options) this.selector = (this.options.target || '') + ' .nav li > a' this.offsets = [] this.targets = [] this.activeTarget = null + this.$scrollElement.on('scroll.bs.scrollspy', process) this.refresh() this.process() } @@ -36,7 +36,7 @@ } ScrollSpy.prototype.refresh = function () { - var offsetMethod = this.$element[0] == window ? 'offset' : 'position' + var offsetMethod = this.$scrollElement[0] == window ? 'offset' : 'position' this.offsets = [] this.targets = [] From 1cfa902e313e848732976ac6e538f3f72c135d90 Mon Sep 17 00:00:00 2001 From: Mike Greiling Date: Thu, 29 May 2014 02:51:49 -0500 Subject: [PATCH 4/4] refactor scrollspy refresh method Closes #13702 by merging a rebased version of it. --- js/scrollspy.js | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/js/scrollspy.js b/js/scrollspy.js index 94171bb1b..644c65f94 100644 --- a/js/scrollspy.js +++ b/js/scrollspy.js @@ -36,7 +36,13 @@ } ScrollSpy.prototype.refresh = function () { - var offsetMethod = this.$scrollElement[0] == window ? 'offset' : 'position' + var offsetMethod = 'offset' + var offsetBase = 0 + + if (!$.isWindow(this.$scrollElement[0])) { + offsetMethod = 'position' + offsetBase = this.$scrollElement.scrollTop() + } this.offsets = [] this.targets = [] @@ -54,7 +60,7 @@ return ($href && $href.length && $href.is(':visible') - && [[$href[offsetMethod]().top + (!$.isWindow(self.$scrollElement.get(0)) && self.$scrollElement.scrollTop()), href]]) || null + && [[$href[offsetMethod]().top + offsetBase, href]]) || null }) .sort(function (a, b) { return a[0] - b[0] }) .each(function () {