1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-12-18 11:30:55 +01:00

Clean up browse/history links in Diffusion

Summary:
Fixes T9126. In particular:

  - Add "Browse" links to all history views.
  - Use icons to show "Browse" and "History" links, instead of text.
  - Use FontAwesome.
  - Generally standardize handling of these elements.

This might need a little design attention, but I think it's an improvement overall.

Test Plan:
  - Viewed repository history.
  - Viewed branch history.
  - Viewed file history.
  - Viewed table of contents on a commit.
  - Viewed merged changes on a merge commit.
  - Viewed a directory containing an external.
  - Viewed a deleted file.

{F788419}

{F788420}

{F788421}

{F788422}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T9126

Differential Revision: https://secure.phabricator.com/D14096
This commit is contained in:
epriestley 2015-09-10 19:28:49 -07:00
parent 1583738842
commit 4e181a5611
16 changed files with 186 additions and 176 deletions

View file

@ -7,12 +7,12 @@
*/
return array(
'names' => array(
'core.pkg.css' => '3fcfaed8',
'core.pkg.css' => 'a2cf2f6c',
'core.pkg.js' => '47dc9ebb',
'darkconsole.pkg.js' => 'e7393ebb',
'differential.pkg.css' => '2de124c9',
'differential.pkg.js' => '52d725be',
'diffusion.pkg.css' => '385e85b3',
'differential.pkg.js' => '6223dd9d',
'diffusion.pkg.css' => 'f45955ed',
'diffusion.pkg.js' => '0115b37c',
'maniphest.pkg.css' => '4845691a',
'maniphest.pkg.js' => '3ec6a6d5',
@ -25,7 +25,7 @@ return array(
'rsrc/css/aphront/notification.css' => '9c279160',
'rsrc/css/aphront/panel-view.css' => '8427b78d',
'rsrc/css/aphront/phabricator-nav-view.css' => 'a24cb589',
'rsrc/css/aphront/table-view.css' => 'e3632cc9',
'rsrc/css/aphront/table-view.css' => '34ee903e',
'rsrc/css/aphront/tokenizer.css' => '04875312',
'rsrc/css/aphront/tooltip.css' => '7672b60f',
'rsrc/css/aphront/typeahead-browse.css' => 'd8581d2c',
@ -65,7 +65,7 @@ return array(
'rsrc/css/application/differential/revision-history.css' => '0e8eb855',
'rsrc/css/application/differential/revision-list.css' => 'f3c47d33',
'rsrc/css/application/differential/table-of-contents.css' => 'ae4b7a55',
'rsrc/css/application/diffusion/diffusion-icons.css' => '4ba18923',
'rsrc/css/application/diffusion/diffusion-icons.css' => '2941baf1',
'rsrc/css/application/diffusion/diffusion-readme.css' => '2106ea08',
'rsrc/css/application/diffusion/diffusion-source.css' => '66fdf661',
'rsrc/css/application/feed/feed.css' => 'ecd4ec57',
@ -99,7 +99,7 @@ return array(
'rsrc/css/application/releeph/releeph-preview-branch.css' => 'b7a6f4a5',
'rsrc/css/application/releeph/releeph-request-differential-create-dialog.css' => '8d8b92cd',
'rsrc/css/application/releeph/releeph-request-typeahead.css' => '667a48ae',
'rsrc/css/application/search/search-results.css' => '586db3a4',
'rsrc/css/application/search/search-results.css' => '7dea472c',
'rsrc/css/application/slowvote/slowvote.css' => '475b4bd2',
'rsrc/css/application/tokens/tokens.css' => '3d0f239e',
'rsrc/css/application/uiexample/example.css' => '528b19de',
@ -276,14 +276,10 @@ return array(
'rsrc/image/icon/fatcow/flag_purple.png' => 'cc517522',
'rsrc/image/icon/fatcow/flag_red.png' => '04ec726f',
'rsrc/image/icon/fatcow/flag_yellow.png' => '73946fd4',
'rsrc/image/icon/fatcow/folder.png' => '95a435af',
'rsrc/image/icon/fatcow/folder_go.png' => '001cbc94',
'rsrc/image/icon/fatcow/key_question.png' => '52a0c26a',
'rsrc/image/icon/fatcow/link.png' => '7afd4d5e',
'rsrc/image/icon/fatcow/page_white_edit.png' => '39a2eed8',
'rsrc/image/icon/fatcow/page_white_link.png' => 'a90023c7',
'rsrc/image/icon/fatcow/page_white_put.png' => '08c95a0c',
'rsrc/image/icon/fatcow/page_white_text.png' => '1e1f79c3',
'rsrc/image/icon/fatcow/source/conduit.png' => '4ea01d2f',
'rsrc/image/icon/fatcow/source/email.png' => '9bab3239',
'rsrc/image/icon/fatcow/source/fax.png' => '04195e68',
@ -357,13 +353,13 @@ return array(
'rsrc/js/application/dashboard/behavior-dashboard-query-panel-select.js' => '453c5375',
'rsrc/js/application/dashboard/behavior-dashboard-tab-panel.js' => 'd4eecc63',
'rsrc/js/application/differential/ChangesetViewManager.js' => '58562350',
'rsrc/js/application/differential/DifferentialInlineCommentEditor.js' => 'd4c87bf4',
'rsrc/js/application/differential/DifferentialInlineCommentEditor.js' => '64a5550f',
'rsrc/js/application/differential/behavior-add-reviewers-and-ccs.js' => 'e10f8e18',
'rsrc/js/application/differential/behavior-comment-jump.js' => '4fdb476d',
'rsrc/js/application/differential/behavior-comment-preview.js' => 'b064af76',
'rsrc/js/application/differential/behavior-diff-radios.js' => 'e1ff79b1',
'rsrc/js/application/differential/behavior-dropdown-menus.js' => '2035b9cb',
'rsrc/js/application/differential/behavior-edit-inline-comments.js' => '037b59eb',
'rsrc/js/application/differential/behavior-edit-inline-comments.js' => '65ef6074',
'rsrc/js/application/differential/behavior-keyboard-nav.js' => '2c426492',
'rsrc/js/application/differential/behavior-populate.js' => '8694b1df',
'rsrc/js/application/differential/behavior-toggle-files.js' => 'ca3f91eb',
@ -496,7 +492,7 @@ return array(
'aphront-list-filter-view-css' => '5d6f0526',
'aphront-multi-column-view-css' => 'fd18389d',
'aphront-panel-view-css' => '8427b78d',
'aphront-table-view-css' => 'e3632cc9',
'aphront-table-view-css' => '34ee903e',
'aphront-tokenizer-control-css' => '04875312',
'aphront-tooltip-css' => '7672b60f',
'aphront-typeahead-control-css' => '0e403212',
@ -517,13 +513,13 @@ return array(
'conpherence-widget-pane-css' => '775eaaba',
'differential-changeset-view-css' => 'b6b0d1bb',
'differential-core-view-css' => '7ac3cabc',
'differential-inline-comment-editor' => 'd4c87bf4',
'differential-inline-comment-editor' => '64a5550f',
'differential-revision-add-comment-css' => 'c47f8c40',
'differential-revision-comment-css' => '14b8565a',
'differential-revision-history-css' => '0e8eb855',
'differential-revision-list-css' => 'f3c47d33',
'differential-table-of-contents-css' => 'ae4b7a55',
'diffusion-icons-css' => '4ba18923',
'diffusion-icons-css' => '2941baf1',
'diffusion-readme-css' => '2106ea08',
'diffusion-source-css' => '66fdf661',
'diviner-shared-css' => '5a337049',
@ -568,7 +564,7 @@ return array(
'javelin-behavior-differential-comment-jump' => '4fdb476d',
'javelin-behavior-differential-diff-radios' => 'e1ff79b1',
'javelin-behavior-differential-dropdown-menus' => '2035b9cb',
'javelin-behavior-differential-edit-inline-comments' => '037b59eb',
'javelin-behavior-differential-edit-inline-comments' => '65ef6074',
'javelin-behavior-differential-feedback-preview' => 'b064af76',
'javelin-behavior-differential-keyboard-navigation' => '2c426492',
'javelin-behavior-differential-populate' => '8694b1df',
@ -738,7 +734,7 @@ return array(
'phabricator-phtize' => 'd254d646',
'phabricator-prefab' => '6920d200',
'phabricator-remarkup-css' => '1c4ac273',
'phabricator-search-results-css' => '586db3a4',
'phabricator-search-results-css' => '7dea472c',
'phabricator-shaped-request' => '7cbe244b',
'phabricator-side-menu-view-css' => 'bec2458e',
'phabricator-slowvote-css' => '475b4bd2',
@ -853,14 +849,6 @@ return array(
'javelin-behavior-device',
'phabricator-title',
),
'037b59eb' => array(
'javelin-behavior',
'javelin-stratcom',
'javelin-dom',
'javelin-util',
'javelin-vector',
'differential-inline-comment-editor',
),
'048330fa' => array(
'javelin-behavior',
'javelin-typeahead-ondemand-source',
@ -1289,6 +1277,22 @@ return array(
'javelin-workflow',
'javelin-dom',
),
'64a5550f' => array(
'javelin-dom',
'javelin-util',
'javelin-stratcom',
'javelin-install',
'javelin-request',
'javelin-workflow',
),
'65ef6074' => array(
'javelin-behavior',
'javelin-stratcom',
'javelin-dom',
'javelin-util',
'javelin-vector',
'differential-inline-comment-editor',
),
'665cf6ac' => array(
'javelin-behavior',
'javelin-util',
@ -1817,14 +1821,6 @@ return array(
'javelin-dom',
'javelin-view',
),
'd4c87bf4' => array(
'javelin-dom',
'javelin-util',
'javelin-stratcom',
'javelin-install',
'javelin-request',
'javelin-workflow',
),
'd4eecc63' => array(
'javelin-behavior',
'javelin-dom',

View file

@ -66,6 +66,21 @@ final class DifferentialChangeType extends Phobject {
return idx($names, coalesce($type, '?'), '???');
}
public static function getIconForFileType($type) {
static $icons = array(
self::FILE_TEXT => 'fa-file-text-o',
self::FILE_IMAGE => 'fa-file-image-o',
self::FILE_BINARY => 'fa-file',
self::FILE_DIRECTORY => 'fa-folder-open',
self::FILE_SYMLINK => 'fa-link',
self::FILE_DELETED => 'fa-file',
self::FILE_NORMAL => 'fa-file-text-o',
self::FILE_SUBMODULE => 'fa-folder-open-o',
);
return idx($icons, $type, 'fa-file');
}
public static function isOldLocationChangeType($type) {
static $types = array(
self::TYPE_MOVE_AWAY => true,

View file

@ -856,6 +856,7 @@ final class DiffusionCommitController extends DiffusionController {
}
private function buildMergesTable(PhabricatorRepositoryCommit $commit) {
$viewer = $this->getViewer();
$drequest = $this->getDiffusionRequest();
$repository = $drequest->getRepository();
@ -892,15 +893,12 @@ final class DiffusionCommitController extends DiffusionController {
new PhutilNumber($limit)));
}
$history_table = new DiffusionHistoryTableView();
$history_table->setUser($this->getRequest()->getUser());
$history_table->setDiffusionRequest($drequest);
$history_table->setHistory($merges);
$history_table->loadRevisions();
$history_table = id(new DiffusionHistoryTableView())
->setUser($viewer)
->setDiffusionRequest($drequest)
->setHistory($merges);
$phids = $history_table->getRequiredHandlePHIDs();
$handles = $this->loadViewerHandles($phids);
$history_table->setHandles($handles);
$history_table->loadRevisions();
$panel = new PHUIObjectBoxView();
$panel->setHeaderText(pht('Merged Changes'));
@ -1110,7 +1108,11 @@ final class DiffusionCommitController extends DiffusionController {
$anchor = substr(md5($path), 0, 8);
$history_link = $diffusion_view->linkHistory($path);
$browse_link = $diffusion_view->linkBrowse($path);
$browse_link = $diffusion_view->linkBrowse(
$path,
array(
'type' => $changeset->getFileType(),
));
$item = id(new PHUIDiffTableOfContentsItemView())
->setChangeset($changeset)

View file

@ -42,15 +42,12 @@ final class DiffusionHistoryController extends DiffusionController {
$show_graph = !strlen($drequest->getPath());
$content = array();
$history_table = new DiffusionHistoryTableView();
$history_table->setUser($request->getUser());
$history_table->setDiffusionRequest($drequest);
$history_table->setHistory($history);
$history_table->loadRevisions();
$history_table = id(new DiffusionHistoryTableView())
->setUser($request->getUser())
->setDiffusionRequest($drequest)
->setHistory($history);
$phids = $history_table->getRequiredHandlePHIDs();
$handles = $this->loadViewerHandles($phids);
$history_table->setHandles($handles);
$history_table->loadRevisions();
if ($show_graph) {
$history_table->setParents($history_results['parents']);

View file

@ -177,8 +177,7 @@ final class DiffusionRepositoryController extends DiffusionController {
$content[] = $this->buildHistoryTable(
$history_results,
$history,
$history_exception,
$handles);
$history_exception);
try {
$content[] = $this->buildTagListTable($drequest);
@ -519,8 +518,7 @@ final class DiffusionRepositoryController extends DiffusionController {
private function buildHistoryTable(
$history_results,
$history,
$history_exception,
array $handles) {
$history_exception) {
$request = $this->getRequest();
$viewer = $request->getUser();
@ -544,7 +542,6 @@ final class DiffusionRepositoryController extends DiffusionController {
$history_table = id(new DiffusionHistoryTableView())
->setUser($viewer)
->setDiffusionRequest($drequest)
->setHandles($handles)
->setHistory($history);
// TODO: Super sketchy.

View file

@ -40,25 +40,26 @@ final class DiffusionBrowseTableView extends DiffusionView {
$browse_link = phutil_tag('strong', array(), $this->linkBrowse(
$base_path.$path->getPath().$dir_slash,
array(
'text' => $this->renderPathIcon('dir', $browse_text),
'type' => $file_type,
'name' => $browse_text,
)));
} else if ($file_type == DifferentialChangeType::FILE_SUBMODULE) {
$browse_text = $path->getPath().'/';
$browse_link = phutil_tag('strong', array(), $this->linkExternal(
$path->getHash(),
$path->getExternalURI(),
$this->renderPathIcon('ext', $browse_text)));
$browse_link = phutil_tag('strong', array(), $this->linkBrowse(
null,
array(
'type' => $file_type,
'name' => $browse_text,
'hash' => $path->getHash(),
'external' => $path->getExternalURI(),
)));
} else {
if ($file_type == DifferentialChangeType::FILE_SYMLINK) {
$type = 'link';
} else {
$type = 'file';
}
$browse_text = $path->getPath();
$browse_link = $this->linkBrowse(
$base_path.$path->getPath(),
array(
'text' => $this->renderPathIcon($type, $browse_text),
'type' => $file_type,
'name' => $browse_text,
));
}
@ -151,16 +152,4 @@ final class DiffusionBrowseTableView extends DiffusionView {
return $view->render();
}
private function renderPathIcon($type, $text) {
require_celerity_resource('diffusion-icons-css');
return phutil_tag(
'span',
array(
'class' => 'diffusion-path-icon diffusion-path-icon-'.$type,
),
$text);
}
}

View file

@ -17,11 +17,11 @@ final class DiffusionEmptyResultView extends DiffusionView {
public function render() {
$drequest = $this->getDiffusionRequest();
$repository = $drequest->getRepository();
$commit = $drequest->getCommit();
$callsign = $drequest->getRepository()->getCallsign();
if ($commit) {
$commit = "r{$callsign}{$commit}";
$commit = $repository->formatCommitName($commit);
} else {
$commit = 'HEAD';
}
@ -37,29 +37,38 @@ final class DiffusionEmptyResultView extends DiffusionView {
break;
case DiffusionBrowseResultSet::REASON_IS_EMPTY:
$title = pht('Empty Directory');
$body = pht("This path was an empty directory at %s.\n", $commit);
$body = pht('This path was an empty directory at %s.', $commit);
$severity = PHUIInfoView::SEVERITY_NOTICE;
break;
case DiffusionBrowseResultSet::REASON_IS_DELETED:
$deleted = $this->browseResultSet->getDeletedAtCommit();
$existed = $this->browseResultSet->getExistedAtCommit();
$browse = $this->linkBrowse(
$drequest->getPath(),
$existed_text = $repository->formatCommitName($existed);
$existed_href = $drequest->generateURI(
array(
'text' => 'existed',
'action' => 'browse',
'path' => $drequest->getPath(),
'commit' => $existed,
'params' => array('view' => $this->view),
'params' => array(
'view' => $this->view,
),
));
$existed_link = phutil_tag(
'a',
array(
'href' => $existed_href,
),
$existed_text);
$title = pht('Path Was Deleted');
$body = pht(
'This path does not exist at %s. It was deleted in %s and last %s '.
'at %s.',
'This path does not exist at %s. It was deleted in %s and last '.
'existed at %s.',
$commit,
self::linkCommit($drequest->getRepository(), $deleted),
$browse,
"r{$callsign}{$existed}");
$existed_link);
$severity = PHUIInfoView::SEVERITY_WARNING;
break;
case DiffusionBrowseResultSet::REASON_IS_UNTRACKED_PARENT:

View file

@ -36,7 +36,7 @@ final class DiffusionHistoryTableView extends DiffusionView {
return $this;
}
public function getRequiredHandlePHIDs() {
private function getRequiredHandlePHIDs() {
$phids = array();
foreach ($this->history as $item) {
$data = $item->getCommitData();
@ -87,7 +87,8 @@ final class DiffusionHistoryTableView extends DiffusionView {
public function render() {
$drequest = $this->getDiffusionRequest();
$handles = $this->handles;
$viewer = $this->getUser();
$handles = $viewer->loadHandles($this->getRequiredHandlePHIDs());
$graph = null;
if ($this->parents) {
@ -188,8 +189,17 @@ final class DiffusionHistoryTableView extends DiffusionView {
}
}
$browse = $this->linkBrowse(
$history->getPath(),
array(
'commit' => $history->getCommitIdentifier(),
'branch' => $drequest->getBranch(),
'type' => $history->getFileType(),
));
$rows[] = array(
$graph ? $graph[$ii++] : null,
$browse,
self::linkCommit(
$drequest->getRepository(),
$history->getCommitIdentifier()),
@ -207,9 +217,10 @@ final class DiffusionHistoryTableView extends DiffusionView {
$view = new AphrontTableView($rows);
$view->setHeaders(
array(
'',
null,
null,
pht('Commit'),
'',
null,
pht('Revision'),
pht('Author/Committer'),
pht('Details'),
@ -219,6 +230,7 @@ final class DiffusionHistoryTableView extends DiffusionView {
$view->setColumnClasses(
array(
'threads',
'nudgeright',
'n',
'icon',
'n',
@ -237,6 +249,7 @@ final class DiffusionHistoryTableView extends DiffusionView {
true,
true,
true,
true,
false,
true,
false,

View file

@ -13,36 +13,6 @@ abstract class DiffusionView extends AphrontView {
return $this->diffusionRequest;
}
final public function linkChange(
$change_type,
$file_type,
$path = null,
$commit_identifier = null) {
$text = DifferentialChangeType::getFullNameForChangeType($change_type);
if ($change_type == DifferentialChangeType::TYPE_CHILD) {
// TODO: Don't link COPY_AWAY without a direct change.
return $text;
}
if ($file_type == DifferentialChangeType::FILE_DIRECTORY) {
return $text;
}
$href = $this->getDiffusionRequest()->generateURI(
array(
'action' => 'change',
'path' => $path,
'commit' => $commit_identifier,
));
return phutil_tag(
'a',
array(
'href' => $href,
),
$text);
}
final public function linkHistory($path) {
$href = $this->getDiffusionRequest()->generateURI(
array(
@ -50,50 +20,83 @@ abstract class DiffusionView extends AphrontView {
'path' => $path,
));
return phutil_tag(
return javelin_tag(
'a',
array(
'href' => $href,
'class' => 'diffusion-link-icon',
'sigil' => 'has-tooltip',
'meta' => array(
'tip' => pht('History'),
'align' => 'E',
),
),
pht('History'));
id(new PHUIIconView())->setIconFont('fa-list-ul blue'));
}
final public function linkBrowse($path, array $details = array()) {
require_celerity_resource('diffusion-icons-css');
Javelin::initBehavior('phabricator-tooltips');
$href = $this->getDiffusionRequest()->generateURI(
$details + array(
'action' => 'browse',
'path' => $path,
));
$file_type = idx($details, 'type');
unset($details['type']);
if (isset($details['text'])) {
$text = $details['text'];
} else {
$text = pht('Browse');
$display_name = idx($details, 'name');
unset($details['name']);
if (strlen($display_name)) {
$display_name = phutil_tag(
'span',
array(
'class' => 'diffusion-browse-name',
),
$display_name);
}
return phutil_tag(
'a',
array(
'href' => $href,
),
$text);
}
final public function linkExternal($hash, $uri, $text) {
$href = id(new PhutilURI('/diffusion/external/'))
->setQueryParams(
array(
'uri' => $uri,
'id' => $hash,
if (isset($details['external'])) {
$href = id(new PhutilURI('/diffusion/external/'))
->setQueryParams(
array(
'uri' => idx($details, 'external'),
'id' => idx($details, 'hash'),
));
$tip = pht('Browse External');
} else {
$href = $this->getDiffusionRequest()->generateURI(
$details + array(
'action' => 'browse',
'path' => $path,
));
$tip = pht('Browse');
}
return phutil_tag(
$icon = DifferentialChangeType::getIconForFileType($file_type);
$icon_view = id(new PHUIIconView())->setIconFont("{$icon} blue");
// If we're rendering a file or directory name, don't show the tooltip.
if ($display_name !== null) {
$sigil = null;
$meta = null;
} else {
$sigil = 'has-tooltip';
$meta = array(
'tip' => $tip,
'align' => 'E',
);
}
return javelin_tag(
'a',
array(
'href' => $href,
'class' => 'diffusion-link-icon',
'sigil' => $sigil,
'meta' => $meta,
),
$text);
array(
$icon_view,
$display_name,
));
}
final public static function nameCommit(

View file

@ -107,7 +107,7 @@ final class PHUIDiffTableOfContentsListView extends AphrontView {
))
->setColumnClasses(
array(
'center',
null,
'differential-toc-char center',
'differential-toc-prop center',
'differential-toc-ftype center',

View file

@ -175,6 +175,10 @@ th.aphront-table-view-sortable-selected {
text-align: right;
}
.aphront-table-view td.nudgeright, .aphront-table-view th.nudgeright {
padding-right: 0;
}
.aphront-table-view td.wrap {
white-space: normal;
}
@ -194,6 +198,8 @@ th.aphront-table-view-sortable-selected {
padding: 0px;
}
div.single-display-line-bounds {
width: 100%;
position: relative;

View file

@ -2,31 +2,6 @@
* @provides diffusion-icons-css
*/
.diffusion-path-icon {
display: block;
padding-left: 28px;
background-repeat: no-repeat;
background-position: 1px 1px;
height: 18px;
padding-top: 1px;
}
.diffusion-path-icon-ext {
background-image: url(/rsrc/image/icon/fatcow/folder_go.png);
}
.diffusion-path-icon-dir {
background-image: url(/rsrc/image/icon/fatcow/folder.png);
}
.diffusion-path-icon-file {
background-image: url(/rsrc/image/icon/fatcow/page_white_text.png);
}
.diffusion-path-icon-link {
background-image: url(/rsrc/image/icon/fatcow/page_white_link.png);
}
input.diffusion-clone-uri {
display: block;
width: 100%;
@ -37,3 +12,11 @@ input.diffusion-clone-uri {
text-align: right;
color: {$lightgreytext};
}
.diffusion-browse-name {
margin-left: 8px;
}
.diffusion-link-icon + .diffusion-link-icon {
margin-left: 6px;
}

Binary file not shown.

Before

Width:  |  Height:  |  Size: 632 B

Binary file not shown.

Before

Width:  |  Height:  |  Size: 803 B

Binary file not shown.

Before

Width:  |  Height:  |  Size: 732 B

Binary file not shown.

Before

Width:  |  Height:  |  Size: 568 B