FIX: Prevent `LockOn` conflicts (#10422)

FIX: Prevent LockOn conflicts (#10422)

If there’s already a LockOn instance, clear its lock before creating creating a new one. Fixes a shaky viewport effect after certain transitions.

Includes:

  • Slight refactor (elementId wasn’t an id, but a selector - it included the “#” prefix)
  • Add support for a[name=X] anchors in jumpToPost
  • Scope down anchors to the #main element (Embeded fontawesome sprites are causing conflicts, e.g. when given bed anchor, <a name="bed"> was at odds with <symbol id="bed" viewBox="0 0 640 512">(…)</symbol>)
diff --git a/app/assets/javascripts/discourse/app/lib/url.js b/app/assets/javascripts/discourse/app/lib/url.js
index 8ed1998..aa7fbb7 100644
--- a/app/assets/javascripts/discourse/app/lib/url.js
+++ b/app/assets/javascripts/discourse/app/lib/url.js
@@ -66,25 +66,32 @@ export function groupPath(subPath) {
 }
 
 let _jumpScheduled = false;
+let _transitioning = false;
+let lockon = null;
+
 export function jumpToElement(elementId) {
   if (_jumpScheduled || isEmpty(elementId)) {
     return;
   }
 
-  const selector = `#${elementId}, a[name=${elementId}]`;
+  const selector = `#main #${elementId}, a[name=${elementId}]`;
   _jumpScheduled = true;
+
   schedule("afterRender", function() {
-    const lockon = new LockOn(selector, {
+    if (lockon) {
+      lockon.clearLock();
+    }
+
+    lockon = new LockOn(selector, {
       finished() {
         _jumpScheduled = false;
+        lockon = null;
       }
     });
     lockon.lock();
   });
 }
 
-let _transitioning = false;
-
 const DiscourseURL = EmberObject.extend({
   isJumpScheduled() {
     return _transitioning || _jumpScheduled;
@@ -98,9 +105,6 @@ const DiscourseURL = EmberObject.extend({
     _transitioning = postNumber > 1;
 
     schedule("afterRender", () => {
-      let elementId;
-      let holder;
-
       if (opts.jumpEnd) {
         let $holder = $(holderId);
         let holderHeight = $holder.height();
@@ -125,27 +129,35 @@ const DiscourseURL = EmberObject.extend({
         return;
       }
 
+      let selector;
+      let holder;
+
       if (opts.anchor) {
-        elementId = opts.anchor;
-        holder = $(elementId);
+        selector = `#main #${opts.anchor}, a[name=${opts.anchor}]`;
+        holder = document.querySelector(selector);
+      }
+
+      if (!holder) {
+        selector = holderId;
+        holder = document.querySelector(selector);
       }
 
-      if (!holder || holder.length === 0) {
-        elementId = holderId;
-        holder = $(elementId);
+      if (lockon) {
+        lockon.clearLock();
       }
 
-      const lockon = new LockOn(elementId, {
+      lockon = new LockOn(selector, {
         finished() {
           _transitioning = false;
+          lockon = null;
         }
       });
 
-      if (holder.length > 0 && opts && opts.skipIfOnScreen) {
+      if (holder && opts.skipIfOnScreen) {
         const elementTop = lockon.elementTop();
         const scrollTop = $(window).scrollTop();
         const windowHeight = $(window).height() - offsetCalculator();
-        const height = holder.height();
+        const height = $(holder).height();
 
         if (
           elementTop > scrollTop &&
@@ -377,9 +389,9 @@ const DiscourseURL = EmberObject.extend({
             jumpEnd: routeOpts.jumpEnd
           };
 
-          const m = /#.+$/.exec(path);
-          if (m) {
-            jumpOpts.anchor = m[0];
+          const anchorMatch = /#(.+)$/.exec(path);
+          if (anchorMatch) {
+            jumpOpts.anchor = anchorMatch[1];
           }
 
           this.jumpToPost(closest, jumpOpts);
diff --git a/app/assets/javascripts/discourse/app/routes/post.js b/app/assets/javascripts/discourse/app/routes/post.js
index 15b8aa3..993e758 100644
--- a/app/assets/javascripts/discourse/app/routes/post.js
+++ b/app/assets/javascripts/discourse/app/routes/post.js
@@ -2,14 +2,16 @@ import DiscourseRoute from "discourse/routes/discourse";
 import { ajax } from "discourse/lib/ajax";
 
 export default DiscourseRoute.extend({
-  beforeModel({ params }) {
+  beforeModel({ params, _discourse_anchor }) {
     return ajax(`/p/${params.post.id}`).then(t => {
-      this.transitionTo(
+      const transition = this.transitionTo(
         "topic.fromParamsNear",
         t.slug,
         t.id,
         t.current_post_number
       );
+
+      transition._discourse_anchor = _discourse_anchor;
     });
   }
 });
diff --git a/app/assets/javascripts/discourse/app/routes/topic-from-params.js b/app/assets/javascripts/discourse/app/routes/topic-from-params.js
index 275d870..e2b7af2 100644
--- a/app/assets/javascripts/discourse/app/routes/topic-from-params.js
+++ b/app/assets/javascripts/discourse/app/routes/topic-from-params.js
@@ -17,7 +17,7 @@ export default DiscourseRoute.extend({
     this.controllerFor("topic").unsubscribe();
   },
 
-  setupController(controller, params) {
+  setupController(controller, params, { _discourse_anchor }) {
     params = params || {};
     params.track_visit = true;
 
@@ -65,8 +65,10 @@ export default DiscourseRoute.extend({
         );
 
         const opts = {};
-        if (document.location.hash && document.location.hash.length) {
-          opts.anchor = document.location.hash;
+        if (document.location.hash) {
+          opts.anchor = document.location.hash.substr(1);
+        } else if (_discourse_anchor) {
+          opts.anchor = _discourse_anchor;
         }
         DiscourseURL.jumpToPost(closest, opts);
 

GitHub sha: d0d651d8

This commit appears in #10422 which was approved by eviltrout. It was merged by CvX.