Add login buttons for button logins, fix LDAP form

Summary: Ref T1536.

Test Plan:
{F46555}

{F46556}

Reviewers: chad, btrahan

Reviewed By: chad

CC: aran

Maniphest Tasks: T1536

Differential Revision: https://secure.phabricator.com/D6209

src/__celerity_resource_map__.php

@@ -921,6 +921,15 @@ celerity_register_resource_map(array(
     ),
     'disk' => '/rsrc/css/aphront/typeahead.css',
   ),
+  'auth-css' =>
+  array(
+    'uri' => '/res/8a95bad7/rsrc/css/application/auth/auth.css',
+    'type' => 'css',
+    'requires' =>
+    array(
+    ),
+    'disk' => '/rsrc/css/application/auth/auth.css',
+  ),
   'config-options-css' =>
   array(
     'uri' => '/res/be77d5a6/rsrc/css/application/config/config-options.css',
@@ -3687,7 +3696,7 @@ celerity_register_resource_map(array(
   ),
   'phui-button-css' =>
   array(
-    'uri' => '/res/8fc82931/rsrc/css/phui/phui-button.css',
+    'uri' => '/res/458ea66c/rsrc/css/phui/phui-button.css',
     'type' => 'css',
     'requires' =>
     array(
@@ -4048,7 +4057,7 @@ celerity_register_resource_map(array(
 ), array(
   'packages' =>
   array(
-    '1cdd0caf' =>
+    '116c8dcd' =>
     array(
       'name' => 'core.pkg.css',
       'symbols' =>
@@ -4096,7 +4105,7 @@ celerity_register_resource_map(array(
         40 => 'phabricator-property-list-view-css',
         41 => 'phabricator-tag-view-css',
       ),
-      'uri' => '/res/pkg/1cdd0caf/core.pkg.css',
+      'uri' => '/res/pkg/116c8dcd/core.pkg.css',
       'type' => 'css',
     ),
     'f2ad0683' =>
@@ -4290,16 +4299,16 @@ celerity_register_resource_map(array(
   'reverse' =>
   array(
     'aphront-attached-file-view-css' => 'a7ca34a9',
-    'aphront-dialog-view-css' => '1cdd0caf',
+    'aphront-dialog-view-css' => '116c8dcd',
-    'aphront-error-view-css' => '1cdd0caf',
+    'aphront-error-view-css' => '116c8dcd',
-    'aphront-form-view-css' => '1cdd0caf',
+    'aphront-form-view-css' => '116c8dcd',
-    'aphront-list-filter-view-css' => '1cdd0caf',
+    'aphront-list-filter-view-css' => '116c8dcd',
-    'aphront-pager-view-css' => '1cdd0caf',
+    'aphront-pager-view-css' => '116c8dcd',
-    'aphront-panel-view-css' => '1cdd0caf',
+    'aphront-panel-view-css' => '116c8dcd',
-    'aphront-table-view-css' => '1cdd0caf',
+    'aphront-table-view-css' => '116c8dcd',
-    'aphront-tokenizer-control-css' => '1cdd0caf',
+    'aphront-tokenizer-control-css' => '116c8dcd',
-    'aphront-tooltip-css' => '1cdd0caf',
+    'aphront-tooltip-css' => '116c8dcd',
-    'aphront-typeahead-control-css' => '1cdd0caf',
+    'aphront-typeahead-control-css' => '116c8dcd',
     'differential-changeset-view-css' => 'dd27a69b',
     'differential-core-view-css' => 'dd27a69b',
     'differential-inline-comment-editor' => '9488bb69',
@@ -4313,7 +4322,7 @@ celerity_register_resource_map(array(
     'differential-table-of-contents-css' => 'dd27a69b',
     'diffusion-commit-view-css' => 'c8ce2d88',
     'diffusion-icons-css' => 'c8ce2d88',
-    'global-drag-and-drop-css' => '1cdd0caf',
+    'global-drag-and-drop-css' => '116c8dcd',
     'inline-comment-summary-css' => 'dd27a69b',
     'javelin-aphlict' => 'f2ad0683',
     'javelin-behavior' => 'a9f14d76',
@@ -4387,55 +4396,55 @@ celerity_register_resource_map(array(
     'javelin-util' => 'a9f14d76',
     'javelin-vector' => 'a9f14d76',
     'javelin-workflow' => 'a9f14d76',
-    'lightbox-attachment-css' => '1cdd0caf',
+    'lightbox-attachment-css' => '116c8dcd',
     'maniphest-task-summary-css' => 'a7ca34a9',
     'maniphest-transaction-detail-css' => 'a7ca34a9',
-    'phabricator-action-list-view-css' => '1cdd0caf',
+    'phabricator-action-list-view-css' => '116c8dcd',
-    'phabricator-application-launch-view-css' => '1cdd0caf',
+    'phabricator-application-launch-view-css' => '116c8dcd',
     'phabricator-busy' => 'f2ad0683',
     'phabricator-content-source-view-css' => 'dd27a69b',
-    'phabricator-core-css' => '1cdd0caf',
+    'phabricator-core-css' => '116c8dcd',
-    'phabricator-crumbs-view-css' => '1cdd0caf',
+    'phabricator-crumbs-view-css' => '116c8dcd',
     'phabricator-drag-and-drop-file-upload' => '9488bb69',
     'phabricator-dropdown-menu' => 'f2ad0683',
     'phabricator-file-upload' => 'f2ad0683',
-    'phabricator-filetree-view-css' => '1cdd0caf',
+    'phabricator-filetree-view-css' => '116c8dcd',
-    'phabricator-flag-css' => '1cdd0caf',
+    'phabricator-flag-css' => '116c8dcd',
-    'phabricator-form-view-css' => '1cdd0caf',
+    'phabricator-form-view-css' => '116c8dcd',
-    'phabricator-header-view-css' => '1cdd0caf',
+    'phabricator-header-view-css' => '116c8dcd',
     'phabricator-hovercard' => 'f2ad0683',
-    'phabricator-jump-nav' => '1cdd0caf',
+    'phabricator-jump-nav' => '116c8dcd',
     'phabricator-keyboard-shortcut' => 'f2ad0683',
     'phabricator-keyboard-shortcut-manager' => 'f2ad0683',
-    'phabricator-main-menu-view' => '1cdd0caf',
+    'phabricator-main-menu-view' => '116c8dcd',
     'phabricator-menu-item' => 'f2ad0683',
-    'phabricator-nav-view-css' => '1cdd0caf',
+    'phabricator-nav-view-css' => '116c8dcd',
     'phabricator-notification' => 'f2ad0683',
-    'phabricator-notification-css' => '1cdd0caf',
+    'phabricator-notification-css' => '116c8dcd',
-    'phabricator-notification-menu-css' => '1cdd0caf',
+    'phabricator-notification-menu-css' => '116c8dcd',
-    'phabricator-object-item-list-view-css' => '1cdd0caf',
+    'phabricator-object-item-list-view-css' => '116c8dcd',
     'phabricator-object-selector-css' => 'dd27a69b',
     'phabricator-phtize' => 'f2ad0683',
     'phabricator-prefab' => 'f2ad0683',
     'phabricator-project-tag-css' => 'a7ca34a9',
-    'phabricator-property-list-view-css' => '1cdd0caf',
+    'phabricator-property-list-view-css' => '116c8dcd',
-    'phabricator-remarkup-css' => '1cdd0caf',
+    'phabricator-remarkup-css' => '116c8dcd',
     'phabricator-shaped-request' => '9488bb69',
-    'phabricator-side-menu-view-css' => '1cdd0caf',
+    'phabricator-side-menu-view-css' => '116c8dcd',
-    'phabricator-standard-page-view' => '1cdd0caf',
+    'phabricator-standard-page-view' => '116c8dcd',
-    'phabricator-tag-view-css' => '1cdd0caf',
+    'phabricator-tag-view-css' => '116c8dcd',
     'phabricator-textareautils' => 'f2ad0683',
     'phabricator-tooltip' => 'f2ad0683',
-    'phabricator-transaction-view-css' => '1cdd0caf',
+    'phabricator-transaction-view-css' => '116c8dcd',
-    'phabricator-zindex-css' => '1cdd0caf',
+    'phabricator-zindex-css' => '116c8dcd',
-    'phui-button-css' => '1cdd0caf',
+    'phui-button-css' => '116c8dcd',
-    'phui-form-css' => '1cdd0caf',
+    'phui-form-css' => '116c8dcd',
-    'phui-icon-view-css' => '1cdd0caf',
+    'phui-icon-view-css' => '116c8dcd',
-    'phui-spacing-css' => '1cdd0caf',
+    'phui-spacing-css' => '116c8dcd',
-    'sprite-apps-large-css' => '1cdd0caf',
+    'sprite-apps-large-css' => '116c8dcd',
-    'sprite-gradient-css' => '1cdd0caf',
+    'sprite-gradient-css' => '116c8dcd',
-    'sprite-icons-css' => '1cdd0caf',
+    'sprite-icons-css' => '116c8dcd',
-    'sprite-menu-css' => '1cdd0caf',
+    'sprite-menu-css' => '116c8dcd',
-    'syntax-highlighting-css' => '1cdd0caf',
+    'syntax-highlighting-css' => '116c8dcd',
   ),
 ));

src/applications/auth/controller/PhabricatorAuthStartController.php

@@ -65,9 +65,51 @@ final class PhabricatorAuthStartController
       $request->setCookie('phcid', Filesystem::readRandomCharacters(16));
     }
 
-    $out = array();
+    $not_buttons = array();
+    $are_buttons = array();
+    $providers = msort($providers, 'getLoginOrder');
     foreach ($providers as $provider) {
-      $out[] = $provider->buildLoginForm($this);
+      if ($provider->isLoginFormAButton()) {
+        $are_buttons[] = $provider->buildLoginForm($this);
+      } else {
+        $not_buttons[] = $provider->buildLoginForm($this);
+      }
+    }
+
+    $out = array();
+    $out[] = $not_buttons;
+    if ($are_buttons) {
+      require_celerity_resource('auth-css');
+
+      foreach ($are_buttons as $key => $button) {
+        $are_buttons[$key] = phutil_tag(
+          'div',
+          array(
+            'class' => 'phabricator-login-button mmb',
+          ),
+          $button);
+      }
+
+      // If we only have one button, add a second pretend button so that we
+      // always have two columns. This makes it easier to get the alignments
+      // looking reasonable.
+      if (count($are_buttons) == 1) {
+        $are_buttons[] = null;
+      }
+
+      $button_columns = id(new AphrontMultiColumnView())
+        ->setFluidLayout(true);
+      $are_buttons = array_chunk($are_buttons, ceil(count($are_buttons) / 2));
+      foreach ($are_buttons as $column) {
+        $button_columns->addColumn($column);
+      }
+
+      $out[] = phutil_tag(
+        'div',
+        array(
+          'class' => 'phabricator-login-buttons',
+        ),
+        $button_columns);
     }
 
     $login_message = PhabricatorEnv::getEnvConfig('auth.login-message');

src/applications/auth/provider/PhabricatorAuthProvider.php

@@ -167,4 +167,16 @@ abstract class PhabricatorAuthProvider {
     throw new Exception("Not implemented!");
   }
 
+  public function getLoginOrder() {
+    return '500-'.$this->getProviderName();
+  }
+
+  protected function getLoginIcon() {
+    return 'Generic';
+  }
+
+  public function isLoginFormAButton() {
+    return false;
+  }
+
 }

src/applications/auth/provider/PhabricatorAuthProviderLDAP.php

@@ -67,11 +67,17 @@ final class PhabricatorAuthProviderLDAP
 
     $viewer = $request->getUser();
 
-    $submit = id(new AphrontFormSubmitControl())
+    $dialog = id(new AphrontDialogView())
-      ->setValue(pht('Login or Register'));
+      ->setSubmitURI($this->getLoginURI())
+      ->setUser($viewer);
 
-    $header = id(new PhabricatorHeaderView())
+    if ($this->shouldAllowRegistration()) {
-      ->setHeader(pht('Login with LDAP'));
+      $dialog->setTitle(pht('Login or Register with LDAP'));
+      $dialog->addSubmitButton(pht('Login or Register'));
+    } else {
+      $dialog->setTitle(pht('Login with LDAP'));
+      $dialog->addSubmitButton(pht('Login'));
+    }
 
     $v_user = $request->getStr('ldap_username');
 
@@ -87,10 +93,9 @@ final class PhabricatorAuthProviderLDAP
       $errors[] = pht('Username or password are incorrect.');
     }
 
-    $form = id(new AphrontFormView())
+    $form = id(new AphrontFormLayoutView())
-      ->setAction($this->getLoginURI())
       ->setUser($viewer)
-      ->setFlexible(true)
+      ->setFullWidth(true)
       ->appendChild(
         id(new AphrontFormTextControl())
           ->setLabel('LDAP Username')
@@ -101,18 +106,17 @@ final class PhabricatorAuthProviderLDAP
         id(new AphrontFormPasswordControl())
           ->setLabel('LDAP Password')
           ->setName('ldap_password')
-          ->setError($e_pass))
+          ->setError($e_pass));
-      ->appendChild($submit);
 
     if ($errors) {
       $errors = id(new AphrontErrorView())->setErrors($errors);
     }
 
-    return array(
+    $dialog->appendChild($errors);
-      $errors,
+    $dialog->appendChild($form);
-      $header,
+
-      $form,
+
-    );
+    return $dialog;
   }
 
   public function processLoginRequest(

src/applications/auth/provider/PhabricatorAuthProviderOAuth.php

@@ -36,35 +36,33 @@ abstract class PhabricatorAuthProviderOAuth extends PhabricatorAuthProvider {
     return $adapter;
   }
 
+  public function isLoginFormAButton() {
+    return true;
+  }
+
   public function buildLoginForm(
     PhabricatorAuthStartController $controller) {
 
     $request = $controller->getRequest();
     $viewer = $request->getUser();
 
-    $form = id(new AphrontFormView())
-      ->setUser($viewer);
-
-    $submit = new AphrontFormSubmitControl();
-
     if ($this->shouldAllowRegistration()) {
-      $submit->setValue(
+      $button_text = pht('Login or Register');
-        pht("Login or Register with %s \xC2\xBB", $this->getProviderName()));
-      $header = pht("Login or Register with %s", $this->getProviderName());
     } else {
-      $submit->setValue(
+      $button_text = pht('Login');
-        pht("Login with %s \xC2\xBB", $this->getProviderName()));
-      $header = pht("Login with %s", $this->getProviderName());
     }
 
-    $form->appendChild($submit);
+    $icon = id(new PHUIIconView())
+      ->setSpriteSheet(PHUIIconView::SPRITE_LOGIN)
+      ->setSpriteIcon($this->getLoginIcon());
 
-    // TODO: This is pretty hideous.
+    $button = id(new PHUIButtonView())
-    $panel = new AphrontPanelView();
+        ->setTag('a')
-    $panel->setHeader($header);
+        ->setSize(PHUIButtonView::BIG)
-    $panel->setWidth(AphrontPanelView::WIDTH_FORM);
+        ->setColor(PHUIButtonView::GREY)
-    $panel->setNoBackground(true);
+        ->setIcon($icon)
-    $panel->appendChild($form);
+        ->setText($button_text)
+        ->setSubtext($this->getProviderName());
 
     $adapter = $this->getAdapter();
     $adapter->setState(PhabricatorHash::digest($request->getCookie('phcid')));
@@ -73,14 +71,25 @@ abstract class PhabricatorAuthProviderOAuth extends PhabricatorAuthProvider {
     $params = $uri->getQueryParams();
     $uri->setQueryParams(array());
 
-    $form->setAction((string)$uri);
+    $content = array($button);
+
     foreach ($params as $key => $value) {
-      $form->addHiddenInput($key, $value);
+      $content[] = phutil_tag(
+        'input',
+        array(
+          'type' => 'hidden',
+          'name' => $key,
+          'value' => $value,
+        ));
     }
 
-    $form->setMethod('GET');
+    return phabricator_form(
-
+      $viewer,
-    return $panel;
+      array(
+        'method' => 'GET',
+        'action' => (string)$uri,
+      ),
+      $content);
   }
 
   public function processLoginRequest(

src/applications/auth/provider/PhabricatorAuthProviderOAuthDisqus.php

@@ -11,6 +11,10 @@ final class PhabricatorAuthProviderOAuthDisqus
     return new PhutilAuthAdapterOAuthDisqus();
   }
 
+  protected function getLoginIcon() {
+    return 'Disqus';
+  }
+
   public function isEnabled() {
     return parent::isEnabled() &&
            PhabricatorEnv::getEnvConfig('disqus.auth-enabled');

src/applications/auth/provider/PhabricatorAuthProviderOAuthFacebook.php

@@ -11,6 +11,10 @@ final class PhabricatorAuthProviderOAuthFacebook
     return new PhutilAuthAdapterOAuthFacebook();
   }
 
+  protected function getLoginIcon() {
+    return 'Facebook';
+  }
+
   public function isEnabled() {
     return parent::isEnabled() &&
            PhabricatorEnv::getEnvConfig('facebook.auth-enabled');

src/applications/auth/provider/PhabricatorAuthProviderOAuthGitHub.php

@@ -11,6 +11,10 @@ final class PhabricatorAuthProviderOAuthGitHub
     return new PhutilAuthAdapterOAuthGitHub();
   }
 
+  protected function getLoginIcon() {
+    return 'Github';
+  }
+
   public function isEnabled() {
     return parent::isEnabled() &&
            PhabricatorEnv::getEnvConfig('github.auth-enabled');

src/applications/auth/provider/PhabricatorAuthProviderOAuthGoogle.php

@@ -11,6 +11,10 @@ final class PhabricatorAuthProviderOAuthGoogle
     return new PhutilAuthAdapterOAuthGoogle();
   }
 
+  protected function getLoginIcon() {
+    return 'Google';
+  }
+
   public function isEnabled() {
     return parent::isEnabled() &&
            PhabricatorEnv::getEnvConfig('google.auth-enabled');

src/applications/auth/provider/PhabricatorAuthProviderPassword.php

@@ -24,6 +24,11 @@ final class PhabricatorAuthProviderPassword
     return $this->adapter;
   }
 
+  public function getLoginOrder() {
+    // Make sure username/password appears first if it is enabled.
+    return '100-'.$this->getProviderName();
+  }
+
   public function shouldAllowLogin() {
     return true;
   }

src/view/phui/PHUIButtonView.php

@@ -93,8 +93,7 @@ final class PHUIButtonView extends AphrontTagView {
       $subtext = null;
       if ($this->subtext) {
         $subtext = phutil_tag(
-          'span', array('class' => 'phui-button-subtext'), $this->subtext);
+          'div', array('class' => 'phui-button-subtext'), $this->subtext);
-        $subtext = hsprintf('<br />%s', $subtext);
       }
       $text = phutil_tag(
         'div', array('class' => 'phui-button-text'), array($text, $subtext));

webroot/rsrc/css/application/auth/auth.css

@@ -0,0 +1,20 @@
+/**
+ * @provides auth-css
+ */
+
+.phabricator-login-buttons {
+  max-width: 450px;
+  margin: auto;
+}
+
+.phabricator-login-buttons .phabricator-login-button .button {
+  width: 180px;
+}
+
+.device-desktop .phabricator-login-buttons .aphront-multi-column-column-last {
+  text-align: right;
+}
+
+.device .phabricator-login-buttons {
+  text-align: center;
+}

webroot/rsrc/css/phui/phui-button.css

@@ -246,6 +246,7 @@ a.toggle-fixed {
 .button.big.has-icon {
   padding: 6px 20px 6px 12px;
   border-radius: 4px;
+  text-align: left;
 }
 
 .button.big.has-icon .phui-button-text {
@@ -257,7 +258,6 @@ a.toggle-fixed {
 .button.big.has-icon .phui-button-subtext {
   color: #888;
   font-size: 12px;
-  line-height: 16px;
+  line-height: 14px;
   font-weight: normal;
-  float: left;
 }