Index: resources/celerity/map.php =================================================================== --- resources/celerity/map.php +++ resources/celerity/map.php @@ -10,7 +10,7 @@ 'core.pkg.css' => '607946ba', 'core.pkg.js' => 'c7854cc5', 'darkconsole.pkg.js' => 'ca8671ce', - 'differential.pkg.css' => '5a65a762', + 'differential.pkg.css' => '6aef439e', 'differential.pkg.js' => '322ea941', 'diffusion.pkg.css' => '3783278d', 'diffusion.pkg.js' => '7b51e80a', @@ -60,7 +60,6 @@ 'rsrc/css/application/differential/core.css' => '8135cb0c', 'rsrc/css/application/differential/local-commits-view.css' => '19649019', 'rsrc/css/application/differential/results-table.css' => '239924f9', - 'rsrc/css/application/differential/revision-comment-list.css' => 'bc291c47', 'rsrc/css/application/differential/revision-comment.css' => '48186045', 'rsrc/css/application/differential/revision-history.css' => 'f37aee8f', 'rsrc/css/application/differential/revision-list.css' => 'f3c47d33', @@ -144,7 +143,7 @@ 'rsrc/css/phui/phui-status.css' => '2f562399', 'rsrc/css/phui/phui-tag-view.css' => '295d81c4', 'rsrc/css/phui/phui-text.css' => '23e9b4b7', - 'rsrc/css/phui/phui-timeline-view.css' => '6c5e2da9', + 'rsrc/css/phui/phui-timeline-view.css' => 'd3ccba00', 'rsrc/css/phui/phui-workboard-view.css' => 'bf70dd2e', 'rsrc/css/phui/phui-workpanel-view.css' => '6f8527f6', 'rsrc/css/sprite-actions.css' => '4557baf8', @@ -359,7 +358,7 @@ 'rsrc/js/application/differential/behavior-edit-inline-comments.js' => '93f43142', 'rsrc/js/application/differential/behavior-keyboard-nav.js' => 'da3e88f9', 'rsrc/js/application/differential/behavior-populate.js' => 'ce0c217a', - 'rsrc/js/application/differential/behavior-show-all-comments.js' => '784b8218', + 'rsrc/js/application/differential/behavior-show-all-comments.js' => '7c273581', 'rsrc/js/application/differential/behavior-show-field-details.js' => '441f2137', 'rsrc/js/application/differential/behavior-show-more.js' => 'dd7e8ef5', 'rsrc/js/application/differential/behavior-toggle-files.js' => 'ca3f91eb', @@ -508,7 +507,6 @@ 'differential-results-table-css' => '239924f9', 'differential-revision-add-comment-css' => 'c478bcaa', 'differential-revision-comment-css' => '48186045', - 'differential-revision-comment-list-css' => 'bc291c47', 'differential-revision-history-css' => 'f37aee8f', 'differential-revision-list-css' => 'f3c47d33', 'differential-table-of-contents-css' => '19566f76', @@ -549,7 +547,6 @@ 'javelin-behavior-differential-feedback-preview' => '127f2018', 'javelin-behavior-differential-keyboard-navigation' => 'da3e88f9', 'javelin-behavior-differential-populate' => 'ce0c217a', - 'javelin-behavior-differential-show-all-comments' => '784b8218', 'javelin-behavior-differential-show-field-details' => '441f2137', 'javelin-behavior-differential-show-more' => 'dd7e8ef5', 'javelin-behavior-differential-toggle-files' => 'ca3f91eb', @@ -598,6 +595,7 @@ 'javelin-behavior-phabricator-remarkup-assist' => 'c021950a', 'javelin-behavior-phabricator-reveal-content' => '8f24abfc', 'javelin-behavior-phabricator-search-typeahead' => 'f6b56f7a', + 'javelin-behavior-phabricator-show-all-transactions' => '7c273581', 'javelin-behavior-phabricator-tooltips' => 'e5dd1c6d', 'javelin-behavior-phabricator-transaction-comment-form' => '9084a36f', 'javelin-behavior-phabricator-transaction-list' => 'bfb45968', @@ -756,7 +754,7 @@ 'phui-status-list-view-css' => '2f562399', 'phui-tag-view-css' => '295d81c4', 'phui-text-css' => '23e9b4b7', - 'phui-timeline-view-css' => '6c5e2da9', + 'phui-timeline-view-css' => 'd3ccba00', 'phui-workboard-view-css' => 'bf70dd2e', 'phui-workpanel-view-css' => '6f8527f6', 'policy-css' => '957ea14c', @@ -1235,12 +1233,6 @@ 0 => 'javelin-install', 1 => 'javelin-util', ), - '784b8218' => - array( - 0 => 'javelin-behavior', - 1 => 'javelin-stratcom', - 2 => 'javelin-dom', - ), '79473b62' => array( 0 => 'javelin-install', @@ -1256,6 +1248,12 @@ 1 => 'javelin-dom', 2 => 'javelin-reactor-dom', ), + '7c273581' => + array( + 0 => 'javelin-behavior', + 1 => 'javelin-stratcom', + 2 => 'javelin-dom', + ), '7e41274a' => array( 0 => 'javelin-install', @@ -2114,11 +2112,10 @@ 5 => 'differential-table-of-contents-css', 6 => 'differential-revision-comment-css', 7 => 'differential-revision-add-comment-css', - 8 => 'differential-revision-comment-list-css', - 9 => 'phabricator-object-selector-css', - 10 => 'phabricator-content-source-view-css', - 11 => 'differential-local-commits-view-css', - 12 => 'inline-comment-summary-css', + 8 => 'phabricator-object-selector-css', + 9 => 'phabricator-content-source-view-css', + 10 => 'differential-local-commits-view-css', + 11 => 'inline-comment-summary-css', ), 'differential.pkg.js' => array( Index: resources/celerity/packages.php =================================================================== --- resources/celerity/packages.php +++ resources/celerity/packages.php @@ -124,7 +124,6 @@ 'differential-table-of-contents-css', 'differential-revision-comment-css', 'differential-revision-add-comment-css', - 'differential-revision-comment-list-css', 'phabricator-object-selector-css', 'phabricator-content-source-view-css', 'differential-local-commits-view-css', Index: src/applications/transactions/view/PhabricatorApplicationTransactionView.php =================================================================== --- src/applications/transactions/view/PhabricatorApplicationTransactionView.php +++ src/applications/transactions/view/PhabricatorApplicationTransactionView.php @@ -51,7 +51,7 @@ return $this; } - public function buildEvents() { + public function buildEvents($with_hiding = false) { $user = $this->getUser(); $anchor = $this->anchorOffset; @@ -62,11 +62,50 @@ $xactions = $this->groupRelatedTransactions($xactions); $groups = $this->groupDisplayTransactions($xactions); + // If the viewer has interacted with this object, we hide things from + // before their most recent interaction by default. This tends to make + // very long threads much more manageable, because you don't have to + // scroll through a lot of history and can focus on just new stuff. + + $show_group = null; + + if ($with_hiding) { + // Find the most recent comment by the viewer. + $group_keys = array_keys($groups); + $group_keys = array_reverse($group_keys); + + // If we would only hide a small number of transactions, don't hide + // anything. Just don't examine the last few keys. Also, we always + // want to show the most recent pieces of activity, so don't examine + // the first few keys either. + $group_keys = array_slice($group_keys, 2, -2); + + $type_comment = PhabricatorTransactions::TYPE_COMMENT; + foreach ($group_keys as $group_key) { + $group = $groups[$group_key]; + foreach ($group as $xaction) { + if ($xaction->getAuthorPHID() == $user->getPHID() && + $xaction->getTransactionType() == $type_comment) { + // This is the most recent group where the user commented. + $show_group = $group_key; + break 2; + } + } + } + } + $events = array(); - foreach ($groups as $group) { + $hide_by_default = ($show_group !== null); + + foreach ($groups as $group_key => $group) { + if ($hide_by_default && ($show_group === $group_key)) { + $hide_by_default = false; + } + $group_event = null; foreach ($group as $xaction) { $event = $this->renderEvent($xaction, $group, $anchor); + $event->setHideByDefault($hide_by_default); $anchor++; if (!$group_event) { $group_event = $event; @@ -75,6 +114,7 @@ } } $events[] = $group_event; + } return $events; @@ -86,7 +126,7 @@ } $view = new PHUITimelineView(); - $events = $this->buildEvents(); + $events = $this->buildEvents($with_hiding = true); foreach ($events as $event) { $view->addEvent($event); } Index: src/infrastructure/internationalization/translation/PhabricatorBaseEnglishTranslation.php =================================================================== --- src/infrastructure/internationalization/translation/PhabricatorBaseEnglishTranslation.php +++ src/infrastructure/internationalization/translation/PhabricatorBaseEnglishTranslation.php @@ -826,6 +826,11 @@ 'users will need to sign again. Proceed carefully.', ), + '%s older changes(s) are hidden.' => array( + '%d older change is hidden.', + '%d older changes are hidden.', + ), + ); } Index: src/view/phui/PHUITimelineEventView.php =================================================================== --- src/view/phui/PHUITimelineEventView.php +++ src/view/phui/PHUITimelineEventView.php @@ -15,6 +15,16 @@ private $transactionPHID; private $isPreview; private $eventGroup = array(); + private $hideByDefault; + + public function setHideByDefault($hide_by_default) { + $this->hideByDefault = $hide_by_default; + return $this; + } + + public function getHideByDefault() { + return $this->hideByDefault; + } public function setTransactionPHID($transaction_phid) { $this->transactionPHID = $transaction_phid; @@ -80,6 +90,10 @@ return $this; } + public function getAnchor() { + return $this->anchor; + } + public function setTitle($title) { $this->title = $title; return $this; Index: src/view/phui/PHUITimelineView.php =================================================================== --- src/view/phui/PHUITimelineView.php +++ src/view/phui/PHUITimelineView.php @@ -20,12 +20,74 @@ $spacer = self::renderSpacer(); - $events = array(); + $hide = array(); + $show = array(); + foreach ($this->events as $event) { + if ($event->getHideByDefault()) { + $hide[] = $event; + } else { + $show[] = $event; + } + } + + $events = array(); + if ($hide) { + $hidden = phutil_implode_html($spacer, $hide); + $count = count($hide); + + $show_id = celerity_generate_unique_node_id(); + $hide_id = celerity_generate_unique_node_id(); + $link_id = celerity_generate_unique_node_id(); + + Javelin::initBehavior( + 'phabricator-show-all-transactions', + array( + 'anchors' => array_filter(mpull($hide, 'getAnchor')), + 'linkID' => $link_id, + 'hideID' => $hide_id, + 'showID' => $show_id, + )); + + $events[] = phutil_tag( + 'div', + array( + 'id' => $hide_id, + 'class' => 'phui-timeline-older-transactions-are-hidden', + ), + array( + pht('%s older changes(s) are hidden.', new PhutilNumber($count)), + ' ', + javelin_tag( + 'a', + array( + 'href' => '#', + 'mustcapture' => true, + 'id' => $link_id, + ), + pht('Show all changes.')), + )); + + $events[] = phutil_tag( + 'div', + array( + 'id' => $show_id, + 'style' => 'display: none', + ), + $hidden); + } + + if ($hide && $show) { $events[] = $spacer; - $events[] = $event; } - $events[] = $spacer; + + if ($show) { + $events[] = phutil_implode_html($spacer, $show); + } + + if ($events) { + $events = array($spacer, $events, $spacer); + } return phutil_tag( 'div', Index: webroot/rsrc/css/application/differential/revision-comment-list.css =================================================================== --- webroot/rsrc/css/application/differential/revision-comment-list.css +++ /dev/null @@ -1,12 +0,0 @@ -/** - * @provides differential-revision-comment-list-css - */ - -.differential-older-comments-are-hidden { - background: {$lightyellow}; - border: 1px solid {$yellow}; - text-align: center; - padding: 12px; - color: {$darkgreytext}; - margin-top: 16px; -} Index: webroot/rsrc/css/phui/phui-timeline-view.css =================================================================== --- webroot/rsrc/css/phui/phui-timeline-view.css +++ webroot/rsrc/css/phui/phui-timeline-view.css @@ -260,3 +260,12 @@ border-color: #efefef; border-width: 1px 0; } + +.phui-timeline-older-transactions-are-hidden { + background: {$lightyellow}; + border: 1px solid {$yellow}; + text-align: center; + padding: 12px; + color: {$darkgreytext}; + margin-top: 16px; +} Index: webroot/rsrc/js/application/differential/behavior-show-all-comments.js =================================================================== --- webroot/rsrc/js/application/differential/behavior-show-all-comments.js +++ webroot/rsrc/js/application/differential/behavior-show-all-comments.js @@ -1,59 +1,66 @@ /** - * @provides javelin-behavior-differential-show-all-comments + * @provides javelin-behavior-phabricator-show-all-transactions * @requires javelin-behavior * javelin-stratcom * javelin-dom */ -JX.behavior('differential-show-all-comments', function(config) { +/** + * Automatically show older transactions if the user follows an anchor to a + * transaction which is hidden by the "N older changes are hidden." shield. + */ +JX.behavior('phabricator-show-all-transactions', function(config) { - var shown = false; - function reveal(node) { - if (shown) { - return false; - } - shown = true; - node = node || JX.DOM.find( - document.body, - 'div', - 'differential-all-comments-container'); - if (node) { - JX.DOM.setContent(node, JX.$H(config.markup)); + var revealed = false; + + function get_hash() { + return window.location.hash.replace(/^#/, ''); + } + + function hash_is_hidden() { + var hash = get_hash(); + for (var ii = 0; ii < config.anchors.length; ii++) { + if (config.anchors[ii] == hash) { + return true; + } } - return true; + return false; } - // Reveal the hidden comments if the user clicks "Show All Comments", or if - // there's an anchor in the URL, since we don't want to link to "#comment-3" - // and have it collapsed. + function reveal() { + if (revealed) { + return false; + } + + JX.DOM.hide(JX.$(config.hideID)); + JX.DOM.show(JX.$(config.showID)); + revealed = true; - function at_comment_hash() { - return window.location.hash && window.location.hash.match(/comment/); + return true; } - if (at_comment_hash()) { - reveal(); - } else { - JX.Stratcom.listen( - 'hashchange', - null, - function(e) { - if (at_comment_hash() && reveal()) { - try { - var target = JX.$(window.location.hash.replace(/^#/, '')); - window.scrollTo(0, target.offsetTop); - } catch (ex) { - } + function check_hash() { + if (hash_is_hidden()) { + if (reveal()) { + try { + var target = JX.$(get_hash()); + JX.DOM.scrollTo(target); + } catch (ignored) { + // We did our best. } - }); + } + } } - JX.Stratcom.listen( + JX.DOM.listen( + JX.$(config.linkID), 'click', - 'differential-show-all-comments', - function(e) { - reveal(e.getNode('differential-all-comments-container')); + null, + function (e) { e.kill(); + reveal(); }); + JX.Stratcom.listen('hashchange', null, check_hash); + check_hash(); });