From 74d6bcbdceb99d46925d709aba92e5b548136165 Mon Sep 17 00:00:00 2001 From: epriestley Date: Wed, 18 Sep 2019 14:44:51 -0700 Subject: [PATCH] Allow a user to target "#anchor" by navigating to any prefix Summary: Ref T13410. We currently generate some less-than-ideal anchors in remarkup, but it's hard to change the algorithm without breaking stuff. To mitigate this, allow `#xyz` to match any target on the page which begins with `xyz`. This means we can make anchors longer with no damage, and savvy users are free to shorten anchors to produce more presentation-friendly links. Test Plan: Browsed to `#header-th`, was scrolled to `#header-three`, etc. Maniphest Tasks: T13410 Differential Revision: https://secure.phabricator.com/D20820 --- resources/celerity/map.php | 18 +-- webroot/rsrc/js/core/behavior-watch-anchor.js | 122 ++++++++++++------ 2 files changed, 95 insertions(+), 45 deletions(-) diff --git a/resources/celerity/map.php b/resources/celerity/map.php index 73e7c19144..13a2619854 100644 --- a/resources/celerity/map.php +++ b/resources/celerity/map.php @@ -10,7 +10,7 @@ return array( 'conpherence.pkg.css' => '3c8a0668', 'conpherence.pkg.js' => '020aebcf', 'core.pkg.css' => 'c69171e6', - 'core.pkg.js' => '73a06a9f', + 'core.pkg.js' => '6e5c894f', 'differential.pkg.css' => '8d8360fb', 'differential.pkg.js' => '0b037a4f', 'diffusion.pkg.css' => '42c75c37', @@ -506,7 +506,7 @@ return array( 'rsrc/js/core/behavior-tokenizer.js' => '3b4899b0', 'rsrc/js/core/behavior-tooltip.js' => '73ecc1f8', 'rsrc/js/core/behavior-user-menu.js' => '60cd9241', - 'rsrc/js/core/behavior-watch-anchor.js' => '0e6d261f', + 'rsrc/js/core/behavior-watch-anchor.js' => '3972dadb', 'rsrc/js/core/behavior-workflow.js' => '9623adc1', 'rsrc/js/core/darkconsole/DarkLog.js' => '3b869402', 'rsrc/js/core/darkconsole/DarkMessage.js' => '26cd4b73', @@ -655,7 +655,7 @@ return array( 'javelin-behavior-phabricator-tooltips' => '73ecc1f8', 'javelin-behavior-phabricator-transaction-comment-form' => '2bdadf1a', 'javelin-behavior-phabricator-transaction-list' => '9cec214e', - 'javelin-behavior-phabricator-watch-anchor' => '0e6d261f', + 'javelin-behavior-phabricator-watch-anchor' => '3972dadb', 'javelin-behavior-pholio-mock-edit' => '3eed1f2b', 'javelin-behavior-pholio-mock-view' => '5aa1544e', 'javelin-behavior-phui-dropdown-menu' => '5cf0501a', @@ -999,12 +999,6 @@ return array( '0d2490ce' => array( 'javelin-install', ), - '0e6d261f' => array( - 'javelin-behavior', - 'javelin-stratcom', - 'javelin-dom', - 'javelin-vector', - ), '0eaa33a9' => array( 'javelin-behavior', 'javelin-dom', @@ -1227,6 +1221,12 @@ return array( 'javelin-install', 'javelin-dom', ), + '3972dadb' => array( + 'javelin-behavior', + 'javelin-stratcom', + 'javelin-dom', + 'javelin-vector', + ), '398fdf13' => array( 'javelin-behavior', 'trigger-rule-editor', diff --git a/webroot/rsrc/js/core/behavior-watch-anchor.js b/webroot/rsrc/js/core/behavior-watch-anchor.js index 49cac764ae..4f8c6b93b4 100644 --- a/webroot/rsrc/js/core/behavior-watch-anchor.js +++ b/webroot/rsrc/js/core/behavior-watch-anchor.js @@ -8,52 +8,102 @@ JX.behavior('phabricator-watch-anchor', function() { - var highlighted; + // When the user loads a page with an "#anchor" or changes the "#anchor" on + // an existing page, we try to scroll the page to the relevant location. - function highlight() { - highlighted && JX.DOM.alterClass(highlighted, 'anchor-target', false); - try { - highlighted = JX.$('anchor-' + window.location.hash.replace('#', '')); - } catch (ex) { - highlighted = null; - } - highlighted && JX.DOM.alterClass(highlighted, 'anchor-target', true); - } + // Browsers do this on their own, but we have some additional rules to try + // to match anchors more flexibly and handle cases where an anchor is not + // yet present in the document because something is still loading or + // rendering it, often via Ajax. - // Defer invocation so other listeners can update the document. - function defer_highlight() { - setTimeout(highlight, 0); - } + // Number of milliseconds we'll keep trying to find an anchor for. + var wait_max = 5000; + + // Wait between retries. + var wait_ms = 100; + + var target; + var retry_ms; - // In some cases, we link to an anchor but the anchor target ajaxes in - // later. If it pops in within the first few seconds, jump to it. function try_anchor() { + retry_ms = wait_max; + seek_anchor(); + } + + function seek_anchor() { var anchor = window.location.hash.replace('#', ''); - try { - // If the anchor exists, assume the browser handled the jump. - if (anchor) { - JX.$(anchor); + + if (!anchor.length) { + return; + } + + var ii; + var node = null; + + // When the user navigates to "#abc", we'll try to find a node with + // either ID "abc" or ID "anchor-abc". + var ids = [anchor, 'anchor-' + anchor]; + + for (ii = 0; ii < ids.length; ii++) { + try { + node = JX.$(ids[ii]); + break; + } catch (e) { + // Continue. } - defer_highlight(); - } catch (e) { - var n = 50; - var try_anchor_again = function () { - try { - var node = JX.$(anchor); - var pos = JX.Vector.getPosWithScroll(node); - JX.DOM.scrollToPosition(0, pos.y - 60); - defer_highlight(); - } catch (e) { - if (n--) { - setTimeout(try_anchor_again, 100); - } + } + + // If we haven't found a matching node yet, look for an "" tag with + // a "name" attribute that has our anchor as a prefix. For example, you + // can navigate to "#cat" and we'll match "#cat-and-mouse". + + if (!node) { + var anchor_nodes = JX.DOM.scry(document.body, 'a'); + for (ii = 0; ii < anchor_nodes.length; ii++) { + if (!anchor_nodes[ii].name) { + continue; } - }; - try_anchor_again(); + + if (anchor_nodes[ii].name.substring(0, anchor.length) === anchor) { + node = anchor_nodes[ii]; + break; + } + } + } + + // If we already have an anchor highlighted, unhighlight it and throw + // it away if it doesn't match the new target. + if (target && (target !== node)) { + JX.DOM.alterClass(target, 'anchor-target', false); + target = null; + } + + // If we didn't find a matching anchor, try again soon. This allows + // rendering logic some time to complete Ajax requests and draw elements + // onto the page. + if (!node) { + if (retry_ms > 0) { + retry_ms -= wait_ms; + setTimeout(try_anchor, wait_ms); + return; + } + } + + // If we've found a new target, highlight it. + if (target !== node) { + target = node; + JX.DOM.alterClass(target, 'anchor-target', true); + } + + // Try to scroll to the new target. + try { + var pos = JX.Vector.getPosWithScroll(node); + JX.DOM.scrollToPosition(0, pos.y - 60); + } catch (e) { + // Ignore issues with scrolling the document. } } JX.Stratcom.listen('hashchange', null, try_anchor); try_anchor(); - });