From c9311a9eaeba70bbc194c7327ad7884e7ca9108c Mon Sep 17 00:00:00 2001 From: epriestley Date: Thu, 3 Apr 2014 11:23:03 -0700 Subject: [PATCH] Make errors in dialogs look reasonable instead of hideous Summary: I accidentally made these exceptionally ugly recently. Test Plan: {F137411} Reviewers: btrahan, chad Reviewed By: chad Subscribers: epriestley, chad Differential Revision: https://secure.phabricator.com/D8684 --- ...AphrontDefaultApplicationConfiguration.php | 5 +---- .../ConpherenceUpdateController.php | 2 -- .../engine/PhabricatorSystemActionEngine.php | 13 +++++++----- src/view/AphrontDialogView.php | 12 +++++++++++ src/view/form/AphrontErrorView.php | 20 +----------------- webroot/rsrc/css/aphront/error-view.css | 21 +++++++++++-------- 6 files changed, 34 insertions(+), 39 deletions(-) diff --git a/src/aphront/configuration/AphrontDefaultApplicationConfiguration.php b/src/aphront/configuration/AphrontDefaultApplicationConfiguration.php index e81f77294f..483aaaf1e6 100644 --- a/src/aphront/configuration/AphrontDefaultApplicationConfiguration.php +++ b/src/aphront/configuration/AphrontDefaultApplicationConfiguration.php @@ -112,13 +112,10 @@ class AphrontDefaultApplicationConfiguration } if ($ex instanceof PhabricatorSystemActionRateLimitException) { - $error_view = id(new AphrontErrorView()) - ->setErrors(array(pht('You are being rate limited.'))); - $dialog = id(new AphrontDialogView()) ->setTitle(pht('Slow Down!')) ->setUser($user) - ->appendChild($error_view) + ->setErrors(array(pht('You are being rate limited.'))) ->appendParagraph($ex->getMessage()) ->appendParagraph($ex->getRateExplanation()) ->addCancelButton('/', pht('Okaaaaaaaaaaaaaay...')); diff --git a/src/applications/conpherence/controller/ConpherenceUpdateController.php b/src/applications/conpherence/controller/ConpherenceUpdateController.php index c4f1f55916..2b631c313e 100644 --- a/src/applications/conpherence/controller/ConpherenceUpdateController.php +++ b/src/applications/conpherence/controller/ConpherenceUpdateController.php @@ -156,8 +156,6 @@ final class ConpherenceUpdateController if ($errors) { $error_view = id(new AphrontErrorView()) - ->setTitle(pht('Errors editing conpherence.')) - ->setInsideDialogue(true) ->setErrors($errors); } diff --git a/src/applications/system/engine/PhabricatorSystemActionEngine.php b/src/applications/system/engine/PhabricatorSystemActionEngine.php index 4b38cf4840..770faf284e 100644 --- a/src/applications/system/engine/PhabricatorSystemActionEngine.php +++ b/src/applications/system/engine/PhabricatorSystemActionEngine.php @@ -15,7 +15,7 @@ final class PhabricatorSystemActionEngine extends Phobject { foreach ($blocked as $actor => $actor_score) { throw new PhabricatorSystemActionRateLimitException( $action, - $actor_score + ($score / self::getWindow())); + $actor_score); } } } @@ -25,14 +25,17 @@ final class PhabricatorSystemActionEngine extends Phobject { public static function loadBlockedActors( array $actors, - PhabricatorSystemAction $action) { + PhabricatorSystemAction $action, + $score) { $scores = self::loadScores($actors, $action); + $window = self::getWindow(); $blocked = array(); - foreach ($scores as $actor => $score) { - if ($action->shouldBlockActor($actor, $score)) { - $blocked[$actor] = $score; + foreach ($scores as $actor => $actor_score) { + $actor_score = $actor_score + ($score / $window); + if ($action->shouldBlockActor($actor, $actor_score)) { + $blocked[$actor] = $actor_score; } } diff --git a/src/view/AphrontDialogView.php b/src/view/AphrontDialogView.php index fbf5d6747a..4b2b824897 100644 --- a/src/view/AphrontDialogView.php +++ b/src/view/AphrontDialogView.php @@ -19,6 +19,7 @@ final class AphrontDialogView extends AphrontView { private $disableWorkflowOnSubmit; private $disableWorkflowOnCancel; private $width = 'default'; + private $errors; const WIDTH_DEFAULT = 'default'; const WIDTH_FORM = 'form'; @@ -34,6 +35,11 @@ final class AphrontDialogView extends AphrontView { return $this; } + public function setErrors(array $errors) { + $this->errors = $errors; + return $this; + } + public function getIsStandalone() { return $this->isStandalone; } @@ -252,6 +258,12 @@ final class AphrontDialogView extends AphrontView { $children = $this->renderChildren(); + if ($this->errors) { + $children = array( + id(new AphrontErrorView())->setErrors($this->errors), + $children); + } + $header = new PhabricatorActionHeaderView(); $header->setHeaderTitle($this->title); $header->setHeaderColor($this->headerColor); diff --git a/src/view/form/AphrontErrorView.php b/src/view/form/AphrontErrorView.php index 938f0ae9ec..393cbe9739 100644 --- a/src/view/form/AphrontErrorView.php +++ b/src/view/form/AphrontErrorView.php @@ -11,15 +11,6 @@ final class AphrontErrorView extends AphrontView { private $errors; private $severity; private $id; - private $insideDialogue; - - public function setInsideDialogue($inside_dialogue) { - $this->insideDialogue = $inside_dialogue; - return $this; - } - public function getInsideDialogue() { - return $this->insideDialogue; - } public function setTitle($title) { $this->title = $title; @@ -41,15 +32,6 @@ final class AphrontErrorView extends AphrontView { return $this; } - private function getBaseClass() { - if ($this->getInsideDialogue()) { - $class = 'aphront-error-view-dialogue'; - } else { - $class = 'aphront-error-view'; - } - return $class; - } - final public function render() { require_celerity_resource('aphront-error-view-css'); @@ -88,7 +70,7 @@ final class AphrontErrorView extends AphrontView { $this->severity = nonempty($this->severity, self::SEVERITY_ERROR); $classes = array(); - $classes[] = $this->getBaseClass(); + $classes[] = 'aphront-error-view'; $classes[] = 'aphront-error-severity-'.$this->severity; $classes = implode(' ', $classes); diff --git a/webroot/rsrc/css/aphront/error-view.css b/webroot/rsrc/css/aphront/error-view.css index f6568c75d5..0d1696b33d 100644 --- a/webroot/rsrc/css/aphront/error-view.css +++ b/webroot/rsrc/css/aphront/error-view.css @@ -2,22 +2,14 @@ * @provides aphront-error-view-css */ -.aphront-error-view, -.aphront-error-view-dialogue { +.aphront-error-view { border-style: solid; border-width: 1px; } -form.aphront-dialog-view .aphront-error-view { - margin: 0 0 12px 0; -} - .aphront-error-view { margin: 16px; } -.aphront-error-view-dialogue { - margin: 0 0 16px 0; -} .device-phone .aphront-error-view { margin: 8px; @@ -84,3 +76,14 @@ h1.aphront-error-view-head { color: {$greytext}; background-color: #fff; } + +.aphront-dialog-body .aphront-error-view { + margin: -16px -16px 16px -16px; + border-width: 0 0 1px 0; + border-bottom: 1px solid {$lightblueborder}; +} + +.aphront-dialog-body .aphront-error-view .aphront-error-view-list { + margin: 0 0 0 16px; + list-style: disc; +}