From 21f0ce73626926652477712791138b25f884907b Mon Sep 17 00:00:00 2001 From: Chad Little Date: Wed, 8 Apr 2015 08:38:15 -0700 Subject: [PATCH] Make taller tables the default for AphrontTableView Summary: I considered at the time just making all tables taller. This removes the special casing and adds the space universally. On first glance all smaller tables look great, but Diffusion seems a little bloated. After a short time period though that went away for me. I do think Diffusion overall needs a UI refresh. Test Plan: Tested numerous tables in Phortune, Diffusion, etc. Spacing feels more readable. Reviewers: btrahan, epriestley Reviewed By: epriestley Subscribers: Korvin, epriestley Differential Revision: https://secure.phabricator.com/D12328 --- resources/celerity/map.php | 22 +++++++++---------- .../phortune/view/PhortuneChargeTableView.php | 1 - .../phortune/view/PhortuneOrderTableView.php | 1 - .../view/PhortuneSubscriptionTableView.php | 1 - src/view/control/AphrontTableView.php | 9 -------- webroot/rsrc/css/aphront/table-view.css | 12 ++-------- .../diffusion/behavior-commit-graph.js | 2 +- 7 files changed, 14 insertions(+), 34 deletions(-) diff --git a/resources/celerity/map.php b/resources/celerity/map.php index ef8f80da19..507e5c66b0 100644 --- a/resources/celerity/map.php +++ b/resources/celerity/map.php @@ -7,13 +7,13 @@ */ return array( 'names' => array( - 'core.pkg.css' => 'efa903e0', + 'core.pkg.css' => '7d67693e', 'core.pkg.js' => '6a4f677f', 'darkconsole.pkg.js' => '8ab24e01', 'differential.pkg.css' => '3500921f', 'differential.pkg.js' => 'c0506961', 'diffusion.pkg.css' => '591664fa', - 'diffusion.pkg.js' => 'bfc0737b', + 'diffusion.pkg.js' => '0115b37c', 'maniphest.pkg.css' => '68d4dd3d', 'maniphest.pkg.js' => 'df4aa49f', 'rsrc/css/aphront/aphront-bars.css' => '231ac33c', @@ -26,7 +26,7 @@ return array( 'rsrc/css/aphront/pager-view.css' => '2e3539af', 'rsrc/css/aphront/panel-view.css' => '8427b78d', 'rsrc/css/aphront/phabricator-nav-view.css' => '7aeaf435', - 'rsrc/css/aphront/table-view.css' => '3e77fefe', + 'rsrc/css/aphront/table-view.css' => '59e2c0f8', 'rsrc/css/aphront/tokenizer.css' => '82ce2142', 'rsrc/css/aphront/tooltip.css' => '7672b60f', 'rsrc/css/aphront/transaction.css' => 'bd9f9f6e', @@ -380,7 +380,7 @@ return array( 'rsrc/js/application/diffusion/DiffusionLocateFileSource.js' => 'b42eddc7', 'rsrc/js/application/diffusion/behavior-audit-preview.js' => 'd835b03a', 'rsrc/js/application/diffusion/behavior-commit-branches.js' => 'bdaf4d04', - 'rsrc/js/application/diffusion/behavior-commit-graph.js' => 'f7f1289f', + 'rsrc/js/application/diffusion/behavior-commit-graph.js' => '9007c197', 'rsrc/js/application/diffusion/behavior-jump-to.js' => '73d09eef', 'rsrc/js/application/diffusion/behavior-load-blame.js' => '42126667', 'rsrc/js/application/diffusion/behavior-locate-file.js' => '6d3e1947', @@ -505,7 +505,7 @@ return array( 'aphront-multi-column-view-css' => 'fd18389d', 'aphront-pager-view-css' => '2e3539af', 'aphront-panel-view-css' => '8427b78d', - 'aphront-table-view-css' => '3e77fefe', + 'aphront-table-view-css' => '59e2c0f8', 'aphront-tokenizer-control-css' => '82ce2142', 'aphront-tooltip-css' => '7672b60f', 'aphront-two-column-view-css' => '16ab3ad2', @@ -579,7 +579,7 @@ return array( 'javelin-behavior-differential-toggle-files' => 'ca3f91eb', 'javelin-behavior-differential-user-select' => 'a8d8459d', 'javelin-behavior-diffusion-commit-branches' => 'bdaf4d04', - 'javelin-behavior-diffusion-commit-graph' => 'f7f1289f', + 'javelin-behavior-diffusion-commit-graph' => '9007c197', 'javelin-behavior-diffusion-jump-to' => '73d09eef', 'javelin-behavior-diffusion-locate-file' => '6d3e1947', 'javelin-behavior-diffusion-pull-lastmodified' => '2b228192', @@ -1524,6 +1524,11 @@ return array( 'javelin-uri', 'phabricator-notification', ), + '9007c197' => array( + 'javelin-behavior', + 'javelin-dom', + 'javelin-stratcom', + ), '9414ff18' => array( 'javelin-behavior', 'javelin-resource', @@ -1955,11 +1960,6 @@ return array( 'javelin-util', 'phabricator-shaped-request', ), - 'f7f1289f' => array( - 'javelin-behavior', - 'javelin-dom', - 'javelin-stratcom', - ), 'f7fc67ec' => array( 'javelin-install', 'javelin-typeahead', diff --git a/src/applications/phortune/view/PhortuneChargeTableView.php b/src/applications/phortune/view/PhortuneChargeTableView.php index 0ce6ebda74..4e82404cc6 100644 --- a/src/applications/phortune/view/PhortuneChargeTableView.php +++ b/src/applications/phortune/view/PhortuneChargeTableView.php @@ -55,7 +55,6 @@ final class PhortuneChargeTableView extends AphrontView { } $table = id(new AphrontTableView($rows)) - ->setTallTable(true) ->setHeaders( array( pht('ID'), diff --git a/src/applications/phortune/view/PhortuneOrderTableView.php b/src/applications/phortune/view/PhortuneOrderTableView.php index cb4d6b5cfe..a2e492c5be 100644 --- a/src/applications/phortune/view/PhortuneOrderTableView.php +++ b/src/applications/phortune/view/PhortuneOrderTableView.php @@ -125,7 +125,6 @@ final class PhortuneOrderTableView extends AphrontView { } $table = id(new AphrontTableView($rows)) - ->setTallTable(true) ->setNoDataString($this->getNoDataString()) ->setRowClasses($rowc) ->setHeaders( diff --git a/src/applications/phortune/view/PhortuneSubscriptionTableView.php b/src/applications/phortune/view/PhortuneSubscriptionTableView.php index edb134b410..e5d6e5d517 100644 --- a/src/applications/phortune/view/PhortuneSubscriptionTableView.php +++ b/src/applications/phortune/view/PhortuneSubscriptionTableView.php @@ -61,7 +61,6 @@ final class PhortuneSubscriptionTableView extends AphrontView { } $table = id(new AphrontTableView($rows)) - ->setTallTable(true) ->setHeaders( array( pht('ID'), diff --git a/src/view/control/AphrontTableView.php b/src/view/control/AphrontTableView.php index a9ce74eac0..9edd851c19 100644 --- a/src/view/control/AphrontTableView.php +++ b/src/view/control/AphrontTableView.php @@ -11,7 +11,6 @@ final class AphrontTableView extends AphrontView { protected $zebraStripes = true; protected $noDataString; protected $className; - protected $tallTable; protected $columnVisibility = array(); private $deviceVisibility = array(); @@ -76,11 +75,6 @@ final class AphrontTableView extends AphrontView { return $this; } - public function setTallTable($tall) { - $this->tallTable = $tall; - return $this; - } - public function setShortHeaders(array $short_headers) { $this->shortHeaders = $short_headers; return $this; @@ -295,9 +289,6 @@ final class AphrontTableView extends AphrontView { if ($this->className !== null) { $classes[] = $this->className; } - if ($this->tallTable) { - $classes[] = 'aphront-tall-table-view'; - } if ($this->deviceReadyTable) { $classes[] = 'aphront-table-view-device-ready'; } diff --git a/webroot/rsrc/css/aphront/table-view.css b/webroot/rsrc/css/aphront/table-view.css index 96a5167433..d82cc52c64 100644 --- a/webroot/rsrc/css/aphront/table-view.css +++ b/webroot/rsrc/css/aphront/table-view.css @@ -95,21 +95,13 @@ th.aphront-table-view-sortable-selected { */ .aphront-table-view th { - padding: 8px 10px; + padding: 10px 10px; font-size: 13px; } -.aphront-table-view.aphront-tall-table-view th { - padding: 10px 10px; -} - .aphront-table-view td { - padding: 6px 10px; - font-size: 12px; -} - -.aphront-table-view.aphront-tall-table-view td { padding: 8px 10px; + font-size: 12px; } .device-tablet .aphront-table-view td, diff --git a/webroot/rsrc/js/application/diffusion/behavior-commit-graph.js b/webroot/rsrc/js/application/diffusion/behavior-commit-graph.js index 75844369b2..97f1de568d 100644 --- a/webroot/rsrc/js/application/diffusion/behavior-commit-graph.js +++ b/webroot/rsrc/js/application/diffusion/behavior-commit-graph.js @@ -52,7 +52,7 @@ JX.behavior('diffusion-commit-graph', function(config) { return (col * cell) + (cell / 2); }; - var h = 26; + var h = 30; var w = cell * config.count; var canvas = JX.$N('canvas', {width: w, height: h});