revi-archive/phorge-phorge
1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-11-10 00:42:41 +01:00
Code Issues Releases Wiki Activity

Hide seen transactions by default in all modern applications

Browse source 
Summary:
Ref T2222. This restores the "N older comments are hidden." shield to all ApplicationTransactions applications. Roughly the rule this uses is that transactions older than your most recent comment are hidden, under the assumption that you've already read and dealt with them, since you replied afterward. Then we show your last comment to remind/contextualize you, and anything afterward. We also don't hide transactions if we'd only be hiding a handfull, and we never hide the few most recent transactions.

This might need some #design help.

Test Plan:
The tricky part here is the anchor rule, which deals with the case where you follow a link to `T123#4`, but that would normally be hidden. We simulate a click on "show all" if you hit an anchor which is hidden. Here's what it looks like in Maniphest:

{F112891}

Reviewers: btrahan, chad

Reviewed By: btrahan

CC: chad, aran

Maniphest Tasks: T2222

Differential Revision: https://secure.phabricator.com/D8229
This commit is contained in:
epriestley 2014-02-14 10:23:07 -08:00
parent 390a0f91b0
commit 7a96de10f0
9 changed files with 198 additions and 77 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

33
resources/celerity/map.php
View file

@ -10,7 +10,7 @@ return array(
'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 @@ return array(
'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 @@ return array(
'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 @@ return array(
'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 @@ return array(
'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 @@ return array(
'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 @@ return array(
'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 @@ return array(
'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 @@ return array(
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 @@ return array(
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 @@ return array(
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(

1
resources/celerity/packages.php
View file

@ -124,7 +124,6 @@ return array(
'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',

46
src/applications/transactions/view/PhabricatorApplicationTransactionView.php
View file

@ -51,7 +51,7 @@ class PhabricatorApplicationTransactionView extends AphrontView {
return $this;
}
public function buildEvents() {
public function buildEvents($with_hiding = false) {
$user = $this->getUser();
$anchor = $this->anchorOffset;
@ -62,11 +62,50 @@ class PhabricatorApplicationTransactionView extends AphrontView {
$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 @@ class PhabricatorApplicationTransactionView extends AphrontView {
}
}
$events[] = $group_event;
}
return $events;
@ -86,7 +126,7 @@ class PhabricatorApplicationTransactionView extends AphrontView {
}
$view = new PHUITimelineView();
$events = $this->buildEvents();
$events = $this->buildEvents($with_hiding = true);
foreach ($events as $event) {
$view->addEvent($event);
}

5
src/infrastructure/internationalization/translation/PhabricatorBaseEnglishTranslation.php
View file

@ -826,6 +826,11 @@ abstract class PhabricatorBaseEnglishTranslation
'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.',
),
);
}

14
src/view/phui/PHUITimelineEventView.php
View file

@ -15,6 +15,16 @@ final class PHUITimelineEventView extends AphrontView {
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 @@ final class PHUITimelineEventView extends AphrontView {
return $this;
}
public function getAnchor() {
return $this->anchor;
}
public function setTitle($title) {
$this->title = $title;
return $this;

70
src/view/phui/PHUITimelineView.php
View file

@ -20,12 +20,74 @@ final class PHUITimelineView extends AphrontView {
$spacer = self::renderSpacer();
$events = array();
$hide = array();
$show = array();
foreach ($this->events as $event) {
$events[] = $spacer;
$events[] = $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;
}
if ($show) {
$events[] = phutil_implode_html($spacer, $show);
}
if ($events) {
$events = array($spacer, $events, $spacer);
}
$events[] = $spacer;
return phutil_tag(
'div',

12
webroot/rsrc/css/application/differential/revision-comment-list.css
View file

@ -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;
}

9
webroot/rsrc/css/phui/phui-timeline-view.css
View file

@ -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;
}

85
webroot/rsrc/js/application/differential/behavior-show-all-comments.js
View file

@ -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) {
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 false;
}
function reveal() {
if (revealed) {
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));
}
JX.DOM.hide(JX.$(config.hideID));
JX.DOM.show(JX.$(config.showID));
revealed = true;
return true;
}
// 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 at_comment_hash() {
return window.location.hash && window.location.hash.match(/comment/);
}
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();
});