1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-11-10 08:52:39 +01:00

Clean up file browse view a bit

Summary:
  - Use DiffusionCommitQuery
  - Get rid of the "Author" column.
  - Collapse commit + revision together.
  - Better tooltips to cover for the removed information.
  - Colorize only the "line" column.
  - Generally, reduce the amount of visual noise and non-code-stuff going on in this interface.
  - I'd like to make the "<<" thing look nicer too but that might take some actual design.

Test Plan: See screenshots.

Reviewers: btrahan, chad

Reviewed By: chad

CC: chad, aran

Differential Revision: https://secure.phabricator.com/D7457
This commit is contained in:
epriestley 2013-10-30 13:15:26 -07:00
parent d51ae49f61
commit 93d7a1451b
3 changed files with 95 additions and 64 deletions

View file

@ -1154,7 +1154,7 @@ celerity_register_resource_map(array(
), ),
'diffusion-source-css' => 'diffusion-source-css' =>
array( array(
'uri' => '/res/f4a2f867/rsrc/css/application/diffusion/diffusion-source.css', 'uri' => '/res/5076c269/rsrc/css/application/diffusion/diffusion-source.css',
'type' => 'css', 'type' => 'css',
'requires' => 'requires' =>
array( array(

View file

@ -548,35 +548,49 @@ final class DiffusionBrowseFileController extends DiffusionBrowseController {
++$line_number; ++$line_number;
} }
$request = $this->getRequest();
$viewer = $request->getUser();
$commits = array_filter(ipull($display, 'commit')); $commits = array_filter(ipull($display, 'commit'));
if ($commits) { if ($commits) {
$commits = id(new PhabricatorAuditCommitQuery()) $commits = id(new DiffusionCommitQuery())
->withIdentifiers($drequest->getRepository()->getID(), $commits) ->setViewer($viewer)
->needCommitData(true) ->withRepositoryIDs(array($drequest->getRepository()->getID()))
->withIdentifiers($commits)
->execute(); ->execute();
$commits = mpull($commits, null, 'getCommitIdentifier'); $commits = mpull($commits, null, 'getCommitIdentifier');
} }
$request = $this->getRequest();
$user = $request->getUser();
$revision_ids = id(new DifferentialRevision()) $revision_ids = id(new DifferentialRevision())
->loadIDsByCommitPHIDs(mpull($commits, 'getPHID')); ->loadIDsByCommitPHIDs(mpull($commits, 'getPHID'));
$revisions = array(); $revisions = array();
if ($revision_ids) { if ($revision_ids) {
$revisions = id(new DifferentialRevisionQuery()) $revisions = id(new DifferentialRevisionQuery())
->setViewer($user) ->setViewer($viewer)
->withIDs($revision_ids) ->withIDs($revision_ids)
->execute(); ->execute();
} }
$phids = array();
foreach ($commits as $commit) {
if ($commit->getAuthorPHID()) {
$phids[] = $commit->getAuthorPHID();
}
}
foreach ($revisions as $revision) {
if ($revision->getAuthorPHID()) {
$phids[] = $revision->getAuthorPHID();
}
}
$handles = $this->loadViewerHandles($phids);
Javelin::initBehavior('phabricator-oncopy', array()); Javelin::initBehavior('phabricator-oncopy', array());
$engine = null; $engine = null;
$inlines = array(); $inlines = array();
if ($this->getRequest()->getStr('lint') !== null && $this->lintMessages) { if ($this->getRequest()->getStr('lint') !== null && $this->lintMessages) {
$engine = new PhabricatorMarkupEngine(); $engine = new PhabricatorMarkupEngine();
$engine->setViewer($user); $engine->setViewer($viewer);
foreach ($this->lintMessages as $message) { foreach ($this->lintMessages as $message) {
$inline = id(new PhabricatorAuditInlineComment()) $inline = id(new PhabricatorAuditInlineComment())
@ -624,15 +638,15 @@ final class DiffusionBrowseFileController extends DiffusionBrowseController {
if (idx($line, 'commit')) { if (idx($line, 'commit')) {
$commit = $line['commit']; $commit = $line['commit'];
$summary = 'Unknown';
if (idx($commits, $commit)) { if (idx($commits, $commit)) {
$summary = $commits[$commit]->getCommitData()->getSummary(); $tooltip = $this->renderCommitTooltip(
$commits[$commit],
$handles,
$line['author']);
} else {
$tooltip = null;
} }
$tooltip = phabricator_date(
$line['epoch'],
$user)." \xC2\xB7 ".$summary;
Javelin::initBehavior('phabricator-tooltips', array()); Javelin::initBehavior('phabricator-tooltips', array());
require_celerity_resource('aphront-tooltip-css'); require_celerity_resource('aphront-tooltip-css');
@ -660,18 +674,12 @@ final class DiffusionBrowseFileController extends DiffusionBrowseController {
if ($revision_id) { if ($revision_id) {
$revision = idx($revisions, $revision_id); $revision = idx($revisions, $revision_id);
if (!$revision) { if ($revision) {
$tooltip = pht('(Invalid revision)'); $tooltip = $this->renderRevisionTooltip($revision, $handles);
} else {
$tooltip =
phabricator_date($revision->getDateModified(), $user).
" \xC2\xB7 ".
$revision->getTitle();
}
$revision_link = javelin_tag( $revision_link = javelin_tag(
'a', 'a',
array( array(
'href' => '/D'.$revision_id, 'href' => '/D'.$revision->getID(),
'sigil' => 'has-tooltip', 'sigil' => 'has-tooltip',
'meta' => array( 'meta' => array(
'tip' => $tooltip, 'tip' => $tooltip,
@ -679,7 +687,8 @@ final class DiffusionBrowseFileController extends DiffusionBrowseController {
'size' => 600, 'size' => 600,
), ),
), ),
'D'.$revision_id); 'D'.$revision->getID());
}
} }
$uri = $line_href->alter('before', $commit); $uri = $line_href->alter('before', $commit);
@ -701,39 +710,29 @@ final class DiffusionBrowseFileController extends DiffusionBrowseController {
'th', 'th',
array( array(
'class' => 'diffusion-blame-link', 'class' => 'diffusion-blame-link',
'style' => $style,
), ),
$before_link); $before_link);
$blame[] = phutil_tag( $object_links = array();
'th', $object_links[] = $commit_link;
array( if ($revision_link) {
'class' => 'diffusion-rev-link', $object_links[] = phutil_tag('span', array(), '/');
'style' => $style, $object_links[] = $revision_link;
), }
$commit_link);
$blame[] = phutil_tag( $blame[] = phutil_tag(
'th', 'th',
array( array(
'class' => 'diffusion-rev-link', 'class' => 'diffusion-rev-link',
'style' => $style,
), ),
$revision_link); $object_links);
$blame[] = phutil_tag(
'th',
array(
'class' => 'diffusion-author-link',
'style' => $style,
),
idx($line, 'author'));
} }
$line_link = phutil_tag( $line_link = phutil_tag(
'a', 'a',
array( array(
'href' => $line_href, 'href' => $line_href,
'style' => $style,
), ),
$line['line']); $line['line']);
@ -977,4 +976,36 @@ final class DiffusionBrowseFileController extends DiffusionBrowseController {
return head($parents); return head($parents);
} }
private function renderRevisionTooltip(
DifferentialRevision $revision,
array $handles) {
$viewer = $this->getRequest()->getUser();
$date = phabricator_date($revision->getDateModified(), $viewer);
$id = $revision->getID();
$title = $revision->getTitle();
$header = "D{$id} {$title}";
$author = $handles[$revision->getAuthorPHID()]->getName();
return "{$header}\n{$date} \xC2\xB7 {$author}";
}
private function renderCommitTooltip(
PhabricatorRepositoryCommit $commit,
array $handles,
$author) {
$viewer = $this->getRequest()->getUser();
$date = phabricator_date($commit->getEpoch(), $viewer);
$summary = trim($commit->getSummary());
if ($commit->getAuthorPHID()) {
$author = $handles[$commit->getAuthorPHID()]->getName();
}
return "{$summary}\n{$date} \xC2\xB7 {$author}";
}
} }

View file

@ -37,8 +37,7 @@
} }
.diffusion-blame-link, .diffusion-blame-link,
.diffusion-rev-link, .diffusion-rev-link {
.diffusion-author-link {
white-space: nowrap; white-space: nowrap;
} }
@ -46,26 +45,27 @@
min-width: 28px; min-width: 28px;
} }
.diffusion-rev-link { .diffusion-source th.diffusion-rev-link {
min-width: 90px; text-align: left;
} min-width: 130px;
.diffusion-author-link {
min-width: 120px;
} }
.diffusion-blame-link a, .diffusion-blame-link a,
.diffusion-rev-link a, .diffusion-rev-link a,
.diffusion-author-link a,
.diffusion-line-link a { .diffusion-line-link a {
color: {$darkbluetext}; color: {$darkbluetext};
} }
.diffusion-rev-link a, .diffusion-rev-link a,
.diffusion-author-link span, .diffusion-rev-link span {
.diffusion-author-link a {
margin: 2px 8px 0; margin: 2px 8px 0;
display: block; display: inline-block;
}
.diffusion-rev-link span {
margin-right: -4px;
margin-left: -4px;
color: {$lightgreytext};
} }
.diffusion-blame-link a, .diffusion-blame-link a,