From d8713f6f0b8c09b2873e00f80999e86f3587774e Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Fri, 21 Mar 2014 14:40:05 -0700
Subject: [PATCH] Make dialogs a little easier to use

Summary:
  - Dialog pages currently have no titles or crumbs, and look shoddy. Add titles and crumbs.
  - Dialog titles aren't always great for crumbs, add an optional "short title" for crumbs.
  - `AphrontDialogResponse` is pure boilerplate. Allow controllers to just return a `DialogView` instead and get the same effect.
  - Building dialogs requires a bit of boilerplate, and we generally construct them with no explicit `"action"`, which has some issues with T4593. Provide a convenience method to set the viewer and get a reasonable, explict submit URI.

Test Plan:
  - Viewed dialog on its own.
  - Viewed dialog as a dialog.

{F132353}

Reviewers: btrahan, chad

Reviewed By: chad

Subscribers: epriestley

Differential Revision: https://secure.phabricator.com/D8577
---
 ...bricatorAuthTerminateSessionController.php | 13 ++---
 .../base/controller/PhabricatorController.php | 53 +++++++++++++++----
 src/view/AphrontDialogView.php                | 10 ++++
 3 files changed, 57 insertions(+), 19 deletions(-)

diff --git a/src/applications/auth/controller/PhabricatorAuthTerminateSessionController.php b/src/applications/auth/controller/PhabricatorAuthTerminateSessionController.php
index 7461660d3d..d2534c4a45 100644
--- a/src/applications/auth/controller/PhabricatorAuthTerminateSessionController.php
+++ b/src/applications/auth/controller/PhabricatorAuthTerminateSessionController.php
@@ -36,8 +36,7 @@ final class PhabricatorAuthTerminateSessionController
     $panel_uri = '/settings/panel/sessions/';
 
     if (!$sessions) {
-      $dialog = id(new AphrontDialogView())
-        ->setUser($viewer)
+      return $this->newDialog()
         ->setTitle(pht('No Matching Sessions'))
         ->appendParagraph(
           pht('There are no matching sessions to terminate.'))
@@ -46,8 +45,6 @@ final class PhabricatorAuthTerminateSessionController
             '(You can not terminate your current login session. To '.
             'terminate it, log out.)'))
         ->addCancelButton($panel_uri);
-
-      return id(new AphrontDialogResponse())->setDialog($dialog);
     }
 
     if ($request->isDialogFormPost()) {
@@ -59,24 +56,24 @@ final class PhabricatorAuthTerminateSessionController
 
     if ($is_all) {
       $title = pht('Terminate Sessions?');
+      $short = pht('Terminate Sessions');
       $body = pht(
         'Really terminate all sessions? (Your current login session will '.
         'not be terminated.)');
     } else {
       $title = pht('Terminate Session?');
+      $short = pht('Terminate Session');
       $body = pht(
         'Really terminate session %s?',
         phutil_tag('strong', array(), substr($session->getSessionKey(), 0, 6)));
     }
 
-    $dialog = id(new AphrontDialogView())
-      ->setUser($viewer)
+    return $this->newDialog()
       ->setTitle($title)
+      ->setShortTitle($short)
       ->appendParagraph($body)
       ->addSubmitButton(pht('Terminate'))
       ->addCancelButton($panel_uri);
-
-    return id(new AphrontDialogResponse())->setDialog($dialog);
   }
 
 
diff --git a/src/applications/base/controller/PhabricatorController.php b/src/applications/base/controller/PhabricatorController.php
index e1d3db8668..ec91c27c89 100644
--- a/src/applications/base/controller/PhabricatorController.php
+++ b/src/applications/base/controller/PhabricatorController.php
@@ -258,6 +258,11 @@ abstract class PhabricatorController extends AphrontController {
   }
 
   public function didProcessRequest($response) {
+    // If a bare DialogView is returned, wrap it in a DialogResponse.
+    if ($response instanceof AphrontDialogView) {
+      $response = id(new AphrontDialogResponse())->setDialog($response);
+    }
+
     $request = $this->getRequest();
     $response->setRequest($request);
 
@@ -278,17 +283,28 @@ abstract class PhabricatorController extends AphrontController {
 
     if ($response instanceof AphrontDialogResponse) {
       if (!$request->isAjax()) {
-        $view = new PhabricatorStandardPageView();
-        $view->setRequest($request);
-        $view->setController($this);
-        $view->appendChild(phutil_tag(
-          'div',
-          array('style' => 'padding: 2em 0;'),
-          $response->buildResponseString()));
-        $page_response = new AphrontWebpageResponse();
-        $page_response->setContent($view->render());
-        $page_response->setHTTPResponseCode($response->getHTTPResponseCode());
-        return $page_response;
+        $dialog = $response->getDialog();
+
+        $title = $dialog->getTitle();
+        $short = $dialog->getShortTitle();
+
+        $crumbs = $this->buildApplicationCrumbs();
+        $crumbs->addTextCrumb(coalesce($short, $title));
+
+        $page_content = array(
+          $crumbs,
+          $response->buildResponseString(),
+        );
+
+        $view = id(new PhabricatorStandardPageView())
+          ->setRequest($request)
+          ->setController($this)
+          ->setTitle($title)
+          ->appendChild($page_content);
+
+        $response = id(new AphrontWebpageResponse())
+          ->setContent($view->render())
+          ->setHTTPResponseCode($response->getHTTPResponseCode());
       } else {
         $response->getDialog()->setIsStandalone(true);
 
@@ -306,6 +322,7 @@ abstract class PhabricatorController extends AphrontController {
             ));
       }
     }
+
     return $response;
   }
 
@@ -448,4 +465,18 @@ abstract class PhabricatorController extends AphrontController {
   }
 
 
+  /**
+   * Create a new @{class:AphrontDialogView} with defaults filled in.
+   *
+   * @return AphrontDialogView New dialog.
+   */
+  protected function newDialog() {
+    $submit_uri = new PhutilURI($this->getRequest()->getRequestURI());
+    $submit_uri = $submit_uri->getPath();
+
+    return id(new AphrontDialogView())
+      ->setUser($this->getRequest()->getUser())
+      ->setSubmitURI($submit_uri);
+  }
+
 }
diff --git a/src/view/AphrontDialogView.php b/src/view/AphrontDialogView.php
index 117b585fdb..fbf5d6747a 100644
--- a/src/view/AphrontDialogView.php
+++ b/src/view/AphrontDialogView.php
@@ -3,6 +3,7 @@
 final class AphrontDialogView extends AphrontView {
 
   private $title;
+  private $shortTitle;
   private $submitButton;
   private $cancelURI;
   private $cancelText = 'Cancel';
@@ -51,6 +52,15 @@ final class AphrontDialogView extends AphrontView {
     return $this->title;
   }
 
+  public function setShortTitle($short_title) {
+    $this->shortTitle = $short_title;
+    return $this;
+  }
+
+  public function getShortTitle() {
+    return $this->shortTitle;
+  }
+
   public function addSubmitButton($text = null) {
     if (!$text) {
       $text = pht('Okay');