From da3a97240044619cb5e56a5a9765790b8428f6ff Mon Sep 17 00:00:00 2001 From: epriestley Date: Thu, 5 Jan 2012 16:36:59 -0800 Subject: [PATCH] Retry failed page anchor navigation Summary: When the user loads a page with an anchor on it like #thing, or clicks a link to #thing, and #thing doesn't exist, keep trying to navigate to #thing for a few seconds. This allows anchors to work when the target is in content which is later ajaxed in. In particular, this affects inline comments in Differential. Test Plan: Opened inline comment links in a new tab, was in the right place when I switched tabs. Reviewers: nh, btrahan, jungejason Reviewed By: btrahan CC: aran, btrahan, epriestley Maniphest Tasks: T492 Differential Revision: https://secure.phabricator.com/D1327 --- src/__celerity_resource_map__.php | 57 ++++++++++--------- .../DifferentialInlineCommentView.php | 5 +- .../application/core/behavior-watch-anchor.js | 38 +++++++++++-- 3 files changed, 67 insertions(+), 33 deletions(-) diff --git a/src/__celerity_resource_map__.php b/src/__celerity_resource_map__.php index 4c1e11945a..8dc9bc5a5d 100644 --- a/src/__celerity_resource_map__.php +++ b/src/__celerity_resource_map__.php @@ -731,13 +731,14 @@ celerity_register_resource_map(array( ), 'javelin-behavior-phabricator-watch-anchor' => array( - 'uri' => '/res/96f40736/rsrc/js/application/core/behavior-watch-anchor.js', + 'uri' => '/res/2a4963c3/rsrc/js/application/core/behavior-watch-anchor.js', 'type' => 'js', 'requires' => array( 0 => 'javelin-behavior', 1 => 'javelin-stratcom', 2 => 'javelin-dom', + 3 => 'javelin-vector', ), 'disk' => '/rsrc/js/application/core/behavior-watch-anchor.js', ), @@ -1729,24 +1730,6 @@ celerity_register_resource_map(array( 'uri' => '/res/pkg/11a5c52c/differential.pkg.js', 'type' => 'js', ), - '4e7acf1a' => - array( - 'name' => 'core.pkg.js', - 'symbols' => - array( - 0 => 'javelin-mask', - 1 => 'javelin-workflow', - 2 => 'javelin-behavior-workflow', - 3 => 'javelin-behavior-aphront-form-disable-on-submit', - 4 => 'phabricator-keyboard-shortcut-manager', - 5 => 'phabricator-keyboard-shortcut', - 6 => 'javelin-behavior-phabricator-keyboard-shortcuts', - 7 => 'javelin-behavior-refresh-csrf', - 8 => 'javelin-behavior-phabricator-watch-anchor', - ), - 'uri' => '/res/pkg/4e7acf1a/core.pkg.js', - 'type' => 'js', - ), '540effd7' => array( 'name' => 'typeahead.pkg.js', @@ -1763,6 +1746,24 @@ celerity_register_resource_map(array( 'uri' => '/res/pkg/540effd7/typeahead.pkg.js', 'type' => 'js', ), + '75c76dab' => + array( + 'name' => 'core.pkg.js', + 'symbols' => + array( + 0 => 'javelin-mask', + 1 => 'javelin-workflow', + 2 => 'javelin-behavior-workflow', + 3 => 'javelin-behavior-aphront-form-disable-on-submit', + 4 => 'phabricator-keyboard-shortcut-manager', + 5 => 'phabricator-keyboard-shortcut', + 6 => 'javelin-behavior-phabricator-keyboard-shortcuts', + 7 => 'javelin-behavior-refresh-csrf', + 8 => 'javelin-behavior-phabricator-watch-anchor', + ), + 'uri' => '/res/pkg/75c76dab/core.pkg.js', + 'type' => 'js', + ), '8b139246' => array( 'name' => 'differential.pkg.css', @@ -1855,7 +1856,7 @@ celerity_register_resource_map(array( 'javelin-behavior-aphront-basic-tokenizer' => '540effd7', 'javelin-behavior-aphront-drag-and-drop' => '11a5c52c', 'javelin-behavior-aphront-drag-and-drop-textarea' => '11a5c52c', - 'javelin-behavior-aphront-form-disable-on-submit' => '4e7acf1a', + 'javelin-behavior-aphront-form-disable-on-submit' => '75c76dab', 'javelin-behavior-differential-accept-with-errors' => '11a5c52c', 'javelin-behavior-differential-add-reviewers-and-ccs' => '11a5c52c', 'javelin-behavior-differential-comment-jump' => '11a5c52c', @@ -1865,16 +1866,16 @@ celerity_register_resource_map(array( 'javelin-behavior-differential-keyboard-navigation' => '11a5c52c', 'javelin-behavior-differential-populate' => '11a5c52c', 'javelin-behavior-differential-show-more' => '11a5c52c', - 'javelin-behavior-phabricator-keyboard-shortcuts' => '4e7acf1a', + 'javelin-behavior-phabricator-keyboard-shortcuts' => '75c76dab', 'javelin-behavior-phabricator-object-selector' => '11a5c52c', - 'javelin-behavior-phabricator-watch-anchor' => '4e7acf1a', - 'javelin-behavior-refresh-csrf' => '4e7acf1a', - 'javelin-behavior-workflow' => '4e7acf1a', + 'javelin-behavior-phabricator-watch-anchor' => '75c76dab', + 'javelin-behavior-refresh-csrf' => '75c76dab', + 'javelin-behavior-workflow' => '75c76dab', 'javelin-dom' => 'b164acea', 'javelin-event' => 'b164acea', 'javelin-install' => 'b164acea', 'javelin-json' => 'b164acea', - 'javelin-mask' => '4e7acf1a', + 'javelin-mask' => '75c76dab', 'javelin-request' => 'b164acea', 'javelin-stratcom' => 'b164acea', 'javelin-tokenizer' => '540effd7', @@ -1886,14 +1887,14 @@ celerity_register_resource_map(array( 'javelin-uri' => 'b164acea', 'javelin-util' => 'b164acea', 'javelin-vector' => 'b164acea', - 'javelin-workflow' => '4e7acf1a', + 'javelin-workflow' => '75c76dab', 'phabricator-content-source-view-css' => '8b139246', 'phabricator-core-buttons-css' => 'faf854fc', 'phabricator-core-css' => 'faf854fc', 'phabricator-directory-css' => 'faf854fc', 'phabricator-drag-and-drop-file-upload' => '11a5c52c', - 'phabricator-keyboard-shortcut' => '4e7acf1a', - 'phabricator-keyboard-shortcut-manager' => '4e7acf1a', + 'phabricator-keyboard-shortcut' => '75c76dab', + 'phabricator-keyboard-shortcut-manager' => '75c76dab', 'phabricator-object-selector-css' => '8b139246', 'phabricator-remarkup-css' => 'faf854fc', 'phabricator-shaped-request' => '11a5c52c', diff --git a/src/applications/differential/view/inlinecomment/DifferentialInlineCommentView.php b/src/applications/differential/view/inlinecomment/DifferentialInlineCommentView.php index 28f532c157..b430e7055e 100644 --- a/src/applications/differential/view/inlinecomment/DifferentialInlineCommentView.php +++ b/src/applications/differential/view/inlinecomment/DifferentialInlineCommentView.php @@ -178,10 +178,13 @@ final class DifferentialInlineCommentView extends AphrontView { } } + $anchor_name = 'inline-'.$inline->getID(); + $anchor = phutil_render_tag( 'a', array( - 'name' => 'inline-'.$inline->getID(), + 'name' => $anchor_name, + 'id' => $anchor_name, ), ''); diff --git a/webroot/rsrc/js/application/core/behavior-watch-anchor.js b/webroot/rsrc/js/application/core/behavior-watch-anchor.js index 66cf06dae0..f0a6f7d232 100644 --- a/webroot/rsrc/js/application/core/behavior-watch-anchor.js +++ b/webroot/rsrc/js/application/core/behavior-watch-anchor.js @@ -3,6 +3,7 @@ * @requires javelin-behavior * javelin-stratcom * javelin-dom + * javelin-vector */ JX.behavior('phabricator-watch-anchor', function() { @@ -20,9 +21,38 @@ JX.behavior('phabricator-watch-anchor', function() { } // Defer invocation so other listeners can update the document. - var fn = function() { + function defer_highlight() { setTimeout(highlight, 0); - }; - JX.Stratcom.listen('hashchange', null, fn); - fn(); + } + + // 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(anchor) { + try { + // If the anchor exists, assume the browser handled the jump. + JX.$(anchor); + defer_highlight(); + } catch (e) { + var n = 50; + var try_anchor_again = function () { + try { + window.scrollTo(0, JX.$V(JX.$(anchor)).y - 60); + defer_highlight(); + } catch (e) { + if (n--) { + setTimeout(try_anchor_again, 100); + } + } + }; + try_anchor_again(); + } + } + + JX.Stratcom.listen('hashchange', null, try_anchor); + + var anchor = window.location.hash.replace('#', ''); + if (anchor) { + try_anchor(anchor); + } + });