From 39e5da7ea700097f2cb0170ea76901286a3165ad Mon Sep 17 00:00:00 2001 From: Chad Little Date: Sat, 8 Jul 2017 19:54:23 -0700 Subject: [PATCH] Simplify Diffusion Browse Table Summary: Cleans up colors, removes commit hash and links the text instead. Also unsure how valuable "lint" column is here, but left it. I'd maybe like to understand that workflow since it just seems like clutter overall. Also Fixes T12905 Test Plan: Review Phabricator, hg, and a few other test repositories locally. Holler if anything here seems bad, but this feels easier to read and use to me. {F5038425} Reviewers: epriestley Reviewed By: epriestley Subscribers: Korvin Maniphest Tasks: T12905 Differential Revision: https://secure.phabricator.com/D18189 --- resources/celerity/map.php | 16 ++++++------ .../constants/DifferentialChangeType.php | 14 +++++----- .../DiffusionLastModifiedController.php | 6 ++++- .../view/DiffusionBrowseTableView.php | 26 +++++-------------- .../diffusion/view/DiffusionView.php | 13 ++++++++++ .../diffusion/diffusion-history.css | 10 +++++++ .../application/diffusion/diffusion-icons.css | 1 + webroot/rsrc/css/phui/phui-icon.css | 4 +++ 8 files changed, 55 insertions(+), 35 deletions(-) diff --git a/resources/celerity/map.php b/resources/celerity/map.php index 6a3b8778e6..939b48a643 100644 --- a/resources/celerity/map.php +++ b/resources/celerity/map.php @@ -9,12 +9,12 @@ return array( 'names' => array( 'conpherence.pkg.css' => 'ff161f2d', 'conpherence.pkg.js' => 'b5b51108', - 'core.pkg.css' => '37dd219b', + 'core.pkg.css' => '7ae9e755', 'core.pkg.js' => '5d80e0db', 'darkconsole.pkg.js' => '1f9a31bc', 'differential.pkg.css' => '4ec4a37a', 'differential.pkg.js' => 'd4ab0e81', - 'diffusion.pkg.css' => 'b93d9b8c', + 'diffusion.pkg.css' => 'a2d17c7d', 'diffusion.pkg.js' => '6134c5a1', 'favicon.ico' => '30672e08', 'maniphest.pkg.css' => '4845691a', @@ -71,8 +71,8 @@ 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-history.css' => '4540f568', - 'rsrc/css/application/diffusion/diffusion-icons.css' => 'a6a1e2ba', + 'rsrc/css/application/diffusion/diffusion-history.css' => '898ed727', + 'rsrc/css/application/diffusion/diffusion-icons.css' => '0c15255e', 'rsrc/css/application/diffusion/diffusion-readme.css' => '419dd5b6', 'rsrc/css/application/diffusion/diffusion-repository.css' => 'ee6f20ec', 'rsrc/css/application/diffusion/diffusion-source.css' => '750add59', @@ -161,7 +161,7 @@ return array( 'rsrc/css/phui/phui-header-view.css' => 'e7de7ee2', 'rsrc/css/phui/phui-hovercard.css' => 'f0592bcf', 'rsrc/css/phui/phui-icon-set-selector.css' => '87db8fee', - 'rsrc/css/phui/phui-icon.css' => '4c46b6ba', + 'rsrc/css/phui/phui-icon.css' => '5c4a5de6', 'rsrc/css/phui/phui-image-mask.css' => 'a8498f9c', 'rsrc/css/phui/phui-info-panel.css' => '27ea50a1', 'rsrc/css/phui/phui-info-view.css' => '6e217679', @@ -570,8 +570,8 @@ return array( 'differential-revision-history-css' => '0e8eb855', 'differential-revision-list-css' => 'f3c47d33', 'differential-table-of-contents-css' => 'ae4b7a55', - 'diffusion-history-css' => '4540f568', - 'diffusion-icons-css' => 'a6a1e2ba', + 'diffusion-history-css' => '898ed727', + 'diffusion-icons-css' => '0c15255e', 'diffusion-readme-css' => '419dd5b6', 'diffusion-repository-css' => 'ee6f20ec', 'diffusion-source-css' => '750add59', @@ -849,7 +849,7 @@ return array( 'phui-hovercard' => '1bd28176', 'phui-hovercard-view-css' => 'f0592bcf', 'phui-icon-set-selector-css' => '87db8fee', - 'phui-icon-view-css' => '4c46b6ba', + 'phui-icon-view-css' => '5c4a5de6', 'phui-image-mask-css' => 'a8498f9c', 'phui-info-panel-css' => '27ea50a1', 'phui-info-view-css' => '6e217679', diff --git a/src/applications/differential/constants/DifferentialChangeType.php b/src/applications/differential/constants/DifferentialChangeType.php index 7ee3c89b65..fd0bcf81bb 100644 --- a/src/applications/differential/constants/DifferentialChangeType.php +++ b/src/applications/differential/constants/DifferentialChangeType.php @@ -71,7 +71,7 @@ final class DifferentialChangeType extends Phobject { 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_DIRECTORY => 'fa-folder', self::FILE_SYMLINK => 'fa-link', self::FILE_DELETED => 'fa-file', self::FILE_NORMAL => 'fa-file-text-o', @@ -83,14 +83,14 @@ final class DifferentialChangeType extends Phobject { public static function getIconColorForFileType($type) { static $icons = array( - self::FILE_TEXT => 'black', - self::FILE_IMAGE => 'black', + self::FILE_TEXT => 'bluetext', + self::FILE_IMAGE => 'bluetext', self::FILE_BINARY => 'green', - self::FILE_DIRECTORY => 'blue', - self::FILE_SYMLINK => 'blue', + self::FILE_DIRECTORY => 'bluetext', + self::FILE_SYMLINK => 'bluetext', self::FILE_DELETED => 'red', - self::FILE_NORMAL => 'black', - self::FILE_SUBMODULE => 'blue', + self::FILE_NORMAL => 'bluetext', + self::FILE_SUBMODULE => 'bluetext', ); return idx($icons, $type, 'black'); diff --git a/src/applications/diffusion/controller/DiffusionLastModifiedController.php b/src/applications/diffusion/controller/DiffusionLastModifiedController.php index faf9457c15..945f8a58b5 100644 --- a/src/applications/diffusion/controller/DiffusionLastModifiedController.php +++ b/src/applications/diffusion/controller/DiffusionLastModifiedController.php @@ -132,7 +132,11 @@ final class DiffusionLastModifiedController extends DiffusionController { } } - $details = AphrontTableView::renderSingleDisplayLine($data->getSummary()); + $details = DiffusionView::linkDetail( + $drequest->getRepository(), + $commit->getCommitIdentifier(), + $data->getSummary()); + $details = AphrontTableView::renderSingleDisplayLine($details); } else { $author = ''; $details = ''; diff --git a/src/applications/diffusion/view/DiffusionBrowseTableView.php b/src/applications/diffusion/view/DiffusionBrowseTableView.php index ffc5ce61ed..bd53cf97a1 100644 --- a/src/applications/diffusion/view/DiffusionBrowseTableView.php +++ b/src/applications/diffusion/view/DiffusionBrowseTableView.php @@ -20,6 +20,7 @@ final class DiffusionBrowseTableView extends DiffusionView { public function render() { $request = $this->getDiffusionRequest(); $repository = $request->getRepository(); + require_celerity_resource('diffusion-history-css'); $base_path = trim($request->getPath(), '/'); if ($base_path) { @@ -74,7 +75,6 @@ final class DiffusionBrowseTableView extends DiffusionView { $dict = array( 'lint' => celerity_generate_unique_node_id(), - 'commit' => celerity_generate_unique_node_id(), 'date' => celerity_generate_unique_node_id(), 'author' => celerity_generate_unique_node_id(), 'details' => celerity_generate_unique_node_id(), @@ -86,13 +86,13 @@ final class DiffusionBrowseTableView extends DiffusionView { } $rows[] = array( - $history_link, $browse_link, idx($dict, 'lint'), - $dict['commit'], $dict['details'], $dict['date'], + $history_link, ); + } if ($need_pull) { @@ -113,27 +113,16 @@ final class DiffusionBrowseTableView extends DiffusionView { $lint = $request->getLint(); $view = new AphrontTableView($rows); - $view->setHeaders( - array( - null, - pht('Path'), - ($lint ? $lint : pht('Lint')), - pht('Modified'), - pht('Details'), - pht('Committed'), - )); $view->setColumnClasses( array( - 'nudgeright', '', '', - '', - 'wide', + 'wide commit-detail', 'right', + 'right narrow', )); $view->setColumnVisibility( array( - true, true, $show_lint, true, @@ -144,15 +133,14 @@ final class DiffusionBrowseTableView extends DiffusionView { $view->setDeviceVisibility( array( true, - true, - false, false, true, false, + false, )); - return $view->render(); + return phutil_tag_div('diffusion-browse-table', $view->render()); } } diff --git a/src/applications/diffusion/view/DiffusionView.php b/src/applications/diffusion/view/DiffusionView.php index d058dc5cec..eb43e49f3c 100644 --- a/src/applications/diffusion/view/DiffusionView.php +++ b/src/applications/diffusion/view/DiffusionView.php @@ -156,6 +156,19 @@ abstract class DiffusionView extends AphrontView { $commit_name); } + final public static function linkDetail( + PhabricatorRepository $repository, + $commit, + $detail) { + + return phutil_tag( + 'a', + array( + 'href' => $repository->getCommitURI($commit), + ), + $detail); + } + final public static function linkRevision($id) { if (!$id) { return null; diff --git a/webroot/rsrc/css/application/diffusion/diffusion-history.css b/webroot/rsrc/css/application/diffusion/diffusion-history.css index f4a51f7b56..7339d05ac1 100644 --- a/webroot/rsrc/css/application/diffusion/diffusion-history.css +++ b/webroot/rsrc/css/application/diffusion/diffusion-history.css @@ -48,6 +48,16 @@ margin-right: 4px; } +/* - Browse Styles ----------------------------------------------------------*/ + +.diffusion-browse-table .commit-detail { + padding-left: 32px; +} + +.diffusion-browse-table .commit-detail a { + color: {$darkbluetext}; +} + /* - Phone Style ------------------------------------------------------------*/ .device-phone.diffusion-history-view .phui-two-column-view diff --git a/webroot/rsrc/css/application/diffusion/diffusion-icons.css b/webroot/rsrc/css/application/diffusion/diffusion-icons.css index 8edc034975..072db01660 100644 --- a/webroot/rsrc/css/application/diffusion/diffusion-icons.css +++ b/webroot/rsrc/css/application/diffusion/diffusion-icons.css @@ -20,6 +20,7 @@ input.diffusion-clone-uri { .diffusion-browse-name { margin-left: 8px; + letter-spacing: 0.02em; } .diffusion-link-icon + .diffusion-link-icon { diff --git a/webroot/rsrc/css/phui/phui-icon.css b/webroot/rsrc/css/phui/phui-icon.css index 5fd7fd97bc..acc7818765 100644 --- a/webroot/rsrc/css/phui/phui-icon.css +++ b/webroot/rsrc/css/phui/phui-icon.css @@ -45,6 +45,10 @@ img.phui-image-disabled { filter: grayscale(100%); } +.phui-icon-view.bluetext { + color: {$bluetext}; +} + /* - Icon in a Circle ------------------------------------------------------- */ .phui-icon-circle {