From 54ec566281b1dc62688544a6d949de9ff093e423 Mon Sep 17 00:00:00 2001 From: epriestley Date: Mon, 4 May 2020 09:57:30 -0700 Subject: [PATCH] Restore highlighting when jumping to transactions using URI anchors Summary: At some point, the highlighting behavior for the timeline broke. When you follow a link to a particular timeline story, the story should be highlighted. Prior to this change, the `` tag itself highlights, but there's no associated CSS and it's too deep in the tree to do anything useful. (Since this change is fairly straightforward, I gave up digging for the root cause before finding it.) Test Plan: - Clicked a timeline story anchor, saw the story highlight. Differential Revision: https://secure.phabricator.com/D21213 --- resources/celerity/map.php | 24 +++++++++---------- src/view/phui/PHUITimelineEventView.php | 10 ++++---- webroot/rsrc/css/phui/phui-timeline-view.css | 3 +++ webroot/rsrc/js/core/behavior-watch-anchor.js | 17 +++++++++++-- 4 files changed, 35 insertions(+), 19 deletions(-) diff --git a/resources/celerity/map.php b/resources/celerity/map.php index 906b8e97ba..3aa68da324 100644 --- a/resources/celerity/map.php +++ b/resources/celerity/map.php @@ -9,8 +9,8 @@ return array( 'names' => array( 'conpherence.pkg.css' => '0e3cf785', 'conpherence.pkg.js' => '020aebcf', - 'core.pkg.css' => '589cd2fe', - 'core.pkg.js' => '49814bac', + 'core.pkg.css' => 'f5ebbd7d', + 'core.pkg.js' => '632fb8f5', 'dark-console.pkg.js' => '187792c2', 'differential.pkg.css' => '2d70b7b9', 'differential.pkg.js' => 'c8f88d74', @@ -178,7 +178,7 @@ return array( 'rsrc/css/phui/phui-spacing.css' => 'b05cadc3', 'rsrc/css/phui/phui-status.css' => 'e5ff8be0', 'rsrc/css/phui/phui-tag-view.css' => '8519160a', - 'rsrc/css/phui/phui-timeline-view.css' => '1e348e4b', + 'rsrc/css/phui/phui-timeline-view.css' => '2d32d7a9', 'rsrc/css/phui/phui-two-column-view.css' => 'f96d319f', 'rsrc/css/phui/workboards/phui-workboard-color.css' => 'e86de308', 'rsrc/css/phui/workboards/phui-workboard.css' => '74fc9d98', @@ -508,7 +508,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' => '3972dadb', + 'rsrc/js/core/behavior-watch-anchor.js' => 'e55a1db5', 'rsrc/js/core/behavior-workflow.js' => '9623adc1', 'rsrc/js/core/darkconsole/DarkLog.js' => '3b869402', 'rsrc/js/core/darkconsole/DarkMessage.js' => '26cd4b73', @@ -658,7 +658,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' => '3972dadb', + 'javelin-behavior-phabricator-watch-anchor' => 'e55a1db5', 'javelin-behavior-pholio-mock-edit' => '3eed1f2b', 'javelin-behavior-pholio-mock-view' => '5aa1544e', 'javelin-behavior-phui-dropdown-menu' => '5cf0501a', @@ -878,7 +878,7 @@ return array( 'phui-status-list-view-css' => 'e5ff8be0', 'phui-tag-view-css' => '8519160a', 'phui-theme-css' => '35883b37', - 'phui-timeline-view-css' => '1e348e4b', + 'phui-timeline-view-css' => '2d32d7a9', 'phui-two-column-view-css' => 'f96d319f', 'phui-workboard-color-css' => 'e86de308', 'phui-workboard-view-css' => '74fc9d98', @@ -1227,12 +1227,6 @@ return array( 'javelin-install', 'javelin-dom', ), - '3972dadb' => array( - 'javelin-behavior', - 'javelin-stratcom', - 'javelin-dom', - 'javelin-vector', - ), '398fdf13' => array( 'javelin-behavior', 'trigger-rule-editor', @@ -2128,6 +2122,12 @@ return array( 'javelin-dom', 'phuix-dropdown-menu', ), + 'e55a1db5' => array( + 'javelin-behavior', + 'javelin-stratcom', + 'javelin-dom', + 'javelin-vector', + ), 'e5bdb730' => array( 'javelin-behavior', 'javelin-stratcom', diff --git a/src/view/phui/PHUITimelineEventView.php b/src/view/phui/PHUITimelineEventView.php index e4f11b2b1d..83c1889ed7 100644 --- a/src/view/phui/PHUITimelineEventView.php +++ b/src/view/phui/PHUITimelineEventView.php @@ -516,10 +516,10 @@ final class PHUITimelineEventView extends AphrontView { $outer_classes[] = 'phui-timeline-'.$color; } - $sigil = null; + $sigils = array(); $meta = null; if ($this->getTransactionPHID()) { - $sigil = 'transaction'; + $sigils[] = 'transaction'; $meta = array( 'phid' => $this->getTransactionPHID(), 'anchor' => $this->anchor, @@ -534,17 +534,17 @@ final class PHUITimelineEventView extends AphrontView { 'class' => 'phui-timeline-event-view '. 'phui-timeline-spacer '. 'phui-timeline-spacer-bold', - '', )); } + $sigils[] = 'anchor-container'; + return array( javelin_tag( 'div', array( 'class' => implode(' ', $outer_classes), - 'id' => $this->anchor ? 'anchor-'.$this->anchor : null, - 'sigil' => $sigil, + 'sigil' => implode(' ', $sigils), 'meta' => $meta, ), phutil_tag( diff --git a/webroot/rsrc/css/phui/phui-timeline-view.css b/webroot/rsrc/css/phui/phui-timeline-view.css index 6fae6802be..e08373d589 100644 --- a/webroot/rsrc/css/phui/phui-timeline-view.css +++ b/webroot/rsrc/css/phui/phui-timeline-view.css @@ -329,6 +329,9 @@ .phui-timeline-shell.anchor-target { background: {$lightyellow}; + border-width: 0 1px 0 0; + border-style: solid; + border-color: {$yellow}; padding: 4px; margin: -4px; } diff --git a/webroot/rsrc/js/core/behavior-watch-anchor.js b/webroot/rsrc/js/core/behavior-watch-anchor.js index 4f8c6b93b4..2fb84a0d10 100644 --- a/webroot/rsrc/js/core/behavior-watch-anchor.js +++ b/webroot/rsrc/js/core/behavior-watch-anchor.js @@ -23,6 +23,7 @@ JX.behavior('phabricator-watch-anchor', function() { var wait_ms = 100; var target; + var display_target; var retry_ms; function try_anchor() { @@ -74,8 +75,9 @@ JX.behavior('phabricator-watch-anchor', function() { // 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); + JX.DOM.alterClass(display_target, 'anchor-target', false); target = null; + display_target = null; } // If we didn't find a matching anchor, try again soon. This allows @@ -92,7 +94,18 @@ JX.behavior('phabricator-watch-anchor', function() { // If we've found a new target, highlight it. if (target !== node) { target = node; - JX.DOM.alterClass(target, 'anchor-target', true); + + // If there's an "anchor-container" parent element, we'll make the + // display adjustment to that node instead. For example, this is used + // by the timeline to highlight timeline stories. + var container = JX.DOM.findAbove(node, null, 'anchor-container'); + if (container) { + display_target = container; + } else { + display_target = node; + } + + JX.DOM.alterClass(display_target, 'anchor-target', true); } // Try to scroll to the new target.