From a235768d586e29f2213552e4074efbe04356383a Mon Sep 17 00:00:00 2001 From: epriestley Date: Fri, 4 Oct 2013 15:17:01 -0700 Subject: [PATCH] Modernize "Test Console" and fix a minor display bug with "Always" Summary: - Use the box view in the test console. - Let the test console load tasks and mocks. We should move this to the adapters (`canAdaptObject($object)` or something). - Fix a minor issue with "Always": hiding the whole cell could make the table layout weird in Safari, at least. Just hide the select instead. Test Plan: - Used test console on task. - Used test console on mock. - Created (silly) rule with "Always" and also some other conditions. Reviewers: btrahan Reviewed By: btrahan CC: aran Differential Revision: https://secure.phabricator.com/D7220 --- src/__celerity_resource_map__.php | 2 +- .../HeraldTestConsoleController.php | 31 +++++++++++-------- .../js/application/herald/HeraldRuleEditor.js | 9 ++---- 3 files changed, 21 insertions(+), 21 deletions(-) diff --git a/src/__celerity_resource_map__.php b/src/__celerity_resource_map__.php index 76c02abf09..ea8c7c2079 100644 --- a/src/__celerity_resource_map__.php +++ b/src/__celerity_resource_map__.php @@ -1176,7 +1176,7 @@ celerity_register_resource_map(array( ), 'herald-rule-editor' => array( - 'uri' => '/res/36222dde/rsrc/js/application/herald/HeraldRuleEditor.js', + 'uri' => '/res/c42c0444/rsrc/js/application/herald/HeraldRuleEditor.js', 'type' => 'js', 'requires' => array( diff --git a/src/applications/herald/controller/HeraldTestConsoleController.php b/src/applications/herald/controller/HeraldTestConsoleController.php index 8630d52fe3..0b36b9a41f 100644 --- a/src/applications/herald/controller/HeraldTestConsoleController.php +++ b/src/applications/herald/controller/HeraldTestConsoleController.php @@ -31,6 +31,9 @@ final class HeraldTestConsoleController extends HeraldController { } if (!$errors) { + + // TODO: Let the adapters claim objects instead. + if ($object instanceof DifferentialRevision) { $adapter = HeraldDifferentialRevisionAdapter::newLegacyAdapter( $object, @@ -43,6 +46,12 @@ final class HeraldTestConsoleController extends HeraldController { $object->getRepository(), $object, $data); + } else if ($object instanceof ManiphestTask) { + $adapter = id(new HeraldManiphestTaskAdapter()) + ->setTask($object); + } else if ($object instanceof PholioMock) { + $adapter = id(new HeraldPholioMockAdapter()) + ->setMock($object); } else { throw new Exception("Can not build adapter for object!"); } @@ -77,10 +86,10 @@ final class HeraldTestConsoleController extends HeraldController { $error_view = null; } - $text = pht('Enter an object to test rules '. - 'for, like a Diffusion commit (e.g., rX123) or a '. - 'Differential revision (e.g., D123). You will be shown the '. - 'results of a dry run on the object.'); + $text = pht( + 'Enter an object to test rules for, like a Diffusion commit (e.g., '. + 'rX123) or a Differential revision (e.g., D123). You will be shown '. + 'the results of a dry run on the object.'); $form = id(new AphrontFormView()) ->setUser($user) @@ -96,13 +105,10 @@ final class HeraldTestConsoleController extends HeraldController { id(new AphrontFormSubmitControl()) ->setValue(pht('Test Rules'))); - $nav = $this->buildSideNavView(); - $nav->selectFilter('test'); - $nav->appendChild( - array( - $error_view, - $form, - )); + $box = id(new PHUIObjectBoxView()) + ->setHeaderText(pht('Herald Test Console')) + ->setFormError($error_view) + ->setForm($form); $crumbs = id($this->buildApplicationCrumbs()) ->addCrumb( @@ -112,10 +118,9 @@ final class HeraldTestConsoleController extends HeraldController { ->addCrumb( id(new PhabricatorCrumbView()) ->setName(pht('Test Console'))); - $nav->setCrumbs($crumbs); return $this->buildApplicationPage( - $nav, + $box, array( 'title' => pht('Test Console'), 'device' => true, diff --git a/webroot/rsrc/js/application/herald/HeraldRuleEditor.js b/webroot/rsrc/js/application/herald/HeraldRuleEditor.js index 55b50e3a29..a09af6b994 100644 --- a/webroot/rsrc/js/application/herald/HeraldRuleEditor.js +++ b/webroot/rsrc/js/application/herald/HeraldRuleEditor.js @@ -165,13 +165,8 @@ JX.install('HeraldRuleEditor', { this._onconditionchange(r); var condition_name = this._config.conditions[row_id][1]; - switch (condition_name) { - case 'unconditionally': - JX.DOM.hide(condition_cell); - break; - default: - JX.DOM.show(condition_cell); - break; + if (condition_name == 'unconditionally') { + JX.DOM.hide(condition_select); } }, _onconditionchange : function(r) {