refactor: simplify & document `Magellan._updateActive()`

Changes:
* add comments
* rename variables for improved readibility
* use narrow functions instead of `_this`, and `let`/`const`
* prevent useless DOM manipulations when $active did not changed

Unit tests run, as well as visual tests

diff --git a/js/foundation.magellan.js b/js/foundation.magellan.js
index 43398020852841fd978f31bbdb4772a415e5a641..2fc40b404246cbdc5776463155ac95faf717cbc5 100644 (file)
--- a/js/foundation.magellan.js
+++ b/js/foundation.magellan.js
@@ -149,58 +149,63 @@ class Magellan extends Plugin {
    * @fires Magellan#update
    */
   _updateActive(/*evt, elem, scrollPos*/) {
-    if(this._inTransition) {return;}
-    var winPos = /*scrollPos ||*/ parseInt(window.pageYOffset, 10),
-        curIdx;
+    if(this._inTransition) return;
 
-    if(winPos + this.winHeight === this.docHeight){ curIdx = this.points.length - 1; }
-    else if(winPos < this.points[0]){ curIdx = undefined; }
+    const newScrollPos = parseInt(window.pageYOffset, 10);
+    const isScrollingUp = this.scrollPos > newScrollPos;
+    this.scrollPos = newScrollPos;
+
+    let activeIdx;
+    // Before the first point: no link
+    if(newScrollPos < this.points[0]){ /* do nothing */ }
+    // At the bottom of the page: last link
+    else if(newScrollPos + this.winHeight === this.docHeight){ activeIdx = this.points.length - 1; }
+    // Otherwhise, use the last visible link
     else{
-      var isDown = this.scrollPos < winPos,
-          _this = this,
-          curVisible = this.points.filter(function(p, i){
-            return isDown ? p - _this.options.offset <= winPos : p - _this.options.offset - _this.options.threshold <= winPos;
-          });
-      curIdx = curVisible.length ? curVisible.length - 1 : 0;
+      const visibleLinks = this.points.filter((p, i) => {
+        return (p - this.options.offset - (isScrollingUp ? this.options.threshold : 0)) <= newScrollPos;
+      });
+      activeIdx = visibleLinks.length ? visibleLinks.length - 1 : 0;
     }
 
-    var $oldActive = this.$active;
-    this.$active.removeClass(this.options.activeClass);
-    if(curIdx !== undefined){
-      this.$active = this.$links.filter('[href="#' + this.$targets.eq(curIdx).data('magellan-target') + '"]').addClass(this.options.activeClass);
+    // Get the new active link
+    const $oldActive = this.$active;
+    let activeHash = '';
+    if(activeIdx !== undefined){
+      this.$active = this.$links.filter('[href="#' + this.$targets.eq(activeIdx).data('magellan-target') + '"]');
+      activeHash = this.$active[0].getAttribute('href');
     }else{
       this.$active = $();
     }
-    var changed = !(!this.$active.length && !$oldActive.length) && !this.$active.is($oldActive);
+    const isNewActive = !(!this.$active.length && !$oldActive.length) && !this.$active.is($oldActive);
+    const isNewHash = activeHash !== window.location.hash;
 
-    if(this.options.deepLinking){
-      var hash = "";
-      if(curIdx !== undefined){
-        hash = this.$active[0].getAttribute('href');
-      }
-      if(hash !== window.location.hash) {
-        if(window.history.pushState){
-          // If there is no active idx, move to the same url without hash
-          // https://stackoverflow.com/a/5298684/4317384
-          var url = curIdx !== undefined ? hash : window.location.pathname + window.location.search;
+    // Update the active link element
+    if(isNewActive) {
+      $oldActive.removeClass(this.options.activeClass);
+      this.$active.addClass(this.options.activeClass);
+    }
 
-          if(this.options.updateHistory){
-            window.history.pushState({}, '', url);
-          }else{
-            window.history.replaceState({}, '', url);
-          }
+    // Update the hash (it may have changed with the same active link)
+    if(this.options.deepLinking && isNewHash){
+      if(window.history.pushState){
+        // Set or remove the hash (see: https://stackoverflow.com/a/5298684/4317384
+        const url = activeHash ? activeHash : window.location.pathname + window.location.search;
+        if(this.options.updateHistory){
+          window.history.pushState({}, '', url);
         }else{
-          window.location.hash = hash;
+          window.history.replaceState({}, '', url);
         }
+      }else{
+        window.location.hash = activeHash;
       }
     }
 
-    this.scrollPos = winPos;
-    /**
-     * Fires when magellan is finished updating to the new active element.
-     * @event Magellan#update
-     */
-    if (changed === true) {
+    if (isNewActive) {
+      /**
+       * Fires when magellan is finished updating to the new active element.
+       * @event Magellan#update
+       */
        this.$element.trigger('update.zf.magellan', [this.$active]);
          }
   }