From c453e98c40931e7c8368e42fc0dfa21145c9826e Mon Sep 17 00:00:00 2001 From: Chad Little Date: Sun, 27 Apr 2014 11:18:48 -0700 Subject: [PATCH] Moderize Herald UI Summary: Removes many tables and uses PropertyLists and ObjectItemList when possible. Adds cleaner CSS, makes mobile editing more possible. Test Plan: Test new UI on desktop and mobile. Verify all functionality still exists. Reviewers: btrahan, epriestley Reviewed By: epriestley Subscribers: epriestley, Korvin Maniphest Tasks: T4272 Differential Revision: https://secure.phabricator.com/D8860 --- resources/celerity/map.php | 8 +-- .../herald/adapter/HeraldAdapter.php | 60 ++++++++++++---- .../controller/HeraldRuleListController.php | 3 +- .../controller/HeraldRuleViewController.php | 12 ++-- .../controller/HeraldTranscriptController.php | 39 +++++------ .../HeraldTranscriptListController.php | 68 ++++++++----------- .../css/application/herald/herald-test.css | 35 +++------- .../rsrc/css/application/herald/herald.css | 15 ++++ 8 files changed, 129 insertions(+), 111 deletions(-) diff --git a/resources/celerity/map.php b/resources/celerity/map.php index 059c55af61..a2ad6f0075 100644 --- a/resources/celerity/map.php +++ b/resources/celerity/map.php @@ -69,8 +69,8 @@ return array( 'rsrc/css/application/files/global-drag-and-drop.css' => '697324ad', 'rsrc/css/application/flag/flag.css' => '5337623f', 'rsrc/css/application/harbormaster/harbormaster.css' => 'cec833b7', - 'rsrc/css/application/herald/herald-test.css' => '2b7d0f54', - 'rsrc/css/application/herald/herald.css' => '59d48f01', + 'rsrc/css/application/herald/herald-test.css' => '778b008e', + 'rsrc/css/application/herald/herald.css' => 'c544dd1c', 'rsrc/css/application/maniphest/batch-editor.css' => '8f380ebc', 'rsrc/css/application/maniphest/report.css' => '6fc16517', 'rsrc/css/application/maniphest/task-edit.css' => '8e23031b', @@ -522,9 +522,9 @@ return array( 'font-source-sans-pro' => '91d53463', 'global-drag-and-drop-css' => '697324ad', 'harbormaster-css' => 'cec833b7', - 'herald-css' => '59d48f01', + 'herald-css' => 'c544dd1c', 'herald-rule-editor' => '22d2966a', - 'herald-test-css' => '2b7d0f54', + 'herald-test-css' => '778b008e', 'inline-comment-summary-css' => '14a91639', 'javelin-aphlict' => '493665ee', 'javelin-behavior' => '8a3ed18b', diff --git a/src/applications/herald/adapter/HeraldAdapter.php b/src/applications/herald/adapter/HeraldAdapter.php index ceb2e21942..a246542b02 100644 --- a/src/applications/herald/adapter/HeraldAdapter.php +++ b/src/applications/herald/adapter/HeraldAdapter.php @@ -957,35 +957,71 @@ abstract class HeraldAdapter { public function renderRuleAsText(HeraldRule $rule, array $handles) { assert_instances_of($handles, 'PhabricatorObjectHandle'); - $out = array(); + require_celerity_resource('herald-css'); + + $icon = id(new PHUIIconView()) + ->setIconFont('fa-chevron-circle-right lightgreytext') + ->addClass('herald-list-icon'); if ($rule->getMustMatchAll()) { - $out[] = pht('When all of these conditions are met:'); + $match_text = pht('When all of these conditions are met:'); } else { - $out[] = pht('When any of these conditions are met:'); + $match_text = pht('When any of these conditions are met:'); } - $out[] = null; + $match_title = phutil_tag( + 'p', + array( + 'class' => 'herald-list-description' + ), + $match_text); + + $match_list = array(); foreach ($rule->getConditions() as $condition) { - $out[] = $this->renderConditionAsText($condition, $handles); + $match_list[] = phutil_tag( + 'div', + array( + 'class' => 'herald-list-item' + ), + array( + $icon, + $this->renderConditionAsText($condition, $handles))); } - $out[] = null; $integer_code_for_every = HeraldRepetitionPolicyConfig::toInt( HeraldRepetitionPolicyConfig::EVERY); if ($rule->getRepetitionPolicy() == $integer_code_for_every) { - $out[] = pht('Take these actions every time this rule matches:'); + $action_text = + pht('Take these actions every time this rule matches:'); } else { - $out[] = pht('Take these actions the first time this rule matches:'); + $action_text = + pht('Take these actions the first time this rule matches:'); } - $out[] = null; + $action_title = phutil_tag( + 'p', + array( + 'class' => 'herald-list-description' + ), + $action_text); + + $action_list = array(); foreach ($rule->getActions() as $action) { - $out[] = $this->renderActionAsText($action, $handles); - } + $action_list[] = phutil_tag( + 'div', + array( + 'class' => 'herald-list-item' + ), + array( + $icon, + $this->renderActionAsText($action, $handles))); } - return phutil_implode_html("\n", $out); + return array( + $match_title, + $match_list, + $action_title, + $action_list); } private function renderConditionAsText( diff --git a/src/applications/herald/controller/HeraldRuleListController.php b/src/applications/herald/controller/HeraldRuleListController.php index 8d5ea7f95f..7ef7a8faf8 100644 --- a/src/applications/herald/controller/HeraldRuleListController.php +++ b/src/applications/herald/controller/HeraldRuleListController.php @@ -36,7 +36,8 @@ final class HeraldRuleListController extends HeraldController $content_type_map = HeraldAdapter::getEnabledAdapterMap($viewer); $list = id(new PHUIObjectItemListView()) - ->setUser($viewer); + ->setUser($viewer) + ->setCards(true); foreach ($rules as $rule) { $id = $rule->getID(); diff --git a/src/applications/herald/controller/HeraldRuleViewController.php b/src/applications/herald/controller/HeraldRuleViewController.php index ea5c6b44e2..ef195e7258 100644 --- a/src/applications/herald/controller/HeraldRuleViewController.php +++ b/src/applications/herald/controller/HeraldRuleViewController.php @@ -45,6 +45,7 @@ final class HeraldRuleViewController extends HeraldController { $crumbs = $this->buildApplicationCrumbs(); $crumbs->addTextCrumb("H{$id}"); + $crumbs->setActionList($actions); $object_box = id(new PHUIObjectBoxView()) ->setHeader($header) @@ -148,14 +149,11 @@ final class HeraldRuleViewController extends HeraldController { $view->invokeWillRenderEvent(); - $view->addSectionHeader(pht('Rule Description')); + $view->addSectionHeader( + pht('Rule Description'), + PHUIPropertyListView::ICON_SUMMARY); $view->addTextContent( - phutil_tag( - 'div', - array( - 'style' => 'white-space: pre-wrap;', - ), - $adapter->renderRuleAsText($rule, $this->getLoadedHandles()))); + $adapter->renderRuleAsText($rule, $this->getLoadedHandles())); } return $view; diff --git a/src/applications/herald/controller/HeraldTranscriptController.php b/src/applications/herald/controller/HeraldTranscriptController.php index e0dfb024ae..0feb752d28 100644 --- a/src/applications/herald/controller/HeraldTranscriptController.php +++ b/src/applications/herald/controller/HeraldTranscriptController.php @@ -362,12 +362,11 @@ final class HeraldTranscriptController extends HeraldController { 'wide', )); - $panel = new AphrontPanelView(); - $panel->setHeader(pht('Actions Taken')); - $panel->appendChild($table); - $panel->setNoBackground(); + $box = new PHUIObjectBoxView(); + $box->setHeaderText(pht('Actions Taken')); + $box->appendChild($table); - return $panel; + return $box; } private function buildActionTranscriptPanel($xscript) { @@ -450,17 +449,16 @@ final class HeraldTranscriptController extends HeraldController { ))); } - $panel = ''; + $box = null; if ($rule_markup) { - $panel = new AphrontPanelView(); - $panel->setHeader(pht('Rule Details')); - $panel->setNoBackground(); - $panel->appendChild(phutil_tag( + $box = new PHUIObjectBoxView(); + $box->setHeaderText(pht('Rule Details')); + $box->appendChild(phutil_tag( 'ul', array('class' => 'herald-explain-list'), $rule_markup)); } - return $panel; + return $box; } private function buildObjectTranscriptPanel($xscript) { @@ -512,19 +510,16 @@ final class HeraldTranscriptController extends HeraldController { $rows[] = array($name, $value); } - $table = new AphrontTableView($rows); - $table->setColumnClasses( - array( - 'header', - 'wide', - )); + $property_list = new PHUIPropertyListView(); + foreach ($rows as $row) { + $property_list->addProperty($row[0], $row[1]); + } - $panel = new AphrontPanelView(); - $panel->setHeader(pht('Object Transcript')); - $panel->setNoBackground(); - $panel->appendChild($table); + $box = new PHUIObjectBoxView(); + $box->setHeaderText(pht('Object Transcript')); + $box->appendChild($property_list); - return $panel; + return $box; } diff --git a/src/applications/herald/controller/HeraldTranscriptListController.php b/src/applications/herald/controller/HeraldTranscriptListController.php index 37399c3647..f3a108177f 100644 --- a/src/applications/herald/controller/HeraldTranscriptListController.php +++ b/src/applications/herald/controller/HeraldTranscriptListController.php @@ -62,51 +62,39 @@ final class HeraldTranscriptListController extends HeraldController $handles = $this->loadViewerHandles($phids); } - $rows = array(); + $list = new PHUIObjectItemListView(); + $list->setCards(true); + $list->setFlush(true); foreach ($transcripts as $xscript) { - $rows[] = array( - phabricator_date($xscript->getTime(), $viewer), - phabricator_time($xscript->getTime(), $viewer), - $handles[$xscript->getObjectPHID()]->renderLink(), - $xscript->getDryRun() ? pht('Yes') : '', - number_format((int)(1000 * $xscript->getDuration())).' ms', - phutil_tag( - 'a', - array( - 'href' => '/herald/transcript/'.$xscript->getID().'/', - 'class' => 'button small grey', - ), - pht('View Transcript')), - ); + $view_href = phutil_tag( + 'a', + array( + 'href' => '/herald/transcript/'.$xscript->getID().'/', + ), + pht('View Full Transcript')); + + $item = new PHUIObjectItemView(); + $item->setObjectName($xscript->getID()); + $item->setHeader($view_href); + if ($xscript->getDryRun()) { + $item->addAttribute(pht('Dry Run')); + } + $item->addAttribute($handles[$xscript->getObjectPHID()]->renderLink()); + $item->addAttribute( + number_format((int)(1000 * $xscript->getDuration())).' ms'); + $item->addIcon( + 'none', + phabricator_datetime($xscript->getTime(), $viewer)); + + $list->addItem($item); } - $table = new AphrontTableView($rows); - $table->setHeaders( - array( - pht('Date'), - pht('Time'), - pht('Object'), - pht('Dry Run'), - pht('Duration'), - pht('View'), - )); - $table->setColumnClasses( - array( - '', - 'right', - 'wide wrap', - '', - '', - 'action', - )); - // Render the whole page. - $panel = new AphrontPanelView(); - $panel->setHeader(pht('Herald Transcripts')); - $panel->appendChild($table); - $panel->setNoBackground(); + $box = new PHUIObjectBoxView(); + $box->setHeaderText(pht('Herald Transcripts')); + $box->appendChild($list); - return $panel; + return $box; } diff --git a/webroot/rsrc/css/application/herald/herald-test.css b/webroot/rsrc/css/application/herald/herald-test.css index 10bef47128..178d54653d 100644 --- a/webroot/rsrc/css/application/herald/herald-test.css +++ b/webroot/rsrc/css/application/herald/herald-test.css @@ -3,8 +3,7 @@ */ ul.herald-explain-list { - font-size: 11px; - font-family: "Verdana"; + margin: 12px; } div.herald-condition-note { @@ -27,33 +26,21 @@ ul.herald-explain-list .herald-outcome { ul.herald-explain-list .condition-fail, ul.herald-explain-list .rule-fail { - color: #aa0000; + color: {$red}; } ul.herald-explain-list .condition-pass, ul.herald-explain-list .rule-pass { - color: #00aa00; + color: {$green}; } - - ul.herald-explain-list li.herald-rule-pass, ul.herald-explain-list li.herald-rule-fail { - margin: 0 0 0.75em; -} - -ul.herald-explain-list li.herald-rule-fail { - border: 1px solid #aa0000; -} - -ul.herald-explain-list li.herald-rule-pass { - border: 1px solid #00aa00; + margin: 0 0 8px; } ul.herald-explain-list div.rule-name { - border-bottom: 1px solid #cccccc; - font-size: 12px; - padding: .5em .75em; + padding: 4px 0 8px 0; } ul.herald-explain-list li.herald-rule-fail, @@ -62,7 +49,7 @@ ul.herald-explain-list li.herald-rule-pass { } ul.herald-explain-list ul { - margin: .5em; + margin: 4px; } ul.herald-explain-list ul li { @@ -75,18 +62,17 @@ ul.herald-explain-list ul li { } .outcome-failure { - color: #990000; + color: {$red}; } .outcome-success { - color: #009900; + color: {$green}; } - .action-header { font-weight: bold; - padding-top: 1em; - border-bottom: 1px solid #dddddd; + padding-top: 12px; + border-bottom: 1px solid {$thinblueborder}; } textarea.herald-field-value-transcript { @@ -94,7 +80,6 @@ textarea.herald-field-value-transcript { height: 10em; } - .condition-test-value { color: {$greytext}; } diff --git a/webroot/rsrc/css/application/herald/herald.css b/webroot/rsrc/css/application/herald/herald.css index 2546a08284..99078335a2 100644 --- a/webroot/rsrc/css/application/herald/herald.css +++ b/webroot/rsrc/css/application/herald/herald.css @@ -35,3 +35,18 @@ .herald-action-table td.target { width: 100%; } + +.herald-list-description { + color: {$darkgreytext}; + padding: 8px 0; +} + +.herald-list-icon { + margin-left: 12px; + margin-right: 2px; +} + +.herald-list-item { + padding-bottom: 4px; + color: {$greytext}; +}