From 8c8d56dc5636df949e7bacf77abfeb9e51be7b65 Mon Sep 17 00:00:00 2001 From: epriestley Date: Tue, 5 Feb 2019 05:50:58 -0800 Subject: [PATCH] Replace "Add Auth Provider" radio buttons with a more modern "click to select" UI Summary: Depends on D20094. Ref T13244. Ref T6703. See PHI774. Currently, we use an older-style radio-button UI to choose an auth provider type (Google, Password, LDAP, etc). Instead, use a more modern click-to-select UI. Test Plan: {F6184343} Reviewers: amckinley Reviewed By: amckinley Maniphest Tasks: T13244, T6703 Differential Revision: https://secure.phabricator.com/D20095 --- .../PhabricatorAuthApplication.php | 3 +- .../config/PhabricatorAuthEditController.php | 6 +- .../config/PhabricatorAuthNewController.php | 127 +++++++----------- .../auth/provider/PhabricatorAuthProvider.php | 6 + 4 files changed, 56 insertions(+), 86 deletions(-) diff --git a/src/applications/auth/application/PhabricatorAuthApplication.php b/src/applications/auth/application/PhabricatorAuthApplication.php index df86595b46..15d753a286 100644 --- a/src/applications/auth/application/PhabricatorAuthApplication.php +++ b/src/applications/auth/application/PhabricatorAuthApplication.php @@ -48,8 +48,7 @@ final class PhabricatorAuthApplication extends PhabricatorApplication { '' => 'PhabricatorAuthListController', 'config/' => array( 'new/' => 'PhabricatorAuthNewController', - 'new/(?P[^/]+)/' => 'PhabricatorAuthEditController', - 'edit/(?P\d+)/' => 'PhabricatorAuthEditController', + 'edit/(?:(?P\d+)/)?' => 'PhabricatorAuthEditController', '(?Penable|disable)/(?P\d+)/' => 'PhabricatorAuthDisableController', ), diff --git a/src/applications/auth/controller/config/PhabricatorAuthEditController.php b/src/applications/auth/controller/config/PhabricatorAuthEditController.php index 6ff0be4383..016fe51b1a 100644 --- a/src/applications/auth/controller/config/PhabricatorAuthEditController.php +++ b/src/applications/auth/controller/config/PhabricatorAuthEditController.php @@ -6,8 +6,9 @@ final class PhabricatorAuthEditController public function handleRequest(AphrontRequest $request) { $this->requireApplicationCapability( AuthManageProvidersCapability::CAPABILITY); - $viewer = $request->getUser(); - $provider_class = $request->getURIData('className'); + + $viewer = $this->getViewer(); + $provider_class = $request->getStr('provider'); $config_id = $request->getURIData('id'); if ($config_id) { @@ -275,6 +276,7 @@ final class PhabricatorAuthEditController $form = id(new AphrontFormView()) ->setUser($viewer) + ->addHiddenInput('provider', $provider_class) ->appendChild( id(new AphrontFormCheckboxControl()) ->setLabel(pht('Allow')) diff --git a/src/applications/auth/controller/config/PhabricatorAuthNewController.php b/src/applications/auth/controller/config/PhabricatorAuthNewController.php index dbd43f9ea8..c8fd0ad8a5 100644 --- a/src/applications/auth/controller/config/PhabricatorAuthNewController.php +++ b/src/applications/auth/controller/config/PhabricatorAuthNewController.php @@ -6,44 +6,12 @@ final class PhabricatorAuthNewController public function handleRequest(AphrontRequest $request) { $this->requireApplicationCapability( AuthManageProvidersCapability::CAPABILITY); - $request = $this->getRequest(); - $viewer = $request->getUser(); + + $viewer = $this->getViewer(); + $cancel_uri = $this->getApplicationURI(); $providers = PhabricatorAuthProvider::getAllBaseProviders(); - $e_provider = null; - $errors = array(); - - if ($request->isFormPost()) { - $provider_string = $request->getStr('provider'); - if (!strlen($provider_string)) { - $e_provider = pht('Required'); - $errors[] = pht('You must select an authentication provider.'); - } else { - $found = false; - foreach ($providers as $provider) { - if (get_class($provider) === $provider_string) { - $found = true; - break; - } - } - if (!$found) { - $e_provider = pht('Invalid'); - $errors[] = pht('You must select a valid provider.'); - } - } - - if (!$errors) { - return id(new AphrontRedirectResponse())->setURI( - $this->getApplicationURI('/config/new/'.$provider_string.'/')); - } - } - - $options = id(new AphrontFormRadioButtonControl()) - ->setLabel(pht('Provider')) - ->setName('provider') - ->setError($e_provider); - $configured = PhabricatorAuthProvider::getAllProviders(); $configured_classes = array(); foreach ($configured as $configured_provider) { @@ -55,57 +23,52 @@ final class PhabricatorAuthNewController $providers = msort($providers, 'getLoginOrder'); $providers = array_diff_key($providers, $configured_classes) + $providers; - foreach ($providers as $provider) { - if (isset($configured_classes[get_class($provider)])) { - $disabled = true; - $description = pht('This provider is already configured.'); + $menu = id(new PHUIObjectItemListView()) + ->setViewer($viewer) + ->setBig(true) + ->setFlush(true); + + foreach ($providers as $provider_key => $provider) { + $provider_class = get_class($provider); + + $provider_uri = id(new PhutilURI('/config/edit/')) + ->setQueryParam('provider', $provider_class); + $provider_uri = $this->getApplicationURI($provider_uri); + + $already_exists = isset($configured_classes[get_class($provider)]); + + $item = id(new PHUIObjectItemView()) + ->setHeader($provider->getNameForCreate()) + ->setImageIcon($provider->newIconView()) + ->addAttribute($provider->getDescriptionForCreate()); + + if (!$already_exists) { + $item + ->setHref($provider_uri) + ->setClickable(true); } else { - $disabled = false; - $description = $provider->getDescriptionForCreate(); + $item->setDisabled(true); } - $options->addButton( - get_class($provider), - $provider->getNameForCreate(), - $description, - $disabled ? 'disabled' : null, - $disabled); + + if ($already_exists) { + $messages = array(); + $messages[] = pht('You already have a provider of this type.'); + + $info = id(new PHUIInfoView()) + ->setSeverity(PHUIInfoView::SEVERITY_WARNING) + ->setErrors($messages); + + $item->appendChild($info); + } + + $menu->addItem($item); } - $form = id(new AphrontFormView()) - ->setUser($viewer) - ->appendChild($options) - ->appendChild( - id(new AphrontFormSubmitControl()) - ->addCancelButton($this->getApplicationURI()) - ->setValue(pht('Continue'))); - - $form_box = id(new PHUIObjectBoxView()) - ->setHeaderText(pht('Provider')) - ->setFormErrors($errors) - ->setBackground(PHUIObjectBoxView::BLUE_PROPERTY) - ->setForm($form); - - $crumbs = $this->buildApplicationCrumbs(); - $crumbs->addTextCrumb(pht('Add Provider')); - $crumbs->setBorder(true); - - $title = pht('Add Auth Provider'); - - $header = id(new PHUIHeaderView()) - ->setHeader($title) - ->setHeaderIcon('fa-plus-square'); - - $view = id(new PHUITwoColumnView()) - ->setHeader($header) - ->setFooter(array( - $form_box, - )); - - return $this->newPage() - ->setTitle($title) - ->setCrumbs($crumbs) - ->appendChild($view); - + return $this->newDialog() + ->setTitle(pht('Add Auth Provider')) + ->setWidth(AphrontDialogView::WIDTH_FORM) + ->appendChild($menu) + ->addCancelButton($cancel_uri); } } diff --git a/src/applications/auth/provider/PhabricatorAuthProvider.php b/src/applications/auth/provider/PhabricatorAuthProvider.php index 0525edad54..bcccca5121 100644 --- a/src/applications/auth/provider/PhabricatorAuthProvider.php +++ b/src/applications/auth/provider/PhabricatorAuthProvider.php @@ -311,6 +311,12 @@ abstract class PhabricatorAuthProvider extends Phobject { return 'Generic'; } + public function newIconView() { + return id(new PHUIIconView()) + ->setSpriteSheet(PHUIIconView::SPRITE_LOGIN) + ->setSpriteIcon($this->getLoginIcon()); + } + public function isLoginFormAButton() { return false; }