Move Clone Repository to Dialog

Summary: This moves the clone details on the Repository Home to a button / dialog. Functionally this is to pull content on the page way up, while giving full space to all the clone options. I think we can build this into some FancyJS if needed, but this seems to clean ui the UI dramatically with little overhead. I don't want to attempt the JS dropdown unless we're sure that's the best path (it exposes the most common URI by default, saving a click).

Test Plan: Tested hg, svn, git repositories and the raw URL page. Test close button.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

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

src/__phutil_library_map__.php

@@ -619,6 +619,7 @@ phutil_register_library_map(array(
     'DiffusionCachedResolveRefsQuery' => 'applications/diffusion/query/DiffusionCachedResolveRefsQuery.php',
     'DiffusionChangeController' => 'applications/diffusion/controller/DiffusionChangeController.php',
     'DiffusionChangeHeraldFieldGroup' => 'applications/diffusion/herald/DiffusionChangeHeraldFieldGroup.php',
+    'DiffusionCloneController' => 'applications/diffusion/controller/DiffusionCloneController.php',
     'DiffusionCloneURIView' => 'applications/diffusion/view/DiffusionCloneURIView.php',
     'DiffusionCommandEngine' => 'applications/diffusion/protocol/DiffusionCommandEngine.php',
     'DiffusionCommandEngineTestCase' => 'applications/diffusion/protocol/__tests__/DiffusionCommandEngineTestCase.php',
@@ -5610,6 +5611,7 @@ phutil_register_library_map(array(
     'DiffusionCachedResolveRefsQuery' => 'DiffusionLowLevelQuery',
     'DiffusionChangeController' => 'DiffusionController',
     'DiffusionChangeHeraldFieldGroup' => 'HeraldFieldGroup',
+    'DiffusionCloneController' => 'DiffusionController',
     'DiffusionCloneURIView' => 'AphrontView',
     'DiffusionCommandEngine' => 'Phobject',
     'DiffusionCommandEngineTestCase' => 'PhabricatorTestCase',

src/applications/diffusion/application/PhabricatorDiffusionApplication.php

@@ -55,6 +55,7 @@ final class PhabricatorDiffusionApplication extends PhabricatorApplication {
         '' => 'DiffusionRepositoryController',
         'repository/(?P<dblob>.*)' => 'DiffusionRepositoryController',
         'change/(?P<dblob>.*)' => 'DiffusionChangeController',
+        'clone/' => 'DiffusionCloneController',
         'history/(?P<dblob>.*)' => 'DiffusionHistoryController',
         'graph/(?P<dblob>.*)' => 'DiffusionGraphController',
         'browse/(?P<dblob>.*)' => 'DiffusionBrowseController',

src/applications/diffusion/controller/DiffusionCloneController.php

@@ -0,0 +1,122 @@
+<?php
+
+final class DiffusionCloneController extends DiffusionController {
+
+  public function shouldAllowPublic() {
+    return true;
+  }
+
+  public function handleRequest(AphrontRequest $request) {
+    $viewer = $request->getViewer();
+    $response = $this->loadDiffusionContext();
+    if ($response) {
+      return $response;
+    }
+
+    $drequest = $this->getDiffusionRequest();
+    $repository = $drequest->getRepository();
+
+    $view = id(new PHUIPropertyListView())
+      ->setUser($viewer);
+
+    $display_never = PhabricatorRepositoryURI::DISPLAY_NEVER;
+    $warning = null;
+
+    $uris = $repository->getURIs();
+    foreach ($uris as $uri) {
+      if ($uri->getIsDisabled()) {
+        continue;
+      }
+
+      if ($uri->getEffectiveDisplayType() == $display_never) {
+        continue;
+      }
+
+      if ($repository->isSVN()) {
+        $label = phutil_tag_div('diffusion-clone-label', pht('Checkout'));
+      } else {
+        $label = phutil_tag_div('diffusion-clone-label', pht('Clone'));
+      }
+
+      $view->addProperty(
+        $label,
+        $this->renderCloneURI($repository, $uri));
+    }
+
+    if (!$view->hasAnyProperties()) {
+      $view = id(new PHUIInfoView())
+        ->setSeverity(PHUIInfoView::SEVERITY_NOTICE)
+        ->appendChild(pht('Repository has no URIs set.'));
+    }
+
+    $info = null;
+
+    // Try to load alternatives. This may fail for repositories which have not
+    // cloned yet. If it does, just ignore it and continue.
+    try {
+      $alternatives = $drequest->getRefAlternatives();
+    } catch (ConduitClientException $ex) {
+      $alternatives = array();
+    }
+
+    if ($alternatives) {
+      $message = array(
+        pht(
+          'The ref "%s" is ambiguous in this repository.',
+          $drequest->getBranch()),
+        ' ',
+        phutil_tag(
+          'a',
+          array(
+            'href' => $drequest->generateURI(
+              array(
+                'action' => 'refs',
+              )),
+          ),
+          pht('View Alternatives')),
+      );
+
+      $messages = array($message);
+
+      $warning = id(new PHUIInfoView())
+        ->setSeverity(PHUIInfoView::SEVERITY_WARNING)
+        ->setErrors(array($message));
+    }
+
+    $cancel_uri = $drequest->generateURI(
+      array(
+        'action' => 'branch',
+        'path' => '/',
+      ));
+
+    return $this->newDialog()
+      ->setTitle(pht('Clone Repository'))
+      ->setWidth(AphrontDialogView::WIDTH_FORM)
+      ->addCancelButton($cancel_uri, pht('Close'))
+      ->appendChild(array($view, $warning));
+  }
+
+  private function renderCloneURI(
+    PhabricatorRepository $repository,
+    PhabricatorRepositoryURI $uri) {
+
+    if ($repository->isSVN()) {
+      $display = csprintf(
+        'svn checkout %R %R',
+        (string)$uri->getDisplayURI(),
+        $repository->getCloneName());
+    } else {
+      $display = csprintf('%R', (string)$uri->getDisplayURI());
+    }
+
+    $display = (string)$display;
+    $viewer = $this->getViewer();
+
+    return id(new DiffusionCloneURIView())
+      ->setViewer($viewer)
+      ->setRepository($repository)
+      ->setRepositoryURI($uri)
+      ->setDisplayURI($display);
+  }
+
+}

src/applications/diffusion/controller/DiffusionRepositoryController.php

@@ -27,7 +27,6 @@ final class DiffusionRepositoryController extends DiffusionController {
     $crumbs->setBorder(true);
 
     $header = $this->buildHeaderView($repository);
-    $property_table = $this->buildPropertiesTable($repository);
     $actions = $this->buildActionList($repository);
     $description = $this->buildDescriptionView($repository);
     $locate_file = $this->buildLocateFile();
@@ -90,12 +89,28 @@ final class DiffusionRepositoryController extends DiffusionController {
 
     $tabs = $this->buildTabsView('home');
 
+    $clone_uri = $drequest->generateURI(
+      array(
+        'action' => 'clone',
+      ));
+
+    $clone_button = id(new PHUIButtonView())
+      ->setTag('a')
+      ->setText('Clone')
+      ->setColor(PHUIButtonView::GREEN)
+      ->setIcon('fa-download')
+      ->setWorkflow(true)
+      ->setHref($clone_uri);
+
+    $bar = id(new PHUILeftRightView())
+      ->setLeft($locate_file)
+      ->setRight($clone_button);
+
     $view = id(new PHUITwoColumnView())
       ->setHeader($header)
       ->setTabs($tabs)
       ->setFooter(array(
-        $locate_file,
+        $bar,
-        $property_table,
         $description,
         $content,
       ));
@@ -302,87 +317,6 @@ final class DiffusionRepositoryController extends DiffusionController {
     return null;
   }
 
-  private function buildPropertiesTable(PhabricatorRepository $repository) {
-    $viewer = $this->getViewer();
-
-    $view = id(new PHUIPropertyListView())
-      ->setUser($viewer);
-
-    $display_never = PhabricatorRepositoryURI::DISPLAY_NEVER;
-
-    $uris = $repository->getURIs();
-    foreach ($uris as $uri) {
-      if ($uri->getIsDisabled()) {
-        continue;
-      }
-
-      if ($uri->getEffectiveDisplayType() == $display_never) {
-        continue;
-      }
-
-      if ($repository->isSVN()) {
-        $label = phutil_tag_div('diffusion-clone-label', pht('Checkout'));
-      } else {
-        $label = phutil_tag_div('diffusion-clone-label', pht('Clone'));
-      }
-
-      $view->addProperty(
-        $label,
-        $this->renderCloneURI($repository, $uri));
-    }
-
-    if (!$view->hasAnyProperties()) {
-      $view = id(new PHUIInfoView())
-        ->setSeverity(PHUIInfoView::SEVERITY_NOTICE)
-        ->appendChild(pht('Repository has no URIs set.'));
-    }
-
-    $box = id(new PHUIObjectBoxView())
-      ->setHeaderText(pht('Details'))
-      ->setBackground(PHUIObjectBoxView::BLUE_PROPERTY)
-      ->appendChild($view);
-
-    $info = null;
-    $drequest = $this->getDiffusionRequest();
-
-    // Try to load alternatives. This may fail for repositories which have not
-    // cloned yet. If it does, just ignore it and continue.
-    try {
-      $alternatives = $drequest->getRefAlternatives();
-    } catch (ConduitClientException $ex) {
-      $alternatives = array();
-    }
-
-    if ($alternatives) {
-      $message = array(
-        pht(
-          'The ref "%s" is ambiguous in this repository.',
-          $drequest->getBranch()),
-        ' ',
-        phutil_tag(
-          'a',
-          array(
-            'href' => $drequest->generateURI(
-              array(
-                'action' => 'refs',
-              )),
-          ),
-          pht('View Alternatives')),
-      );
-
-      $messages = array($message);
-
-      $info = id(new PHUIInfoView())
-        ->setSeverity(PHUIInfoView::SEVERITY_WARNING)
-        ->setErrors(array($message));
-
-      $box->setInfoView($info);
-    }
-
-
-    return $box;
-  }
-
   private function buildHistoryTable(
     $history_results,
     $history,
@@ -504,52 +438,13 @@ final class DiffusionRepositoryController extends DiffusionController {
     }
 
     $browse_uri = $drequest->generateURI(array('action' => 'browse'));
-
-    $browse_panel = id(new PHUIObjectBoxView())
-      ->setBackground(PHUIObjectBoxView::BLUE_PROPERTY);
-    $header = id(new PHUIHeaderView())
-      ->setHeader($repository->getName());
-
-    $button = id(new PHUIButtonView())
-      ->setText(pht('Browse'))
-      ->setTag('a')
-      ->setIcon('fa-code')
-      ->setHref($browse_uri);
-
-    $header->addActionLink($button);
-    $browse_panel->setHeader($header);
-    $browse_panel->setTable($browse_table);
-
     $pager->setURI($browse_uri, 'offset');
 
-    if ($pager->willShowPagingControls()) {
+    return id(new PHUIObjectBoxView())
-      $browse_panel->setPager($pager);
+      ->setHeaderText($repository->getName())
-    }
+      ->setBackground(PHUIObjectBoxView::BLUE_PROPERTY)
-
+      ->setTable($browse_table)
-    return $browse_panel;
+      ->setPager($pager);
-  }
-
-  private function renderCloneURI(
-    PhabricatorRepository $repository,
-    PhabricatorRepositoryURI $uri) {
-
-    if ($repository->isSVN()) {
-      $display = csprintf(
-        'svn checkout %R %R',
-        (string)$uri->getDisplayURI(),
-        $repository->getCloneName());
-    } else {
-      $display = csprintf('%R', (string)$uri->getDisplayURI());
-    }
-
-    $display = (string)$display;
-    $viewer = $this->getViewer();
-
-    return id(new DiffusionCloneURIView())
-      ->setViewer($viewer)
-      ->setRepository($repository)
-      ->setRepositoryURI($uri)
-      ->setDisplayURI($display);
   }
 
   private function getTagLimit() {

src/applications/repository/storage/PhabricatorRepository.php

@@ -700,6 +700,7 @@ final class PhabricatorRepository extends PhabricatorRepositoryDAO
     switch ($action) {
       case 'history':
       case 'graph':
+      case 'clone':
       case 'browse':
       case 'change':
       case 'lastmodified':
@@ -818,6 +819,9 @@ final class PhabricatorRepository extends PhabricatorRepositoryDAO
         // it came from a URI.
         $uri = rawurldecode("{$path}{$commit}");
         break;
+      case 'clone':
+        $uri = $this->getPathURI("/{$action}/");
+      break;
     }
 
     if ($action == 'rendering-ref') {