From 078f6c3fb5f36f3c445cb729288a13cfa34d2f9a Mon Sep 17 00:00:00 2001 From: Robin Ward Date: Wed, 20 Jul 2016 15:13:56 -0400 Subject: [PATCH] FIX: Consistency with HTML anchors --- .../javascripts/discourse/lib/lock-on.js.es6 | 3 + .../discourse/lib/offset-calculator.js.es6 | 2 + .../discourse/lib/static-route-builder.js.es6 | 5 +- .../javascripts/discourse/lib/url.js.es6 | 74 +++++++------------ 4 files changed, 33 insertions(+), 51 deletions(-) diff --git a/app/assets/javascripts/discourse/lib/lock-on.js.es6 b/app/assets/javascripts/discourse/lib/lock-on.js.es6 index 0770083c72f..d6b95945840 100644 --- a/app/assets/javascripts/discourse/lib/lock-on.js.es6 +++ b/app/assets/javascripts/discourse/lib/lock-on.js.es6 @@ -42,6 +42,9 @@ export default class LockOn { clearLock(interval) { $('body,html').off(SCROLL_EVENTS); clearInterval(interval); + if (this.options.finished) { + this.options.finished(); + } } lock() { diff --git a/app/assets/javascripts/discourse/lib/offset-calculator.js.es6 b/app/assets/javascripts/discourse/lib/offset-calculator.js.es6 index e9413eb7710..92dd68dea55 100644 --- a/app/assets/javascripts/discourse/lib/offset-calculator.js.es6 +++ b/app/assets/javascripts/discourse/lib/offset-calculator.js.es6 @@ -19,6 +19,8 @@ export default function offsetCalculator(y) { const ideal = headerHeight + ((expectedOffset < 0) ? 0 : expectedOffset); const $container = $('.posts-wrapper'); + if ($container.length === 0) { return expectedOffset; } + const topPos = $container.offset().top; const scrollTop = y || $(window).scrollTop(); diff --git a/app/assets/javascripts/discourse/lib/static-route-builder.js.es6 b/app/assets/javascripts/discourse/lib/static-route-builder.js.es6 index 0174c16483d..7a1baf2e274 100644 --- a/app/assets/javascripts/discourse/lib/static-route-builder.js.es6 +++ b/app/assets/javascripts/discourse/lib/static-route-builder.js.es6 @@ -1,5 +1,5 @@ -import DiscourseURL from 'discourse/lib/url'; import StaticPage from 'discourse/models/static-page'; +import { default as DiscourseURL, jumpToElement } from 'discourse/lib/url'; const configs = { "faq": "faq_url", @@ -23,8 +23,7 @@ export default function(page) { activate() { this._super(); - // Scroll to an element if exists - DiscourseURL.scrollToId(document.location.hash); + jumpToElement(document.location.hash.substr(1)); }, model() { diff --git a/app/assets/javascripts/discourse/lib/url.js.es6 b/app/assets/javascripts/discourse/lib/url.js.es6 index 5bb54f93585..4b72cc26ebb 100644 --- a/app/assets/javascripts/discourse/lib/url.js.es6 +++ b/app/assets/javascripts/discourse/lib/url.js.es6 @@ -2,16 +2,28 @@ import offsetCalculator from 'discourse/lib/offset-calculator'; import LockOn from 'discourse/lib/lock-on'; import { defaultHomepage } from 'discourse/lib/utilities'; -let _jumpScheduled = false; const rewrites = []; - const TOPIC_REGEXP = /\/t\/([^\/]+)\/(\d+)\/?(\d+)?/; +let _jumpScheduled = false; +export function jumpToElement(elementId) { + if (_jumpScheduled || Ember.isEmpty(elementId)) { return; } + + const selector = `#${elementId}, a[name=${elementId}]`; + _jumpScheduled = true; + Ember.run.schedule('afterRender', function() { + const lockon = new LockOn(selector, { + finished() { + _jumpScheduled = false; + } + }); + lockon.lock(); + }); +} + const DiscourseURL = Ember.Object.extend({ - // Used for matching a topic - - isJumpScheduled: function() { + isJumpScheduled() { return _jumpScheduled; }, @@ -56,13 +68,8 @@ const DiscourseURL = Ember.Object.extend({ }); }, - /** - Browser aware replaceState. Will only be invoked if the browser supports it. - - @method replaceState - @param {String} path The path we are replacing our history state with. - **/ - replaceState: function(path) { + // Browser aware replaceState. Will only be invoked if the browser supports it. + replaceState(path) { if (window.history && window.history.pushState && window.history.replaceState && @@ -81,23 +88,6 @@ const DiscourseURL = Ember.Object.extend({ } }, - // Scroll to the same page, different anchor - scrollToId(id) { - if (Em.isEmpty(id)) { return; } - - _jumpScheduled = true; - Em.run.schedule('afterRender', function() { - let $elem = $(id); - if ($elem.length === 0) { - $elem = $("[name='" + id.replace('#', '') + "']"); - } - if ($elem.length > 0) { - $('html,body').scrollTop($elem.offset().top - $('header').height() - 15); - _jumpScheduled = false; - } - }); - }, - routeToTag(a) { if (a && a.host !== document.location.host) { document.location = a.href; @@ -131,10 +121,10 @@ const DiscourseURL = Ember.Object.extend({ } // Scroll to the same page, different anchor - if (path.indexOf('#') === 0) { - this.scrollToId(path); - this.replaceState(path); - return; + const m = /#(.+)$/.exec(path); + if (m) { + jumpToElement(m[1]); + return this.replaceState(path); } const oldPath = window.location.pathname; @@ -299,7 +289,7 @@ const DiscourseURL = Ember.Object.extend({ // Get a controller. Note that currently it uses `__container__` which is not // advised but there is no other way to access the router. - controllerFor: function(name) { + controllerFor(name) { return Discourse.__container__.lookup('controller:' + name); }, @@ -307,7 +297,7 @@ const DiscourseURL = Ember.Object.extend({ Be wary of looking up the router. In this case, we have links in our HTML, say form compiled markdown posts, that need to be routed. **/ - handleURL: function(path, opts) { + handleURL(path, opts) { opts = opts || {}; const router = this.get('router'); @@ -328,19 +318,7 @@ const DiscourseURL = Ember.Object.extend({ const transition = router.handleURL(path); transition._discourse_intercepted = true; - transition.promise.then(function() { - if (elementId) { - - _jumpScheduled = true; - Em.run.next('afterRender', function() { - const offset = $('#' + elementId).offset(); - if (offset && offset.top) { - $('html, body').scrollTop(offset.top - $('header').height() - 10); - _jumpScheduled = false; - } - }); - } - }); + transition.promise.then(() => jumpToElement(elementId)); } }).create();