Modernize file browse controller

Summary: This needs some more cleanup, but gets us a step closer to something reasonable.

Test Plan: See screenshot.

Reviewers: btrahan, chad

Reviewed By: btrahan

CC: aran

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

src/applications/diffusion/controller/DiffusionBrowseController.php

@@ -30,7 +30,7 @@ final class DiffusionBrowseController extends DiffusionController {
     $content = array();
 
     $content[] = $this->buildHeaderView($drequest);
-    $content[] = $this->buildActionView($drequest);
+    $content[] = $this->buildBrowseActionView($drequest);
     $content[] = $this->buildPropertyView($drequest);
 
     $content[] = $this->renderSearchForm();
@@ -278,55 +278,6 @@ final class DiffusionBrowseController extends DiffusionController {
     return $header;
   }
 
-  private function buildActionView(DiffusionRequest $drequest) {
-    $viewer = $this->getRequest()->getUser();
-
-    $view = id(new PhabricatorActionListView())
-      ->setUser($viewer);
-
-    $history_uri = $drequest->generateURI(
-      array(
-        'action' => 'history',
-      ));
-
-    $view->addAction(
-      id(new PhabricatorActionView())
-        ->setName(pht('View History'))
-        ->setHref($history_uri)
-        ->setIcon('perflab'));
-
-    $behind_head = $drequest->getRawCommit();
-    $head_uri = $drequest->generateURI(
-      array(
-        'commit' => '',
-        'action' => 'browse',
-      ));
-    $view->addAction(
-      id(new PhabricatorActionView())
-        ->setName(pht('Jump to HEAD'))
-        ->setHref($head_uri)
-        ->setIcon('home')
-        ->setDisabled(!$behind_head));
-
-    // TODO: Ideally, this should live in Owners and be event-triggered, but
-    // there's no reasonable object for it to react to right now.
-
-    $owners_uri = id(new PhutilURI('/owners/view/search/'))
-      ->setQueryParams(
-        array(
-          'repository' => $drequest->getCallsign(),
-          'path' => '/'.$drequest->getPath(),
-        ));
-
-    $view->addAction(
-      id(new PhabricatorActionView())
-        ->setName(pht('Find Owners'))
-        ->setHref((string)$owners_uri)
-        ->setIcon('preview'));
-
-    return $view;
-  }
-
   private function buildPropertyView(DiffusionRequest $drequest) {
     $viewer = $this->getRequest()->getUser();
 
@@ -361,47 +312,4 @@ final class DiffusionBrowseController extends DiffusionController {
     return $view;
   }
 
-  private function renderPathLinks(DiffusionRequest $drequest) {
-    $path = $drequest->getPath();
-    $path_parts = array_filter(explode('/', trim($path, '/')));
-
-    $links = array();
-    if ($path_parts) {
-      $links[] = phutil_tag(
-        'a',
-        array(
-          'href' => $drequest->generateURI(
-            array(
-              'action' => 'browse',
-              'path' => '',
-            )),
-        ),
-        'r'.$drequest->getRepository()->getCallsign().'/');
-      $accum = '';
-      $last_key = last_key($path_parts);
-      foreach ($path_parts as $key => $part) {
-        $links[] = ' ';
-        $accum .= '/'.$part;
-        if ($key === $last_key) {
-          $links[] = $part;
-        } else {
-          $links[] = phutil_tag(
-            'a',
-            array(
-              'href' => $drequest->generateURI(
-                array(
-                  'action' => 'browse',
-                  'path' => $accum,
-                )),
-            ),
-            $part.'/');
-        }
-      }
-    } else {
-      $links[] = 'r'.$drequest->getRepository()->getCallsign().'/';
-    }
-
-    return $links;
-  }
-
 }

src/applications/diffusion/controller/DiffusionBrowseFileController.php

@@ -71,15 +71,14 @@ final class DiffusionBrowseFileController extends DiffusionController {
 
     require_celerity_resource('diffusion-source-css');
 
-    if ($this->corpusType == 'text') {
-      $view_select_panel = $this->renderViewSelectPanel($selected);
-    } else {
-      $view_select_panel = null;
-    }
-
     // Render the page.
     $content = array();
 
+    $content[] = $this->buildHeaderView($drequest);
+    $view = $this->buildBrowseActionView($drequest);
+    $content[] = $this->enrichActionView($view, $drequest, $selected);
+    $content[] = $this->buildPropertyView($drequest);
+
     $follow  = $request->getStr('follow');
     if ($follow) {
       $notice = new AphrontErrorView();
@@ -111,24 +110,23 @@ final class DiffusionBrowseFileController extends DiffusionController {
       $content[] = $notice;
     }
 
-    $content[] = $view_select_panel;
     $content[] = $corpus;
     $content[] = $this->buildOpenRevisions();
 
-    $nav = $this->buildSideNav('browse', true);
-    $nav->appendChild($content);
     $crumbs = $this->buildCrumbs(
       array(
         'branch' => true,
         'path'   => true,
         'view'   => 'browse',
       ));
-    $nav->setCrumbs($crumbs);
 
     $basename = basename($this->getDiffusionRequest()->getPath());
 
     return $this->buildApplicationPage(
-      $nav,
+      array(
+        $crumbs,
+        $content,
+      ),
       array(
         'title' => $basename,
       ));
@@ -296,7 +294,14 @@ final class DiffusionBrowseFileController extends DiffusionController {
     return $corpus;
   }
 
-  private function renderViewSelectPanel($selected) {
+  private function enrichActionView(
+    PhabricatorActionListView $view,
+    DiffusionRequest $drequest,
+    $selected) {
+
+    $viewer = $this->getRequest()->getUser();
+    $base_uri = $this->getRequest()->getRequestURI();
+
     $toggle_blame = array(
       'highlighted'   => 'blame',
       'blame'         => 'highlighted',
@@ -304,6 +309,7 @@ final class DiffusionBrowseFileController extends DiffusionController {
       'plainblame'    => 'plain',
       'raw'           => 'raw',  // not a real case.
     );
+
     $toggle_highlight = array(
       'highlighted'   => 'plain',
       'blame'         => 'plainblame',
@@ -312,32 +318,40 @@ final class DiffusionBrowseFileController extends DiffusionController {
       'raw'           => 'raw',  // not a real case.
     );
 
-    $user = $this->getRequest()->getUser();
-    $base_uri = $this->getRequest()->getRequestURI();
-
     $blame_on = ($selected == 'blame' || $selected == 'plainblame');
     if ($blame_on) {
       $blame_text = pht('Disable Blame');
+      $blame_icon = 'blame-grey';
     } else {
       $blame_text = pht('Enable Blame');
+      $blame_icon = 'blame';
     }
 
-    $blame_button = $this->createViewAction(
+    $view->addAction(
-      $blame_text,
+      id(new PhabricatorActionView())
-      $base_uri->alter('view', $toggle_blame[$selected]),
+        ->setName($blame_text)
-      $user);
+        ->setHref($base_uri->alter('view', $toggle_blame[$selected]))
+        ->setIcon($blame_icon)
+        ->setUser($viewer)
+        ->setRenderAsForm(true));
 
 
     $highlight_on = ($selected == 'blame' || $selected == 'highlighted');
     if ($highlight_on) {
       $highlight_text = pht('Disable Highlighting');
+      $highlight_icon = 'highlight-grey';
     } else {
       $highlight_text = pht('Enable Highlighting');
+      $highlight_icon = 'highlight';
     }
-    $highlight_button = $this->createViewAction(
+
-      $highlight_text,
+    $view->addAction(
-      $base_uri->alter('view', $toggle_highlight[$selected]),
+      id(new PhabricatorActionView())
-      $user);
+        ->setName($highlight_text)
+        ->setHref($base_uri->alter('view', $toggle_highlight[$selected]))
+        ->setIcon($highlight_icon)
+        ->setUser($viewer)
+        ->setRenderAsForm(true));
 
 
     $href = null;
@@ -347,7 +361,6 @@ final class DiffusionBrowseFileController extends DiffusionController {
 
     } else if ($this->lintCommit === null) {
       $lint_text = pht('Lint not Available');
-
     } else {
       $lint_text = pht(
         'Show %d Lint Message(s)',
@@ -358,46 +371,22 @@ final class DiffusionBrowseFileController extends DiffusionController {
       ))->alter('lint', '');
     }
 
-    $lint_button = $this->createViewAction(
+    $view->addAction(
-      $lint_text,
+      id(new PhabricatorActionView())
-      $href,
+        ->setName($lint_text)
-      $user);
+        ->setHref($href)
+        ->setIcon('warning')
+        ->setDisabled(!$href));
 
-    if (!$href) {
+    $view->addAction(
-      $lint_button->setDisabled(true);
+      id(new PhabricatorActionView())
-    }
+        ->setName(pht('View Raw File'))
+        ->setHref($base_uri->alter('view', 'raw'))
+        ->setIcon('file'));
 
+    $view->addAction($this->createEditAction());
 
-    $raw_button = $this->createViewAction(
+    return $view;
-      pht('View Raw File'),
-      $base_uri->alter('view', 'raw'),
-      $user,
-      'file');
-
-    $edit_button = $this->createEditAction();
-
-    return id(new PhabricatorActionListView())
-      ->setUser($user)
-      ->setObjectURI($this->getRequest()->getRequestURI())
-      ->addAction($blame_button)
-      ->addAction($highlight_button)
-      ->addAction($lint_button)
-      ->addAction($raw_button)
-      ->addAction($edit_button);
-  }
-
-  private function createViewAction(
-    $localized_text,
-    $href,
-    $user,
-    $icon = null) {
-
-    return id(new PhabricatorActionView())
-          ->setName($localized_text)
-          ->setIcon($icon)
-          ->setUser($user)
-          ->setRenderAsForm(true)
-          ->setHref($href);
   }
 
   private function createEditAction() {
@@ -979,4 +968,40 @@ final class DiffusionBrowseFileController extends DiffusionController {
     return head($parents);
   }
 
+  private function buildHeaderView(DiffusionRequest $drequest) {
+    $viewer = $this->getRequest()->getUser();
+
+    $header = id(new PHUIHeaderView())
+      ->setUser($viewer)
+      ->setHeader($this->renderPathLinks($drequest))
+      ->setPolicyObject($drequest->getRepository());
+
+    return $header;
+  }
+
+  private function buildPropertyView(DiffusionRequest $drequest) {
+    $viewer = $this->getRequest()->getUser();
+
+    $view = id(new PhabricatorPropertyListView())
+      ->setUser($viewer);
+
+    $stable_commit = $drequest->getStableCommitName();
+    $callsign = $drequest->getRepository()->getCallsign();
+
+    $view->addProperty(
+      pht('Commit'),
+      phutil_tag(
+        'a',
+        array(
+          'href' => $drequest->generateURI(
+            array(
+              'action' => 'commit',
+              'commit' => $stable_commit,
+            )),
+        ),
+        $drequest->getRepository()->formatCommitName($stable_commit)));
+
+    return $view;
+  }
+
 }

src/applications/diffusion/controller/DiffusionController.php

@@ -344,4 +344,98 @@ abstract class DiffusionController extends PhabricatorController {
     return $this->getApplicationURI($repository->getCallsign().'/'.$path);
   }
 
+
+  protected function buildBrowseActionView(DiffusionRequest $drequest) {
+    $viewer = $this->getRequest()->getUser();
+
+    $view = id(new PhabricatorActionListView())
+      ->setUser($viewer);
+
+    $history_uri = $drequest->generateURI(
+      array(
+        'action' => 'history',
+      ));
+
+    $view->addAction(
+      id(new PhabricatorActionView())
+        ->setName(pht('View History'))
+        ->setHref($history_uri)
+        ->setIcon('perflab'));
+
+    $behind_head = $drequest->getRawCommit();
+    $head_uri = $drequest->generateURI(
+      array(
+        'commit' => '',
+        'action' => 'browse',
+      ));
+    $view->addAction(
+      id(new PhabricatorActionView())
+        ->setName(pht('Jump to HEAD'))
+        ->setHref($head_uri)
+        ->setIcon('home')
+        ->setDisabled(!$behind_head));
+
+    // TODO: Ideally, this should live in Owners and be event-triggered, but
+    // there's no reasonable object for it to react to right now.
+
+    $owners_uri = id(new PhutilURI('/owners/view/search/'))
+      ->setQueryParams(
+        array(
+          'repository' => $drequest->getCallsign(),
+          'path' => '/'.$drequest->getPath(),
+        ));
+
+    $view->addAction(
+      id(new PhabricatorActionView())
+        ->setName(pht('Find Owners'))
+        ->setHref((string)$owners_uri)
+        ->setIcon('preview'));
+
+    return $view;
+  }
+
+  protected function renderPathLinks(DiffusionRequest $drequest) {
+    $path = $drequest->getPath();
+    $path_parts = array_filter(explode('/', trim($path, '/')));
+
+    $links = array();
+    if ($path_parts) {
+      $links[] = phutil_tag(
+        'a',
+        array(
+          'href' => $drequest->generateURI(
+            array(
+              'action' => 'browse',
+              'path' => '',
+            )),
+        ),
+        'r'.$drequest->getRepository()->getCallsign().'/');
+      $accum = '';
+      $last_key = last_key($path_parts);
+      foreach ($path_parts as $key => $part) {
+        $links[] = ' ';
+        $accum .= '/'.$part;
+        if ($key === $last_key) {
+          $links[] = $part;
+        } else {
+          $links[] = phutil_tag(
+            'a',
+            array(
+              'href' => $drequest->generateURI(
+                array(
+                  'action' => 'browse',
+                  'path' => $accum,
+                )),
+            ),
+            $part.'/');
+        }
+      }
+    } else {
+      $links[] = 'r'.$drequest->getRepository()->getCallsign().'/';
+    }
+
+    return $links;
+  }
+
 }
+