mirror of
https://we.phorge.it/source/phorge.git
synced 2025-01-15 01:01:09 +01:00
Partially improve threading UI for adjacent inline comments
Summary: Ref T10563. This isn't a complete fix, but should make viewing complex inline threads a little more manageable. This just tries to put stuff in thread order instead of in pure chronological order. We can likely improve the display treatment -- this is a pretty minimal approach, but should improve clarity. Test Plan: T10563 has a "before" shot. Here's the "after": {F1169018} This makes it a bit easier to follow the conversations. Reviewers: chad Reviewed By: chad Maniphest Tasks: T10563 Differential Revision: https://secure.phabricator.com/D15459
This commit is contained in:
parent
99bc1b05d7
commit
68b468a846
4 changed files with 70 additions and 6 deletions
|
@ -10,7 +10,7 @@ return array(
|
||||||
'core.pkg.css' => '9c8e888d',
|
'core.pkg.css' => '9c8e888d',
|
||||||
'core.pkg.js' => '7d8faf57',
|
'core.pkg.js' => '7d8faf57',
|
||||||
'darkconsole.pkg.js' => 'e7393ebb',
|
'darkconsole.pkg.js' => 'e7393ebb',
|
||||||
'differential.pkg.css' => '2de124c9',
|
'differential.pkg.css' => '7d0a63a7',
|
||||||
'differential.pkg.js' => 'd0cd0df6',
|
'differential.pkg.js' => 'd0cd0df6',
|
||||||
'diffusion.pkg.css' => 'f45955ed',
|
'diffusion.pkg.css' => 'f45955ed',
|
||||||
'diffusion.pkg.js' => '3a9a8bfa',
|
'diffusion.pkg.js' => '3a9a8bfa',
|
||||||
|
@ -59,7 +59,7 @@ return array(
|
||||||
'rsrc/css/application/differential/add-comment.css' => 'c47f8c40',
|
'rsrc/css/application/differential/add-comment.css' => 'c47f8c40',
|
||||||
'rsrc/css/application/differential/changeset-view.css' => 'b6b0d1bb',
|
'rsrc/css/application/differential/changeset-view.css' => 'b6b0d1bb',
|
||||||
'rsrc/css/application/differential/core.css' => '7ac3cabc',
|
'rsrc/css/application/differential/core.css' => '7ac3cabc',
|
||||||
'rsrc/css/application/differential/phui-inline-comment.css' => '0fdb3667',
|
'rsrc/css/application/differential/phui-inline-comment.css' => '5953c28e',
|
||||||
'rsrc/css/application/differential/revision-comment.css' => '14b8565a',
|
'rsrc/css/application/differential/revision-comment.css' => '14b8565a',
|
||||||
'rsrc/css/application/differential/revision-history.css' => '0e8eb855',
|
'rsrc/css/application/differential/revision-history.css' => '0e8eb855',
|
||||||
'rsrc/css/application/differential/revision-list.css' => 'f3c47d33',
|
'rsrc/css/application/differential/revision-list.css' => 'f3c47d33',
|
||||||
|
@ -831,7 +831,7 @@ return array(
|
||||||
'phui-image-mask-css' => 'a8498f9c',
|
'phui-image-mask-css' => 'a8498f9c',
|
||||||
'phui-info-panel-css' => '27ea50a1',
|
'phui-info-panel-css' => '27ea50a1',
|
||||||
'phui-info-view-css' => '6d7c3509',
|
'phui-info-view-css' => '6d7c3509',
|
||||||
'phui-inline-comment-view-css' => '0fdb3667',
|
'phui-inline-comment-view-css' => '5953c28e',
|
||||||
'phui-list-view-css' => '9da2aa00',
|
'phui-list-view-css' => '9da2aa00',
|
||||||
'phui-object-box-css' => '91628842',
|
'phui-object-box-css' => '91628842',
|
||||||
'phui-object-item-list-view-css' => '18b2ce8e',
|
'phui-object-item-list-view-css' => '18b2ce8e',
|
||||||
|
|
|
@ -1000,7 +1000,8 @@ final class DifferentialChangesetParser extends Phobject {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
$this->comments = msort($this->comments, 'getID');
|
$this->comments = $this->reorderAndThreadComments($this->comments);
|
||||||
|
|
||||||
foreach ($this->comments as $comment) {
|
foreach ($this->comments as $comment) {
|
||||||
$final = $comment->getLineNumber() +
|
$final = $comment->getLineNumber() +
|
||||||
$comment->getLineLength();
|
$comment->getLineLength();
|
||||||
|
@ -1569,4 +1570,67 @@ final class DifferentialChangesetParser extends Phobject {
|
||||||
return array($old_back, $new_back);
|
return array($old_back, $new_back);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
private function reorderAndThreadComments(array $comments) {
|
||||||
|
$comments = msort($comments, 'getID');
|
||||||
|
|
||||||
|
// Build an empty map of all the comments we actually have. If a comment
|
||||||
|
// is a reply but the parent has gone missing, we don't want it to vanish
|
||||||
|
// completely.
|
||||||
|
$comment_phids = mpull($comments, 'getPHID');
|
||||||
|
$replies = array_fill_keys($comment_phids, array());
|
||||||
|
|
||||||
|
// Now, remove all comments which are replies, leaving only the top-level
|
||||||
|
// comments.
|
||||||
|
foreach ($comments as $key => $comment) {
|
||||||
|
$reply_phid = $comment->getReplyToCommentPHID();
|
||||||
|
if (isset($replies[$reply_phid])) {
|
||||||
|
$replies[$reply_phid][] = $comment;
|
||||||
|
unset($comments[$key]);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// For each top level comment, add the comment, then add any replies
|
||||||
|
// to it. Do this recursively so threads are shown in threaded order.
|
||||||
|
$results = array();
|
||||||
|
foreach ($comments as $comment) {
|
||||||
|
$results[] = $comment;
|
||||||
|
$phid = $comment->getPHID();
|
||||||
|
$descendants = $this->getInlineReplies($replies, $phid, 1);
|
||||||
|
foreach ($descendants as $descendant) {
|
||||||
|
$results[] = $descendant;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// If we have anything left, they were cyclic references. Just dump
|
||||||
|
// them in a the end. This should be impossible, but users are very
|
||||||
|
// creative.
|
||||||
|
foreach ($replies as $phid => $comments) {
|
||||||
|
foreach ($comments as $comment) {
|
||||||
|
$results[] = $comment;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
return $results;
|
||||||
|
}
|
||||||
|
|
||||||
|
private function getInlineReplies(array &$replies, $phid, $depth) {
|
||||||
|
$comments = idx($replies, $phid, array());
|
||||||
|
unset($replies[$phid]);
|
||||||
|
|
||||||
|
$results = array();
|
||||||
|
foreach ($comments as $comment) {
|
||||||
|
$results[] = $comment;
|
||||||
|
$descendants = $this->getInlineReplies(
|
||||||
|
$replies,
|
||||||
|
$comment->getPHID(),
|
||||||
|
$depth + 1);
|
||||||
|
foreach ($descendants as $descendant) {
|
||||||
|
$results[] = $descendant;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
return $results;
|
||||||
|
}
|
||||||
|
|
||||||
|
|
||||||
}
|
}
|
||||||
|
|
|
@ -176,7 +176,7 @@ final class PHUIDiffInlineCommentDetailView
|
||||||
if ($inline->getHasReplies()) {
|
if ($inline->getHasReplies()) {
|
||||||
$classes[] = 'inline-comment-has-reply';
|
$classes[] = 'inline-comment-has-reply';
|
||||||
}
|
}
|
||||||
// I think this is unused
|
|
||||||
if ($inline->getReplyToCommentPHID()) {
|
if ($inline->getReplyToCommentPHID()) {
|
||||||
$classes[] = 'inline-comment-is-reply';
|
$classes[] = 'inline-comment-is-reply';
|
||||||
}
|
}
|
||||||
|
|
|
@ -61,7 +61,7 @@
|
||||||
|
|
||||||
/* Tighten up spacing on replies */
|
/* Tighten up spacing on replies */
|
||||||
.differential-inline-comment.inline-comment-is-reply {
|
.differential-inline-comment.inline-comment-is-reply {
|
||||||
margin-top: -4px;
|
margin-top: -12px;
|
||||||
}
|
}
|
||||||
|
|
||||||
.differential-inline-comment .inline-head-right {
|
.differential-inline-comment .inline-head-right {
|
||||||
|
|
Loading…
Reference in a new issue